unitycontainer / microsoft-dependency-injection

Unity.Microsoft.DependencyInjection package
Apache License 2.0
63 stars 36 forks source link

HttpClientFactory not working #28

Closed pksorensen closed 5 years ago

pksorensen commented 5 years ago

I have not been able to create a repro for this yet, so this is just to gather information for now.

image

This is the ServiceProvider of a httpclientfactory dependency of a custom service constructor.

and then later when it actually is used the SP is null

image

Need to figure out why this can happen, because it will throw exceptions otherwise. Possible issue is that the sp for the httpclientfactory is disposed. (will update with i have more info)

pksorensen commented 5 years ago

https://github.com/unitycontainer/microsoft-dependency-injection/blob/2.1.1.105/src/ServiceProvider.cs#L83

will set the _container=null when disposed, so this is very likely the problem. Need to figure out why its disposing the serviceprovider

pksorensen commented 5 years ago

Reproduced it to the following unittest:

        [TestMethod]
        public void Test2_failing()
        {

            var serviceColection = new ServiceCollection();

            serviceColection.AddHttpClient();

            var container = new UnityContainer().CreateChildContainer();

            var factory = new ServiceProviderFactory(container);

            var sp = factory.CreateServiceProvider(serviceColection);
            var scopeFactory = sp.GetRequiredService<IServiceScopeFactory>();
            using (var scope = scopeFactory.CreateScope())
            {
                var httpFactory = scope.ServiceProvider.GetRequiredService<IHttpClientFactory>();

                var sp1 = httpFactory.GetType().GetField("_services", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)?.GetValue(httpFactory) as IServiceProvider;
                var c1 = sp1.GetType().GetField("_container", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(sp1);
                Assert.IsNotNull(c1,"In scope");

            }

            {
                var httpFactory = sp.GetRequiredService<IHttpClientFactory>();
                var sp1 = httpFactory.GetType().GetField("_services", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)?.GetValue(httpFactory) as IServiceProvider;
                var c1 = sp1.GetType().GetField("_container", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(sp1);
                Assert.IsNotNull(c1, "after scope");
            }

        }
ENikS commented 5 years ago

I do not think it is a bug. It behaves as designed:

IHttpClientFactory is a singleton but first resolved within the scope. As result it is provided with reference to scoped IServiceProvider (The one where it is being resolved).

When Service Provider goes out of scope it rightfully discards the container but is being retained by singleton instance of IHttpClientFactory

Next time IHttpClientFactory is requested it returns existing instance with invalid reference to long gone scope.

To fix the issue you need to resolve IHttpClientFactory in proper service provider:

var serviceCollection = new ServiceCollection();

serviceCollection.AddHttpClient();

IUnityContainer container = new UnityContainer().CreateChildContainer();

var factory = new ServiceProviderFactory(container);

var sp = factory.CreateServiceProvider(serviceCollection);
var scopeFactory = sp.GetRequiredService<IServiceScopeFactory>();
var httpFactory0 = sp.GetRequiredService<IHttpClientFactory>();
using (var scope = scopeFactory.CreateScope())
{
    var httpFactory = scope.ServiceProvider.GetRequiredService<IHttpClientFactory>();

    var sp1 = httpFactory.GetType().GetField("_services", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)?.GetValue(httpFactory) as IServiceProvider;
    var c1 = sp1.GetType().GetField("_container", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(sp1);
    Assert.NotNull(c1); // "In scope"

}

{
    var httpFactory = sp.GetRequiredService<IHttpClientFactory>();
    var sp1 = httpFactory.GetType().GetField("_services", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)?.GetValue(httpFactory) as IServiceProvider;
    var c1 = sp1.GetType().GetField("_container", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(sp1);
    Assert.NotNull(c1); // "after scope"
}
pksorensen commented 5 years ago

That was also what i ended up doing (resolving it in my application startup code).

That said, i do not think that is how build in DI is behaving in aspnet core. It works even though the first resolution happens in a scope.

And thats what people would expect when swapping to a 3th party container i assume.

ENikS commented 5 years ago

I do not think there is an easy way for Unity to address it. Unfortunately...

pksorensen commented 5 years ago

I agree.

One really just need to be an expert and have in dept knowledge to know these things, which takes time :)

Thanks