InvalidOperationException from Owned<T> lifetime scope circular dependency

[EbenZhang]

We recently upgraded to Autofac 4.8.1 from 3.x and noticed this issue.

Stacktrace:

The tag 'Test.Bar' has already been assigned to a parent lifetime scope.
at Autofac.Core.Lifetime.LifetimeScope.BeginLifetimeScope(Object tag)

Steps to reproduce:

class Foo {
    private readonly Func<Owned<Bar>> _barFactory; 
    public Foo(Func<Owned<Bar>> barFactory) {
        _barFactory = barFactory;
    }

    public void Execute() {
        using (var bar = _barFactory()) {
            bar.Value.Execute();
        }
    }
}

class Bar {
    private readonly Func<Owned<Zzz>> _zzzFactory;
     public Bar(Func<Owned<Zzz>> zzzFactory) {
        _zzzFactory = zzzFactory;
    }

    public void Execute() {
        using (var zzz = _zzzFactory()) {
            zzz.Value.Execute();
        }
    }
}

class Zzz {
    private readonly Func<Owned<Bar>> _barFactory; 
    public Zzz(Func<Owned<Bar>> barFactory) {
        _barFactory = barFactory;
    }

    public void Execute() {
        using (var bar = _barFactory()) {
        }
    }
}

class Test {
    public static void Run() {
        var builder = new ContainerBuilder();
        builder.RegisterType<Foo>().AsSelf();
        builder.RegisterType<Bar>().AsSelf();
        builder.RegisterType<Zzz>().AsSelf();
        var container = builder.Build();
        var test = container.Resolve<Foo>();
        test.Execute();
    }
}

Admittedly, the code is smelly. But just let you know something is broken if not intentionally.

[tillig]

It looks like you have a circular reference. Here's how I'm reading it:

• Foo depends on Bar
• Bar depends on Zzz
• Zzz depends on Bar

Regardless of whether they're Owned<T> or not, that last bit is a circular reference.

The overall change you're seeing is due to #882 - you really shouldn't be nesting lifetime scopes that are tagged with the same name. Like, you can't have a web request scope inside another web request scope.

When Owned<T> gets resolved, it creates a tiny tagged lifetime scope where the tag is actually the owned instance type.

Just brainstorming, I think we could possibly modify this such that the tag on the Owned<T> scope is unique, which would get you past this exception, but I think you'd still see a circular reference problem with the above code.

Off hand, I can't think of any reason the Owned<T> scope tag couldn't be... well, anything unique. A GUID or something. That way if you had Foo => Owned<Bar> => Zzz => Owned<Bar> you could get two different Owned<Bar> instances whereas right now I think you'd hit the same exception about duplicate lifetime scope names.

@alexmg Can you think of anything I'm missing with that?

[tillig]

Now that I'm looking at this, no matter how I slice it the repro always ends up with a circular reference in it. Any hierarchy I create that has the same Owned<T> in it multiple times necessarily implies a circular reference, regardless of how deep. A => B => C => D => A is still circular. However, if I split it up...

   -> Owned<B> -> C
A +
   -> Owned<D> -> Owned<B>

Like if A needs a B and a D and the D needs a B... that won't be in the same lifetime scope hierarchy so you'd never get the exception.

Is there a repro of this that doesn't have a circular dependency chain? If not, I'm feeling more like this is a "don't do that" situation we could cover in documentation - Owned<T> plus circular dependencies don't mix.

[EbenZhang]

It's not a normal circular dependency like

class Foo {
    Bar bar;
}
class Bar {
    Foo foo;
}

In the example I should probably use interface instead of concrete type (result is the same anyway). Using interface is how people break the circular dependency. The code works totally fine without Owned.

interface IFoo {
    void Execute();
}
class Foo : IFoo {
    private readonly Func<Owned<IBar>> _barFactory;
    public Foo(Func<Owned<IBar>> barFactory) {
        _barFactory = barFactory;
    }
    public void Execute() {
        using (var bar = _barFactory()) {
            bar.Value.Execute();
        }
    }
}

interface IBar {
    void Execute();
}
class Bar : IBar {
    private readonly Func<Owned<IZzz>> _zzzFactory; 
    public Bar(Func<Owned<IZzz>> zzzFactory) {
        _zzzFactory = zzzFactory;
    }
    public void Execute() {
        using (var zzz = _zzzFactory()) {
            zzz.Value.Execute();
        }
    }
}

interface IZzz {
    void Execute();
}
class Zzz : IZzz {
    private readonly Func<Owned<IBar>> _barFactory;
    public Zzz(Func<Owned<IBar>> barFactory) {
        _barFactory = barFactory;
    }
    public void Execute() {
        using (var bar = _barFactory()) {
        }
    }
}

