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

GetServiceOrCreateInstance attempts to call wrong constructor when no types registered in D.I. #45119

Closed
ajcvickers opened this issue Nov 23, 2020 · 2 comments

Comments

@ajcvickers
Copy link
Member

Original issue: dotnet/efcore#23306
/cc @bricelam @davidfowl

Using these types:

public class FailureContext : DbContext
{
    public FailureContext(ILogger<FailureContext> logger)
    {
        Console.WriteLine($"With logger {logger}");
    }

    public FailureContext()
    {
        Console.WriteLine("Parameterless");
    }
}

This works for both GetService<FailureContext> and ActivatorUtilities.GetServiceOrCreateInstance<FailureContext> when both these types are in D.I.:

        var serviceProvider1 = new ServiceCollection()
            .AddSingleton<ILogger<FailureContext>, MyLogger>()
            .AddSingleton<FailureContext>()
            .BuildServiceProvider();

        var context1 = serviceProvider1.GetService<FailureContext>();

        var serviceProvider2 = new ServiceCollection()
            .AddSingleton<ILogger<FailureContext>, MyLogger>()
            .AddSingleton<FailureContext>()
            .BuildServiceProvider();

        var context2 = ActivatorUtilities.GetServiceOrCreateInstance<FailureContext>(serviceProvider2);

Output:

With logger MyLogger
With logger MyLogger

It also works if only DbContext is registered in D.I.:

        var serviceProvider1 = new ServiceCollection()
            //.AddSingleton<ILogger<FailureContext>, MyLogger>()
            .AddSingleton<FailureContext>()
            .BuildServiceProvider();

        var context1 = serviceProvider1.GetService<FailureContext>();

        var serviceProvider2 = new ServiceCollection()
            //.AddSingleton<ILogger<FailureContext>, MyLogger>()
            .AddSingleton<FailureContext>()
            .BuildServiceProvider();

        var context2 = ActivatorUtilities.GetServiceOrCreateInstance<FailureContext>(serviceProvider2);

In this case the output shows that the parameterless constructor is used:

Parameterless
Parameterless

It also works if FailureContext is not registered in D.I., but the logger is:

        var serviceProvider1 = new ServiceCollection()
            .AddSingleton<ILogger<FailureContext>, MyLogger>()
            //.AddSingleton<FailureContext>()
            .BuildServiceProvider();

        var context1 = serviceProvider1.GetService<FailureContext>();

        var serviceProvider2 = new ServiceCollection()
            .AddSingleton<ILogger<FailureContext>, MyLogger>()
            //.AddSingleton<FailureContext>()
            .BuildServiceProvider();

        var context2 = ActivatorUtilities.GetServiceOrCreateInstance<FailureContext>(serviceProvider2);

In this case GetService returns null, but GetServiceOrCreateInstance still chooses the correct constructor.

With logger MyLogger

When this fails is when neither the logger or the logger are registered in D..I:

        var serviceProvider1 = new ServiceCollection()
            //.AddSingleton<ILogger<FailureContext>, MyLogger>()
            //.AddSingleton<FailureContext>()
            .BuildServiceProvider();

        var context1 = serviceProvider1.GetService<FailureContext>();

        var serviceProvider2 = new ServiceCollection()
            //.AddSingleton<ILogger<FailureContext>, MyLogger>()
            //.AddSingleton<FailureContext>()
            .BuildServiceProvider();

        var context2 = ActivatorUtilities.GetServiceOrCreateInstance<FailureContext>(serviceProvider2);
Unhandled exception. System.InvalidOperationException: Unable to resolve service for type 'Microsoft.Extensions.Logging.ILogger`1[FailureContext]' while attempting to activate 'FailureContext'.
   at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.ConstructorMatcher.CreateInstance(IServiceProvider provider)
   at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.CreateInstance(IServiceProvider provider, Type instanceType, Object[] parameters)
   at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.GetServiceOrCreateInstance(IServiceProvider provider, Type type)
   at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.GetServiceOrCreateInstance[T](IServiceProvider provider)
   at Program.Main(String[] args) in /home/ajcvickers/AllTogetherNow/FiveOh/Program.cs:line 155

In this case, GetServiceOrCreateInstance should choose the parameterless constructor since there is no other constructor that can work, but it it instead fails by attempting to call a constructor for which the parameter cannot be resolved.

@ghost
Copy link

ghost commented Nov 23, 2020

Tagging subscribers to this area: @eerhardt, @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

Original issue: dotnet/efcore#23306
/cc @bricelam @davidfowl

Using these types:

public class FailureContext : DbContext
{
    public FailureContext(ILogger<FailureContext> logger)
    {
        Console.WriteLine($"With logger {logger}");
    }

    public FailureContext()
    {
        Console.WriteLine("Parameterless");
    }
}

