dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.13k stars 4.7k forks source link

Injected service is of wrong type when registered multiple times with generic registration - Microsoft.Extensions.DependecyInjection #79938

Closed schwarzr closed 1 year ago

schwarzr commented 1 year ago

Description

When registring a service in the IServiceCollection of asp.net core >= 6.0 once with a concrete type and once as a generic registration you get the wrong types of instances from the ServiceProvider.

var builder = WebApplication.CreateBuilder(args);

builder.Services.AddTransient<IProcessor<SampleEntity>, SampleEntityProcessor>();
builder.Services.AddTransient(typeof(IProcessor<>), typeof(GenericProcessor<>));

var app = builder.Build();

// two instances of type SampleEntityProcessor are produced;
var processors = app.Services.GetServices<IProcessor<SampleEntity>>();

app.Run();

Reproduction Steps

Create a new asp.net core 6 Project:

Program.cs

var builder = WebApplication.CreateBuilder(args);

builder.Services.AddTransient<IProcessor<SampleEntity>, SampleEntityProcessor>();
builder.Services.AddTransient(typeof(IProcessor<>), typeof(GenericProcessor<>));

var app = builder.Build();

// two instances of type SampleEntityProcessor are produced;
var processors = app.Services.GetServices<IProcessor<SampleEntity>>();

app.Run();

IProcessor

public interface IProcessor<T>
    where T : class
{
    Task ProcessAsync();
}

GenericProcessor

public class GenericProcessor<T> : IProcessor<T>
    where T : class
{
    public Task ProcessAsync()
    {
        return Task.CompletedTask;
    }
}

SampleEntity

public class SampleEntity
{
    public int Id { get; set; }

    public string Name { get; set; }
}

SampleEntityProcessor

public class SampleEntityProcessor : IProcessor<SampleEntity>
{
    public Task ProcessAsync()
    {
        return Task.CompletedTask;
    }
}

Expected behavior

var processors = app.Services.GetServices<IProcessor<SampleEntity>>();

should return

Actual behavior

var processors = app.Services.GetServices<IProcessor<SampleEntity>>();

currently returns

Screenshot 2022-12-23 184811

Regression?

Tested with:

Known Workarounds

No response

Configuration

Windows 11 x64 .net runtime 6.0.12 .net runtime 7.0.1 Visual Studio 2022

Other information

No response

dotnet-issue-labeler[bot] commented 1 year ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

schwarzr commented 1 year ago

area-Extensions-DependencyInjection

ghost commented 1 year ago

Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection See info in area-owners.md if you want to be subscribed.

Issue Details
### Description When registring a service in the IServiceCollection of asp.net core >= 6.0 once with a concrete type and once as a generic registration you get the wrong types of instances from the ServiceProvider. ```CSharp var builder = WebApplication.CreateBuilder(args); builder.Services.AddTransient, SampleEntityProcessor>(); builder.Services.AddTransient(typeof(IProcessor<>), typeof(GenericProcessor<>)); var app = builder.Build(); // two instances of type SampleEntityProcessor are produced; var processors = app.Services.GetServices>(); app.Run(); ``` ### Reproduction Steps Create a new asp.net core 6 Project: ## Program.cs ```CSharp var builder = WebApplication.CreateBuilder(args); builder.Services.AddTransient, SampleEntityProcessor>(); builder.Services.AddTransient(typeof(IProcessor<>), typeof(GenericProcessor<>)); var app = builder.Build(); // two instances of type SampleEntityProcessor are produced; var processors = app.Services.GetServices>(); app.Run(); ``` ## IProcessor ```CSharp public interface IProcessor where T : class { Task ProcessAsync(); } ``` ## GenericProcessor ```CSharp public class GenericProcessor : IProcessor where T : class { public Task ProcessAsync() { return Task.CompletedTask; } } ``` ## SampleEntity ```CSharp public class SampleEntity { public int Id { get; set; } public string Name { get; set; } } ``` ## SampleEntityProcessor ```CSharp public class SampleEntityProcessor : IProcessor { public Task ProcessAsync() { return Task.CompletedTask; } } ``` ### Expected behavior ```CSharp var processors = app.Services.GetServices>(); ``` should return - instance of SampleEntityProcessor - instance of GenericProcessor ### Actual behavior ```CSharp var processors = app.Services.GetServices>(); ``` currently returns - instance of SampleEntityProcessor - instance of SampleEntityProcessor ![Screenshot 2022-12-23 184811](https://user-images.githubusercontent.com/12955666/209382005-f2feaa8a-fcd2-419e-93ff-4bb67d0e81bd.png) ### Regression? Tested with: - asp.net 3.1 -> OK - asp.net 5 -> OK - asp.net 6 -> Failed (6.0.12) - asp.net 7 -> Failed (7.0.1) ### Known Workarounds _No response_ ### Configuration Windows 11 x64 .net runtime 6.0.12 .net runtime 7.0.1 Visual Studio 2022 ### Other information _No response_
Author: schwarzr
Assignees: -
Labels: `untriaged`, `area-Extensions-DependencyInjection`
Milestone: -
madelson commented 1 year ago