class Program {
    static void Main(string[] args) {
        var builder = new ContainerBuilder();
        builder.RegisterType<Foo>().As<IFoo>();
        builder.RegisterType<Bar>().As<IBar>();
        builder.RegisterType<Zzz>().As<IZzz>();
        var container = builder.Build();
        var fooFactory = container.Resolve<Func<Owned<IFoo>>>();
        using (var foo = fooFactory()) {
            foo.Value.Execute();
        }
    }
}

[alexmg]

It does seem like there is a circular dependency waiting to happen. I noticed that the Zzz class does not access the Value property on the IBar instance and invoke the Execute method. If it did then I think you will find the circular dependency gets created bouncing between the two Execute methods in Bar and Zzz. Just accessing the Value property in Zzz and creating the instance should be alright. It certainly feels very dangerous having the circular dependency just a simple method call away.

[alexmg]

I'm fairly sure that using something like new object() for the lifetime scope tag for the Owned instance would work. I suspect the choice of using the Service as tag was arbitrary and didn't consider that it may be reused or that the change in #882 would be needed.

The question is whether or not the situation is considered too dangerous allow. That circular dependency caused by the Execute method being invoked would result in a StackOverflowException because the duplicate lifetime scope tag is currently the only way to detect it during resolution. The combination of the Func and Owned means you would be on your own at the point the circular dependency kicked in.

[tillig]

By and large Autofac has been pretty laissez-faire about allowing people to do things they want to do when it comes to advanced scenarios without putting up too many guard rails. Like the "parent scope disposal doesn't dispose child lifetime scopes" thing. You set it up, you get to dispose it. Which isn't to say a StackOverflowException from the circular dependency will be better, but it is what it is.

Let me do a quick check to see what happens in this situation if we uniquely name the scopes.

[tillig]

I switched the Owned<T> lifetime to be a Guid.NewGuid() and you can get past this, but the key is really in Zzz where it's not actually using the Bar:

public void Execute()
{
  using (var bar = this._barFactory())
  {
    // Execute isn't getting called so the circle doesn't quite complete... yet.
  }
}

As soon as you complete the circle, it hits StackOverflowException.

public void Execute()
{
  using (var bar = this._barFactory())
  {
    bar.Value.Execute();
  }
}

I recognize in reality that Bar may have other methods that don't actually end up resolving anything, hence it looks like it's not a circular dependency. But it is. And there's not much we can do about it since it's a circle created with factory indirections.

For reference, if you switch the Func<Owned<T>> instances to Func<T> you can still "outsmart" the circular dependency detector because of the indirection of the Func<T> factory deferring the resolution.

public class Bar
{
    private readonly Func<Zzz> _zzzFactory;

    public Bar(Func<Zzz> zzzFactory)
    {
        this._zzzFactory = zzzFactory;
    }

    public void Execute()
    {
        var zzz = this._zzzFactory();
        zzz.Execute();
    }
}

public class Foo
{
    private readonly Func<Bar> _barFactory;

    public Foo(Func<Bar> barFactory)
    {
        this._barFactory = barFactory;
    }

    public void Execute()
    {
        var bar = this._barFactory();
        bar.Execute();
    }
}

public class Zzz
{
    private readonly Func<Bar> _barFactory;

    public Zzz(Func<Bar> barFactory)
    {
        this._barFactory = barFactory;
    }

    public void Execute()
    {
        var bar = this._barFactory();
        // It works if you don't complete the circle.
        // Put this in to make the StackOverflowException happen.
        // bar.Execute();
    }
}

I guess that leaves us a couple of options:

1. Keep throwing the InvalidOperationException and add some clarity to the exception message that if you're using Owned<T> it means you have a circular dependency chain. Slightly less flexible, but far easier to troubleshoot.
2. Change the Owned<T> scope name so the code will work and just let people hit the StackOverflowException when creating a circular dependency. More flexible, but pretty hard to troubleshoot.

I'm currently in favor of option 1- stop the circle from happening and make the exception more clear.

[EbenZhang]

when using v3, there was no error calling the bar.execute, I didn't add that line just because already able to reproduce the exception in v4.

[tillig]

The last Autofac 3 release was 2014. A lot has changed, both from an API/features perspective and from an internals perspective. Either way, if bar.Execute gets called, at this point it will invoke the whole resolution chain all the way down forever yielding a stack overflow. If it didn't throw in v3, that sounds like a bug in v3, not a feature to add in v4.

[tillig]

I'm updating the exception message to:

The tag '{0}' has already been assigned to a parent lifetime scope. If you are using Owned<T> this indicates you may have a circular dependency chain.

Hopefully that will at least make this less confusing until we can improve circular dependency detection.