This works for both GetService<FailureContext> and ActivatorUtilities.GetServiceOrCreateInstance<FailureContext> when both these types are in D.I.:

        var serviceProvider1 = new ServiceCollection()
            .AddSingleton<ILogger<FailureContext>, MyLogger>()
            .AddSingleton<FailureContext>()
            .BuildServiceProvider();

        var context1 = serviceProvider1.GetService<FailureContext>();

        var serviceProvider2 = new ServiceCollection()
            .AddSingleton<ILogger<FailureContext>, MyLogger>()
            .AddSingleton<FailureContext>()
            .BuildServiceProvider();

        var context2 = ActivatorUtilities.GetServiceOrCreateInstance<FailureContext>(serviceProvider2);

Output:

With logger MyLogger
With logger MyLogger

It also works if only DbContext is registered in D.I.:

        var serviceProvider1 = new ServiceCollection()
            //.AddSingleton<ILogger<FailureContext>, MyLogger>()
            .AddSingleton<FailureContext>()
            .BuildServiceProvider();

        var context1 = serviceProvider1.GetService<FailureContext>();

        var serviceProvider2 = new ServiceCollection()
            //.AddSingleton<ILogger<FailureContext>, MyLogger>()
            .AddSingleton<FailureContext>()
            .BuildServiceProvider();

        var context2 = ActivatorUtilities.GetServiceOrCreateInstance<FailureContext>(serviceProvider2);

In this case the output shows that the parameterless constructor is used:

Parameterless
Parameterless

It also works if FailureContext is not registered in D.I., but the logger is:

        var serviceProvider1 = new ServiceCollection()
            .AddSingleton<ILogger<FailureContext>, MyLogger>()
            //.AddSingleton<FailureContext>()
            .BuildServiceProvider();

        var context1 = serviceProvider1.GetService<FailureContext>();

        var serviceProvider2 = new ServiceCollection()
            .AddSingleton<ILogger<FailureContext>, MyLogger>()
            //.AddSingleton<FailureContext>()
            .BuildServiceProvider();

        var context2 = ActivatorUtilities.GetServiceOrCreateInstance<FailureContext>(serviceProvider2);

In this case GetService returns null, but GetServiceOrCreateInstance still chooses the correct constructor.

With logger MyLogger

When this fails is when neither the logger or the logger are registered in D..I:

        var serviceProvider1 = new ServiceCollection()
            //.AddSingleton<ILogger<FailureContext>, MyLogger>()
            //.AddSingleton<FailureContext>()
            .BuildServiceProvider();

        var context1 = serviceProvider1.GetService<FailureContext>();

        var serviceProvider2 = new ServiceCollection()
            //.AddSingleton<ILogger<FailureContext>, MyLogger>()
            //.AddSingleton<FailureContext>()
            .BuildServiceProvider();

        var context2 = ActivatorUtilities.GetServiceOrCreateInstance<FailureContext>(serviceProvider2);
Unhandled exception. System.InvalidOperationException: Unable to resolve service for type 'Microsoft.Extensions.Logging.ILogger`1[FailureContext]' while attempting to activate 'FailureContext'.
   at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.ConstructorMatcher.CreateInstance(IServiceProvider provider)
   at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.CreateInstance(IServiceProvider provider, Type instanceType, Object[] parameters)
   at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.GetServiceOrCreateInstance(IServiceProvider provider, Type type)
   at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.GetServiceOrCreateInstance[T](IServiceProvider provider)
   at Program.Main(String[] args) in /home/ajcvickers/AllTogetherNow/FiveOh/Program.cs:line 155

In this case, GetServiceOrCreateInstance should choose the parameterless constructor since there is no other constructor that can work, but it it instead fails by attempting to call a constructor for which the parameter cannot be resolved.

Author: ajcvickers
Assignees: -
Labels:

area-Extensions-DependencyInjection

Milestone: -

@maryamariyan
Copy link
Member

maryamariyan commented Mar 29, 2021

Closing as dupe of #46132 since there is more discussion there

ML, Extensions, Globalization, etc, POD. automation moved this from Future to Done Mar 29, 2021
lawrence-laz added a commit to lawrence-laz/runtime that referenced this issue Apr 3, 2021
Calling `ActivatorUtilities.CreateInstance` without additional arguments
should prefer parameterless constructor as there are no guarantees that
parameters, for which arguments were not supplied, can be resolved from
the ServiceProvider.
lawrence-laz added a commit to lawrence-laz/runtime that referenced this issue Apr 3, 2021
Previously `ActivatorUtilities.CreateInstance` and
`ActivatorUtilities.CreateFactory` behaved differently: former depended
on ctor definition order to disambiguate ctors, while latter failed
altogether.

The behavior is now unified and addresses concerns raised in dotnet#45119
 dotnet#42339 and dotnet#46132. Constructors are chosen based on the following
factors in a given order:
1) Preferred ctor (having [ActivatorUtilitiesConstructor] attribute)
2) Ctor with the most surjective parameters based on the given arguments.
   In other words constructor that has the least unresolved parameters
   is chosen.
3) Ctor with more resolved parameters is prefered. If two ctors have all
   of the supplied arguments, but one of them has additional parameters
   with default values, then the one with more parameters is chosen.
@ghost ghost locked as resolved and limited conversation to collaborators Apr 29, 2021
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