Open generic registered as SingleInstance in child lifetime scope fails to resolve

[tillig]

Based on the issue autofac/Autofac.Extensions.DependencyInjection#5:

Registering a standard typed service as a singleton in a child lifetime scope correctly allows you to resolve the singleton from within that lifetime scope. However, If you register an open generic in a child lifetime scope as a single instance and it depends on the newly registered singleton it fails:

var c = new ContainerBuilder().Build();
using (var s = c.BeginLifetimeScope(b =>
      {
        b.RegisterType<LoggerFactory>().As<ILoggerFactory>().SingleInstance();

        // Logger<T> takes an ILoggerFactory in as a constructor parameter
        b.RegisterGeneric(typeof(Logger<>)).As(typeof(ILogger<>)).SingleInstance();
      }))
{
  // This will succeed
  s.Resolve<ILoggerFactory>();

  // This blows up with an exception saying it can't resolve the ILoggerFactory type.
  s.Resolve<ILogger<SomeObject>>();
}

The SingleInstance() registration on the open generic has something to do with it. If you remove it, this works fine:

var c = new ContainerBuilder().Build();
using (var s = c.BeginLifetimeScope(b =>
      {
        b.RegisterType<LoggerFactory>().As<ILoggerFactory>().SingleInstance();

        // No longer SingleInstance - works fine.
        b.RegisterGeneric(typeof(Logger<>)).As(typeof(ILogger<>));
      }))
{
  // These both succeed
  s.Resolve<ILoggerFactory>();
  s.Resolve<ILogger<SomeObject>>();
}

I'm guessing there's something about the way the OpenGenericRegistrationSource determines what parameters it can fulfill that doesn't mesh up with the scenario here.

[AndyPook]

Just a friendly bump :)

What are the chances that someone (with the knowledge to understand the nuances) might look at this?

Trying to do a thing with AspNetCore in Azure Service Fabric that relies on the Startup using a child scope rather than a "root" Container (as per the AutofacServiceProviderFactory approach). The fact that a child scope behaves differently from a Container is a little disturbing.

Can someone at least give me some confidence that changing lifetime (on non-instance registrations) to InstancePerLifetimeScope is a "good" workaround?

Thanks.

[tillig]

There are two project owners for core Autofac and the 20+ integration packages, neither of whom have Autofac maintenance as their day job. Unfortunately, that means we have to prioritize and this isn't at the top right now. When it is, we'll comment back, but until then, things that can help speed this along:

• Debug into Autofac to see if you can locate the issue.
• Try out InstancePerLifetimeScope and see for yourself if that fixes it. Run tests for memory usage to see if it leaks.
• Submit a PR of you figure out the issue.

[AndyPook]

Understood, we all need to pay the bills :)

I've been having a poke at it in an attempt to come up with a PR...
Some extra info...

The disturbing thing is that if the parts are registered in a "root" container (rather than a "child" scope with BeginLifetimeScope) then the Resolve works fine.
So the behavior of registrations between root and child is different. I'm guessing that there's some interaction between CreateScopeRestrictedRegistry and the dependents lifestyle. (Still fairly new to autofac, still trying to grok the internals/semantics)

The odd thing (to me) is that it is the lifestyle of the dependent (ILogger) that determines if the dependency (ILoggerFactory) can be resolved.

I'll leave updates here as I find them.

[AndyPook]

OK, I think I have a theory...

I think the problem is one of semantics/expectations ie Singleton doesn't mean what we (well I) think it means. Let me see if I can explain.

Q: What does Singleton mean in the context of a child scope anyway?
If there should only be one - in this scope - then you should use InstancePerLifetimeScope anyway.

When you register with SingleInstance the registration is given a RootScopeLifetime. What this seems to mean is that, even though the registration is in some child scope, it is the root (ultimate parent) scope which is used as the activation context for resolving dependencies (see InstanceLookup:55).

SingleInstance seems to mean, there will be only one instance in this scope, but any dependencies must be resolvable from the root container.

So, of course, our test case above fails. It's trying to find the dependency (ILoggerFactory) in the root container. But it's registered in the child. So it can't resolve it => fail.

This scope mismatch seems counter-intuitive to me. But it's all about the nuances, right? :)
Can someone far more in tune with autofac share some understanding?

Could you also comment on the following...

Q: If you only have a root container, is there any real functional/behavioral difference between SingleInstance and InstancePerLifetimeScope?

Q: if you can resolve a SingleInstance from a child scope. Is it disposed/removed when the scope is disposed? Or is it "held" by the root scope?

