Opt-in to captive dependency detection on a registration

[bentefay]

My company is using Autofac extensively and really loving it.

That being said, one of the most difficult to debug issue we have run into with IoC containers is captive dependencies.

I understand that captive dependencies are complex, and there are cases where captive dependencies are acceptable. However, we have cases where a component absolutely must not become a captive dependency (becomes its behaviour would be broken). We would like to be able to annotate these components so it is impossible for us to accidentally widen their lifetime scope without being notified by an error message.

I am imagining something like a LifetimeScopeCannotBeWidened method that can be called when registering. The intention would be that, when this setting is applied to a registration, Autofac would throw an exception rather than inject the component into another component with a wider lifetime scope.

builder
  .RegisterType<Component>()
  .As<IService>()
  .InstancePerLifetimeScope()
  .LifetimeScopeCannotBeWidened();

Does that make sense? Do you think it would it be possible for us to extend Autofac to provide this functionality, or would this need to be built into Autofac?

Thanks for your help!

[tillig]

I think this is a great idea but I also think it will be... really hard to implement.

For example, let's say you have this registration:

builder.Register(ctx =>
  {
    var a = ctx.Resolve<IDependency1>();
    var b = ctx.Resolve<IDependency2>();
    return new Component(a, b);
  })
  .As<IComponent>();
  .SingleInstance()
  .LifetimeScopeCannotBeWidened();

In a case like that, how would Autofac detect the error? At resolution time there's not really any meta-information returned about the lifetime scope of things getting returned, so we'd never really know if IDependency1 or IDependency2 services were registered as single instances or not.

There are also other interesting challenges, like what happens if you have a singleton class that does this:

public class MySingleton
{
  public MySingleton(Func<IService> serviceFactory) { ... }

  public void DoWork()
  {
    var service = this._serviceFactory();
    service.DoSomethingCool();
  }
}

If this is registered like this:

builder.RegisterType<MySingleton>()
  .AsSelf()
  .SingleInstance()
  .LifetimeScopeCannotBeWidened();
builder.RegisterType<Component>()
  .As<IService>();

IService is instance per dependency and MySingleton is a singleton. But it gets a factory for IService injected that creates a new instance as needed. Is that a captive? Should it be detected as such? What if it's a Lazy<IService> instead of Func<IService>?

The point is... I'm not sure how we'd actually implement such a feature.

I'll leave this issue open since I agree the concept here would definitely be a cool feature. If something comes along or an idea pops up (or if someone has a brainstorm on how to make it happen), comments and thoughts are welcome.

If it hangs out here for a year or two then we might close it as "can't implement" or something. But for now, let's see what we can do.

[bentefay]

Thanks for taking the time to answer with so much detail @tillig. You make some really good points.

Just to be clear, I was imagining that LifetimeScopeCannotBeWidened would only prevent the annotated registration from having its scope widened. Take the example you gave:

builder.RegisterType<MySingleton>()
   .AsSelf()
   .SingleInstance()
   .LifetimeScopeCannotBeWidened();
builder.RegisterType<Component>()
   .As<IService>();

public class MySingleton
{
 public MySingleton(Func<IService> serviceFactory) { ... }

 public void DoWork()
 {
   var service = this._serviceFactory();
   service.DoSomethingCool();
 }
}

In this case MySingleton is capturing Component. I am suggestion that this would be legal, as MySingleton is not having its scope widened. Component is having its scope widened, but we have not annotated Component with LifetimeScopeCannotBeWidened.

However, if you changed the registration as follows:

builder.RegisterType<MySingleton>()
  .AsSelf()
  .SingleInstance()
builder.RegisterType<Component>()
  .As<IService>();
  .LifetimeScopeCannotBeWidened();

public class MySingleton
{
  public MySingleton(IService serviceFactory) { ... }
}

In this case, I am imagining that Autofac would throw an exception if you try to resolve MySingleton, because Component is now having its scope widened from InstancePerDependency to SingleInstance (it is being captured by MySingleton).

To answer your questions about injecting Func and Lazy and custom registrations, one approach would be to take a very strict approach: if Autofac cannot verify that the scope is not being widened at resolve time, it throws an exception. In other words, trying to inject Func and Lazy versions of a component marked with LifetimeScopeCannotBeWidened would throw exceptions. Similarly, trying to resolve a component marked with LifetimeScopeCannotBeWidened in a context where Autofac cannot determine the parent scope would result in an exception. Because I am suggesting LifetimeScopeCannotBeWidened is opt-in, there would be no harm in being overly strict. That seems like a reasonable minimum viable feature, which would meet our needs.

Even then, we probably could support custom registrations with a bit more work. For example:

builder.Register(ctx =>
  {
    var a = ctx.Resolve<IService>();
    return new Component(a, b);
  })
  .AsSelf()
  .SingleInstance();

builder.RegisterType<Service>()
  .As<IService>();
  .LifetimeScopeCannotBeWidened();

If client code tries to resolve Component, Autofac presumably knows, before it executes the custom factory method passed to Register, that the scope of Component is SingleInstance and could put this information on the System.Runtime.Remoting.Messaging.CallContext. When the factory method calls ctx.Resolve<IService>(), I assume Autofac could check that Service is annotated with LifetimeScopeCannotBeWidened. It could then retrieve the scope of IService (InstancePerDependency), and could retrieve the lifetime of Component (SingleInstance) off the CallContext. Given SingleInstance is a wider scope than InstancePerDependency, Autofac could thrown an exception.

Does that all make sense? I am guessing here about the information that Autofac has available to it at resolve-time.

[TylerReid]

@bentefay Did you ever come up with anything in your consuming code that would prevent this or are you still relying on code review? I am also interested in a feature like this.

My first thought was that it would be nice to be able to optionally turn on a check that prevents SingleInstance registrations from resolving anything that is not also SingleInstance. I was thinking of something like builder.DisallowSingleInstanceScopeWidening(); (with a better name hopefully). This would not be as flexible as your proposal, but might be easier to implement and would prevent every bug I have seen of this kind.

@tillig Does this sound like an easier implementation? I was going to take a closer look but would like someone's opinion that didn't just clone the repo today.

[tillig]

Any sort of analysis like this will be a challenge to implement. Happy to see any PR, but if we're going to support it, it really needs to be something folks can rely on if they use it. If it only handles a subset of cases, we will absolutely get more issues asking why it isn't a 100% functional feature and it may become more maintenance burden than it's worth.

[bentefay]

Hey @TylerReid, unfortunately I never found the time to investigate further, but a colleague recently pointed out that, at least for us, we could potentially use (or write) a library like the autofac-analysis project to detect issues like captive depedencies with our app as part of our integration tests. If you're happy detecting issues at test-time, this could present a viable alternative that has the added benefit of not impacting the performance of the app.

[tillig]

It's been about five years since this was first filed and the Autofac team stance here remains the same: We'd entertain a PR for it (that doesn't drastically impact overall performance and which handles all the cases) but we won't be focusing on creating this feature. I'm going to close the issue for now and if someone submits a PR for it, we'd love to check it out. Or if an Autofac team member has a bunch of extra time to dedicate to this in the future, here it is.