autofac / Autofac

An addictive .NET IoC container
https://autofac.org
MIT License
4.47k stars 836 forks source link

Behavior change with registration sources after container is built in version 6 #1225

Closed hvaring closed 10 months ago

hvaring commented 3 years ago

Describe the Bug

In version 5.1.2 and prior, I was able to dynamically add registrations to a built container using a registration source. The behavior seems to have changed in v6, as the new registrations cannot be resolved if that specific registration was resolved prior to adding the registration source.

However, if I haven't resolved the type prior to adding the registration source, it will work.

Not sure if this is a bug or if intended (or if I'm doing something wrong), so any advice would be appreciated. I'm using the version appropriate ExternalRegistrySource from the Autofac repo.

Steps to Reproduce

public class ReproTest
{
    [Fact]
    public void Repro()
    {
        var builder = new ContainerBuilder();
        var container = builder.Build();

        Assert.False(container.IsRegistered<ITest>()); // comment this out in v6.0 and the second assert works.
        AddRegistrations(builder.ComponentRegistryBuilder);
        Assert.True(container.IsRegistered<ITest>()); // returns false in v6.0 if ITest resolved prior to AddRegistrations is called.
    }

    private static void AddRegistrations(IComponentRegistryBuilder registryBuilder)
    {
        var builder = new ContainerBuilder();
        builder.RegisterType<Test>().As<ITest>();
        var container = builder.Build();
        registryBuilder.AddRegistrationSource(new ExternalRegistrySource(container.ComponentRegistry)); // ExternalRegistrySource is the one from the Autofac repo.
    }

    private interface ITest { }
    private class Test : ITest { }
}

Expected Behavior

Expected registrations added through registration sources to be made available even if the type has been resolved prior.

Dependency Versions

Autofac: 5.1.2 and 6.0.0

alistairjevans commented 3 years ago

It appears to me as though you were circumventing an accidental loophole in our "Containers are Immutable" rule, that appears to have been closed as of 6.0.

Internally, the first time you resolve a service from a lifetime scope, we determine and then freeze the set of all implementations of that service. So we won't go looking at registration sources after the first resolve.

Have you considered doing your additional registrations inside a nested scope instead? Something like:

public class ReproTest
{
    [Fact]
    public void Repro()
    {
        var builder = new ContainerBuilder();
        var container = builder.Build();

        Assert.False(container.IsRegistered<ITest>());
        var extendedScope = GetExtendedScope(container);
        Assert.True(extendedScope.IsRegistered<ITest>());
    }

    private static ILifetimeScope GetExtendedScope(IContainer container)
    {
        return container.BeginLifetimeScope(builder => 
        {
            builder.RegisterType<Test>().As<ITest>();
        });
    }

    private interface ITest { }
    private class Test : ITest { }
}
hvaring commented 3 years ago

Thanks for the response.

It appears to me as though you were circumventing an accidental loophole in our "Containers are Immutable" rule, that appears to have been closed as of 6.0.

That was exactly the intention. The use case are modules that can load dynamically during run-time, and one of the suggested solutions on the discussion issue was using registration sources.

Have you considered doing your additional registrations inside a nested scope instead?

This was actually my first approach, but then you would run into potential problems with singletons, even when resolved from the child scope. This particular issue was solved by using a registration source.

public class ReproTest
{
    [Fact]
    public void Repro()
    {
        var builder = new ContainerBuilder();
        builder.RegisterType<Singleton>().SingleInstance();
        var container = builder.Build();

        Assert.False(container.IsRegistered<ITest>());
        var extendedScope = GetExtendedScope(container);
        Assert.True(extendedScope.IsRegistered<ITest>());
        Assert.True(extendedScope.Resolve<Singleton>().Implementations.Count() == 1); // fails
    }

    private static ILifetimeScope GetExtendedScope(IContainer container)
    {
        return container.BeginLifetimeScope(builder =>
        {
            builder.RegisterType<Test>().As<ITest>();
        });
    }

    public interface ITest { }
    public class Test : ITest { }

    public class Singleton
    {
        public IEnumerable<ITest> Implementations { get; }
        public Singleton(IEnumerable<ITest> implementations)
        {
            Implementations = implementations;
        }
    }
}

Are there any tools available to achieve the same behavior in v6.0.0 as in v5xx?

alistairjevans commented 3 years ago

The singleton behaviour is expected and makes sense if you think about it; you can't have components in an outer scope that depend on services only defined inside an inner scope.

@hvaring, does the initial outer container have to be functional? I.e. will someone resolve from that container, as well as the nested scope?

hvaring commented 3 years ago

The singleton behaviour is expected and makes sense if you think about it; you can't have components in an outer scope that depend on services only defined inside an inner scope.

Absolutely, I understand this.

@hvaring, does the initial outer container have to be functional? I.e. will someone resolve from that container, as well as the nested scope?

Unfortunately yes, since modules can be loaded on-demand during runtime. The nested scope would "take over" once the module is loaded, and every subsequent module would result in another child scope from the previous child scope.

alistairjevans commented 3 years ago

I'm afraid I can't think of a way to make this work precisely the way you need it. Lifetime Scopes are the way to go for this sort of functionality, but singletons in an outer scope can't take dependencies from an inner scope.

The closest mechanism I can imagine is to define all your outer-most dependencies in an Autofac Module, then re-register them in the lifetime scope you start for each module. That will override the existing registrations from the outer scope, and allow Singletons to resolve enumerations of components from inside the scope. But that means you will have different singleton instances for each one.

tillig commented 3 years ago

Random thinking aloud:

We cache the list of service activators but for lambdas we can't actually cache the results, just the lambda. I wonder if there's a way to exploit that to allow something more dynamic. I can't provide a specific example or anything, and maybe this wouldn't even work because you'd have to be able to contribute an IEnumerable<T> to the set of things that get resolved by IEnumerable<T> relationships and that's a whole ball of wax.

I strongly believe in the immutable container mechanism but I wonder if there's a use case here for opting in to letting registration sources be queried on every resolve at the cost of performance. Like, "I know this is going to tank me, but do it anyway." Probably not because...

The complexity around dynamically loaded modules at runtime is actually one of the biggest reasons we went to immutable containers. Singleton A resolves an IEnumerable<IMessageHandler> and hangs onto them. Later, Singleton B resolves an IEnumerable<IMessageHandler> and hangs on there, but it's a different set of message handlers because dynamic magic has happened. Now there's a mismatch of how things are being handled because lifetime scopes have probably been configured incorrectly to allow that captive dependency to happen and the whole application state is somewhat invalid.

Back in November 2016 when that "ContainerBuilder.Update is obsolete" discussion issue started we were on Autofac v4.2.1 and the internals of Autofac were different. We're now two full major releases past that, four years later, and... it may be that we just can't support dynamic registrations like this anymore. The use cases for it are pretty limited, and while I absolutely respect that there's a non-zero population of folks who want such a thing, the cost of ownership and support for that feature doesn't fall on the consumers.

tillig commented 10 months ago

This issue has been open for quite some time without any traction. Much as we understand the challenges presented here, we also get very few substantial contributions and we don't have time to really dig into non-trivial issues like this. As such, we're closing this issue. If someone feels like they really want this, we would love to see a PR where this gets addressed.