Q: If you have a child scope, the only difference with using InstancePerLifetimeScope is that dependencies will be resolved from the current scope first then bubble up parents if not found.

Q: if my theory on how it behaves is correct, what would be the use case for SingleInstance?

Thank you for any time you can spend helping me understand

[alexmg]

The ScopeRestrictedRegistry was not applying the ComponentRegistrationLifetimeDecorator to registrations that came from an IRegistrationSource. The ScopeRestrictedRegistry is used for a child lifetime scope that needs to be modified with new registrations. It looks for registrations that have a lifetime of RootScopeLifetime and roots them to the new child lifetime scope instead. When using SingleInstance on a registration its lifetime is set to RootLifetimeScope, but when added to a child lifetime scope the notion of what the root is changes. Registrations that were added directly, and not via a registration source, were being wrapped with the ComponentRegistrationLifetimeDecorator. This explains why it worked for the ILoggerFactory registration but not the ILogger<> open generic registration in the reproduction above. The decorator is now applied to both kinds of registrations so all SingleInstance registrations added in a child lifetime scope are now correctly rooted.

[alexmg]

Thanks for taking the time to help out @AndyPook

Q: What does Singleton mean in the context of a child scope anyway?
If there should only be one - in this scope - then you should use InstancePerLifetimeScope anyway.

The singleton should be rooted to the child lifetime scope that it was defined in. Using InstancePerLifetimeScope would allow for a second instance to be created when a subsequent child lifetime scope was created.

When you register with SingleInstance the registration is given a RootScopeLifetime. What this seems to mean is that, even though the registration is in some child scope, it is the root (ultimate parent) scope which is used as the activation context for resolving dependencies (see InstanceLookup:55).

The registration is always given the RootScopeLifetime when SingleInstance is applied. The registration should have been decorated and given the child lifetime scope as its root. This was the bug and the result you were seeing wasn't correct.

Q: If you only have a root container, is there any real functional/behavioral difference between SingleInstance and InstancePerLifetimeScope?

Yes, most definitely. There will only be one SingleInstance in all lifetime scopes from the point that it was registered. With InstancePerLifetimeScope, there will be one instance per lifetime scope, so when a child lifetime scope is created a new instance will be resolved inside that.

Q: if you can resolve a SingleInstance from a child scope. Is it disposed/removed when the scope is disposed? Or is it "held" by the root scope?

It is held and disposed by the most root lifetime scope that it is defined in. With the fix in place you will see this is working as expected.

Q: If you have a child scope, the only difference with using InstancePerLifetimeScope is that dependencies will be resolved from the current scope first then bubble up parents if not found.

InstancePerLifetimeScope dependencies are always resolved from the current lifetime scope and an instance is created if one does not already exist. That instance will then be returned for all resolve operations in that lifetime scope.

Q: if my theory on how it behaves is correct, what would be the use case for SingleInstance?

Your theory was based on what you were seeing in the code, which was unfortunately incorrect, at least for this particular scenario involving a new child lifetime scope being updated with a singleton registration coming from a registration source. As you can see from my previous comments there is definitely a difference. Don't let this one very specific issue change your view on how things should work. 😃

[AndyPook]

Ahhh, that makes sense. Thanks for the full explanation, very helpful.
And a commit directly against master, nice :)

Q: given the work (the scope restrict register, rescoping etc) does this imply some guidance on perf? ie if you create a scope per request and are registering "a load of stuff" in the child scope. I haven't been able to grok what's happening well enough to gauge what the cost might be... thoughts?

I have a "solution" that "fixes" my problem by using InstancePerLifetimeScope which, given your explanation and the life cycle of my scope, will work fine.

Lastly and fully understanding @tillig 's initial point... do you have any theories, hopes about when this change might hit nuget?

Thanks again for taking the time

[sfmskywalker]

I ran into the exact same issue, and would have never figured it out until I came across this GitHub issue. Thanks everyone for the detailed record provided here, that was a tremendous help.

[alexmg]

This fix is included in 4.6.1 which has been pushed to NuGet.

[AndyPook]

very cool. Thanks for your efforts.

…

On 25 July 2017 at 14:00, Alex Meyer-Gleaves ***@***.***> wrote: This fix is included in 4.6.1 which has been pushed to NuGet. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#847 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAvrq6c68PyqjjMrOWoz4IkZiVGYdpOFks5sReb4gaJpZM4M_YZA> .

[astef]

Shouldn't Autofac.Extensions.DependencyInjection package also be updated?

[tillig]

@astef An update to a base package doesn't require all downstream packages be updated. If you want this update, just add a direct reference to Autofac and get the latest.