Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible Issue With Dependency Injection Container Service Resolution #91366

Closed
ConnerPhillis opened this issue Aug 31, 2023 · 7 comments
Closed

Comments

@ConnerPhillis
Copy link

ConnerPhillis commented Aug 31, 2023

Description

I've been using MediatR lately in some of my ASP.NET Core solutions, and it seems that I've stumbled upon an issue with type resolution with the dependency injection container. It doesn't seem to be correctly resolving types with constrained generics.

Furthermore, it seems that behavior is changing depending on whether ValidateOnBuild is set to true in BuildServiceProvider

I'm struggling to correctly explain all of this, so hopefully my reproduction steps should give enough details as to what's going wrong. I'm also not sure if this is expected behavior or not.

Apologies if my terminology in the following is incorrect, my generics vocabulary isn't the best :)

Reproduction Steps

Relevant code:

using MediatR;

using Microsoft.Extensions.DependencyInjection;

var serviceCollection = new ServiceCollection();

serviceCollection.AddTransient(
	typeof(INotificationHandler<RandomNotification>),
	typeof(RandomClosedGenericNotificationHandler));
serviceCollection.AddTransient(
	typeof(INotificationHandler<>),
	typeof(RandomOpenGenericNotificationHandler<>));

var builtService = serviceCollection.BuildServiceProvider(new ServiceProviderOptions { ValidateOnBuild = true});

var scp = builtService.CreateScope();

var services = scp.ServiceProvider.GetServices(typeof(INotificationHandler<RandomNotification>))
	.ToList();

Console.WriteLine("done, " + services.Count + " service(s)");

internal class OtherRandomNotification : INotification
{
}

public class RandomNotification : INotification
{
}

internal class RandomClosedGenericNotificationHandler : INotificationHandler<RandomNotification>
{
	public Task Handle(RandomNotification notification, CancellationToken cancellationToken)
		=> throw new NotImplementedException();
}

internal class RandomOpenGenericNotificationHandler<TNotification> : INotificationHandler<TNotification>
	where TNotification : OtherRandomNotification
{
	public Task Handle(TNotification notification, CancellationToken cancellationToken)
		=> throw new NotImplementedException();
}
  1. Create a new .NET 7 console project
  2. Download MediatR 12.1.1 from NuGet (It simply provides the interfaces / constraints, I believe this would also work without MediatR)
  3. Paste the above code into the Program file that is created
  4. Execute the code, notice that it outputs that there are two services resolved. If you dig into the services, you'll find that the services that were constructed are both of the same type, they will be instances of the RandomClosedGenericNotificationHandler, there will not be an instance of RandomOpenGenericNotificationHandler
  5. Change ValidateOnBuild = true to false. You should see that only one service is instantiated this time.

Furthermore, it only appears to generate one service if you change the open generic type to be registered first

I can understand why this would fail, the two open generics seem to not fit the service type. What I can't understand is why it would generate two services of the same type from it.

Expected behavior

I'm not 100% sure what the expected behavior should be, so feel free to correct this if neccessary

  1. When ValidateOnBuild is true, the dependency injection container detects that the open generic doesn't fit.
  2. Behavior is consistent depending on the registration order of the types
  3. Service is not provided twice whenever the above is registered in the provided order

Actual behavior

Service is generated once or twice depending on order of adding and service registration

Regression?

unknown

Known Workarounds

No response

Configuration

.NET 7
Windows 11,
X64,
Doesn't appear to be

Other information

I plan on going over to MediatR after this and attempting to fix the type registrations so they respect the constraints placed on the generics. Guessing that fixing it will negate the impact of this, if it is truly a bug.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 31, 2023
@ghost
Copy link

ghost commented Aug 31, 2023

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

Issue Details

Description

I've been using MediatR lately in some of my ASP.NET Core solutions, and it seems that I've stumbled upon an issue with type resolution in ASP.NET Core. It doesn't seem to be correctly resolving types with open generics.

Furthermore, it seems that behavior is changing depending on whether ValidateOnBuild is set to true in BuildServiceProvider

I'm struggling to correctly explain all of this, so hopefully my reproduction steps should give enough details as to what's going wrong. I'm also not sure if this is expected behavior or not.

Apologies if my terminology in the following is incorrect, my generics vocabulary isn't the best :)

Reproduction Steps