I'd be interested in working on this.

It would be helpful to confirm my understanding of the precise desired behavior in cases like this, though. Trying to piece together the intent from the code:

Is that all correct?

davidfowl commented 1 year ago

I believe it's the same issue as this https://github.com/dotnet/runtime/pull/68053. We should confirm though, it only happens when the environment is development right?

madelson commented 1 year ago

I believe it's the same issue as this https://github.com/dotnet/runtime/pull/68053

@davidfowl that seems right; potentially the fix there did not fully resolve the problem.

I believe it's the same issue as this https://github.com/dotnet/runtime/pull/68053. We should confirm though, it only happens when the environment is development right?

@schwarzr should confirm, but from my repro the code shown requires development environment because in that setup it is being triggered by ValidateOnBuild = true in the ServiceProviderOptions.

However, ValidateOnBuild is not the only way to trigger this. For example the following also repros for me:

void Main()
{
    var sc = new ServiceCollection();
    sc.AddTransient<IProcessor<string>, SampleProcessor>();
    sc.AddTransient(typeof(IProcessor<>), typeof(GenericProcessor<>));

        // not using build validation
        using var s = sc.BuildServiceProvider(new ServiceProviderOptions { ValidateOnBuild = false });
        s.GetService(typeof(IProcessor<string>)); // resolve this first: gives SampleProcessor
    s.GetServices<IProcessor<string>>(); // now this gives [SampleProcessor, SampleProcessor]
}

public interface IProcessor<T> { }

public class SampleProcessor : IProcessor<string> { }

public class GenericProcessor<T> : IProcessor<T> { }
davidfowl commented 1 year ago

However, ValidateOnBuild is not the only way to trigger this. For example the following also repros for me:

That raises the priority of this. This was assumed to only be broken in validation scenarios.

schwarzr commented 1 year ago

@madelson

@schwarzr should confirm, but from my repro the code shown requires development environment because in that setup it is being triggered by ValidateOnBuild = true in the ServiceProviderOptions.

I have been able to reproduce this issue outside Visual Studio and without ValidateOnBuild = true. So no, i don't think it is limited do the development environment.

@davidfowl One thing i recognized when i investigated this issue is, that if you register the generic version twice the resulting instances are: [SampleProcessor, GenericProcessor, SampleProcessor]

void Main()
{
    var sc = new ServiceCollection();
    sc.AddTransient<IProcessor<string>, SampleProcessor>();
    sc.AddTransient(typeof(IProcessor<>), typeof(GenericProcessor<>));
    sc.AddTransient(typeof(IProcessor<>), typeof(GenericProcessor<>));

        // not using build validation
        using var s = sc.BuildServiceProvider(new ServiceProviderOptions { ValidateOnBuild = false });
        s.GetService(typeof(IProcessor<string>)); // resolve this first: gives SampleProcessor
    s.GetServices<IProcessor<string>>(); // now this gives [SampleProcessor, GenericProcessor, SampleProcessor]
}

public interface IProcessor<T> { }

public class SampleProcessor : IProcessor<string> { }

public class GenericProcessor<T> : IProcessor<T> { }

and if you register the generic service first, it kind of works as expected.

void Main()
{
    var sc = new ServiceCollection();
    sc.AddTransient(typeof(IProcessor<>), typeof(GenericProcessor<>));
    sc.AddTransient<IProcessor<string>, SampleProcessor>();

        // not using build validation
        using var s = sc.BuildServiceProvider(new ServiceProviderOptions { ValidateOnBuild = false });
        s.GetService(typeof(IProcessor<string>)); // resolve this first: gives SampleProcessor
    s.GetServices<IProcessor<string>>(); // now this gives [GenericProcessor, SampleProcessor]
}

public interface IProcessor<T> { }

public class SampleProcessor : IProcessor<string> { }

public class GenericProcessor<T> : IProcessor<T> { }
steveharter commented 1 year ago

Are there any requests to backport the fix to v7 and possible v6 (since it's LTS)?