Relevant code:

using MediatR;

using Microsoft.Extensions.DependencyInjection;

var serviceCollection = new ServiceCollection();

serviceCollection.AddTransient(
	typeof(INotificationHandler<RandomNotification>),
	typeof(RandomClosedGenericNotificationHandler));
serviceCollection.AddTransient(
	typeof(INotificationHandler<>),
	typeof(RandomOpenGenericNotificationHandler<>));

var builtService = serviceCollection.BuildServiceProvider(new ServiceProviderOptions { ValidateOnBuild = true});

var scp = builtService.CreateScope();

var services = scp.ServiceProvider.GetServices(typeof(INotificationHandler<RandomNotification>))
	.ToList();

Console.WriteLine("done, " + services.Count + " service(s)");

internal class OtherRandomNotification : INotification
{
}

public class RandomNotification : INotification
{
}

internal class RandomClosedGenericNotificationHandler : INotificationHandler<RandomNotification>
{
	public Task Handle(RandomNotification notification, CancellationToken cancellationToken)
		=> throw new NotImplementedException();
}

internal class RandomOpenGenericNotificationHandler<TNotification> : INotificationHandler<TNotification>
	where TNotification : OtherRandomNotification
{
	public Task Handle(TNotification notification, CancellationToken cancellationToken)
		=> throw new NotImplementedException();
}
  1. Create a new .NET 7 console project
  2. Download MediatR 12.1.1 from NuGet (It simply provides the services, I believe this would also work in non-MediatR interfaces)
  3. Paste the above code into the Program file that is created
  4. Execute the code, notice that it outputs that there are two services resolved. If you dig into the services, you'll find that the services that were constructed are both of the same type, they will be instances of the RandomClosedGenericNotificationHandler, there will not be an instance of RandomOpenGenericNotificationHandler
  5. Change ValidateOnBuild = true to false. You should see that only one service is instantiated this time.

Furthermore, it only appears to generate one service if you change the open generic type to be registered first

I can understand why this would fail, by all accounts, the two open generics are being registered wrong. The implementation type does not fit in the service type. What I can't understand is why it would generate two services of the same type from it.

Expected behavior

I'm not 100% sure what the expected behavior should be, so feel free to correct this if neccessary

  1. When ValidateOnBuild is true, the dependency injection container detects that the open generic doesn't fit.
  2. Behavior is consistent depending on the registration order of the types
  3. Service is not provided twice whenever the above is registered in the provided order

Actual behavior

Service is generated once or twice depending on order of adding and service registration

Regression?

unknown

Known Workarounds

No response

Configuration

.NET 7
Windows 11,
X64,
Doesn't appear to be

Other information

I plan on going over to MediatR after this and attempting to fix the type registrations so they respect the constraints placed on the generics. Guessing that fixing it will negate the impact of this, if it is truly a bug.

Author: ConnerPhillis
Assignees: -
Labels:

area-Extensions-DependencyInjection

Milestone: -

@Sebazzz
Copy link

Sebazzz commented Sep 1, 2023

Thank you. This is exactly the issue I hit with MediatR and Microsoft.Extensions.DependencyInjection as well. When an open-generic type is involved, two of the closed services are resolved and the open type is not.

However, I also have it without the type constraints. I noticed this issue actually when switching from IWebHostBuilder to IHostBuilder, which strangely enough shouldn't affect this.

@Sebazzz
Copy link

Sebazzz commented Sep 1, 2023

Related: jbogard/MediatR/issues/884

@MichaelHochriegl
Copy link

There is already a fix in .Net 8 (original issue can be found here #87017 )
As stated in the linked issue above a backport of this fix will be considered once the fix in .Net 8 is battle-tested.

@steveharter
Copy link
Member

Closing as duplicate.

@ConnerPhillis can you verify v8 works for you? Thanks

@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Sep 1, 2023
@ConnerPhillis
Copy link
Author

@steveharter I can but it won't be on Monday or Tuesday. I'm currently traveling and won't be back until then.

Hopefully that'll fix it, I was worried I was crazy.

@Sebazzz
Copy link

Sebazzz commented Sep 1, 2023

The .NET 8 Preview 7 package worked for me, and works fine for me in an otherwise .NET 7 solution.

@ghost ghost locked as resolved and limited conversation to collaborators Oct 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants