Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

try/catch in Lambda Registration Doesn't Reset the Activation Stack #929

Closed
alexandery opened this issue Aug 19, 2018 · 2 comments
Closed
Assignees
Labels

Comments

@alexandery
Copy link

Greetings,

We are running into a Circular Dependency exception when using lambda registration along with some transitive dependencies and manual resolution of components.

This issue happens when the following 2 factors are present:

  1. DependecyResolutionException is thrown inside of registration lambda (obviously handled, but it still affects further resolution)
  2. There are transitive dependencies - eg:
Worker(IService service, IDependency dependency);

class Dependency : IDependency {
  Dependency(IService service) {}
}

It seems Autofac gets in a state that leads to the circular dependency exception when DependecyResolutionException is thrown inside the lambda registratin. It doesn't matter that DependecyResolutionException is handled - as long as it gets thrown - the circular dependency issue happens.

To fully showcase the issue I've created a sample solution.

Partial code (with registration and resolution) is as follows:

    class Program
    {
        private IContainer container;

        static void Main(string[] args)
        {
            var program = new Program();

            program.PrepareContainer();

            program.Action();

            Console.ReadLine();
        }

        public void PrepareContainer()
        {
            var builder = new ContainerBuilder();

            // registering services and a factory to manage service creation based on particular parameters
            builder.RegisterType<SimpleService>().As<ISimpleService>().AsSelf().InstancePerLifetimeScope();
            builder.RegisterGeneratedFactory<SimpleService.Factory>(new TypedService(typeof(SimpleService)));
            builder.RegisterType<ComplexService>().As<IComplexService>().AsSelf().InstancePerLifetimeScope();
            builder.RegisterGeneratedFactory<ComplexService.Factory>(new TypedService(typeof(ComplexService)));
            // custom factory allowing creation of a specific type
            builder.RegisterType<ServiceFactory>().AsImplementedInterfaces().InstancePerLifetimeScope();

            // user code always references IService - specializations are managed at a lower level and not important to user code
            // so, resolution always looks for whether there is a specialized instance already resolved
            builder.Register<IService>(c =>
            {
                try { return c.Resolve<ISimpleService>(); } catch (Exception) { }
                try { return c.Resolve<IComplexService>(); } catch (Exception) { }
                var factory = c.Resolve<SimpleService.Factory>();
                return factory.Invoke(Guid.Empty);
            });

            // SomeDependency takes in IService instance as well
            builder.RegisterType<SomeDependency>().As<ISomeDependency>().InstancePerLifetimeScope();
            builder.RegisterType<MyWorker>().AsSelf().InstancePerLifetimeScope();

            container = builder.Build();
        }

        public void Action()
        {
            using (var scope = container.BeginLifetimeScope())
            {
                var serviceFactory = scope.Resolve<IServiceFactory>();
                // lower level code always makes a decision which concrete service to assign to a given lifetime scope
                // since the lifecycle is InstancePerLifetimeScope - as long as any implementation is resolved, it should be available to all users in a given scope
                
                // as long as IComplexService resolution attempt is *first* in the lambda registration above - resolution of MyWorker below will work correctly
                // currently IComplextService resolution attempt follows ISimpleService resolution attempt (which throws an exception) - the instance of IComplexService gets returned properly
                // but MyWorker below will not get resolved correctly
                var service = serviceFactory.CreateComplexService(Guid.NewGuid());

                // Uncomment this (and comment "serviceFactory.CreateComplexService(Guid.NewGuid());" above) to observe correct resolution of everything
                //var service = serviceFactory.CreateSimpleService(Guid.NewGuid());

                // This throws circular dependendy exception if:
                // - service type resolved above is *not the first* to be resolved by lambda registration
                // - seems that a DependencyResolutionException that gets thrown in this situation by first "c.Resolve<>" somehow affects the behavior of the container
                var worker = scope.Resolve<MyWorker>();
                worker.InvokeAction();
            }
        }
    }

P.S.: I've looked through the opened issues and didn't see the lambda registration use case. Apologies if I did miss it (found few other use cases for Circular Dependency).

@tillig
Copy link
Member

tillig commented Oct 24, 2018

I've spent a couple of hours reducing your example much further. Figuring out what was going on was a lot easier once some moving parts were removed; a true "minimal repro" really helped.

Here's the much simpler version that still reproduces the issue.

using System;
using Autofac;
using Autofac.Builder;
using Autofac.Core;

namespace Sample
{
    public interface IService
    {
    }

    public class MyWorker
    {
        public SomeDependency dependency;

        public IService service;

        public MyWorker(IService service, SomeDependency dependency)
        {
            this.service = service;
            this.dependency = dependency;
        }
    }

    public static class Program
    {
        public static IContainer PrepareContainer()
        {
            var builder = new ContainerBuilder();
            builder.RegisterType<SimpleService>().AsSelf();
            builder.RegisterGeneratedFactory<SimpleService.Factory>(new TypedService(typeof(SimpleService)));
            builder.Register<IService>(c =>
            {
                // Comment out this try/catch to enable things to work.
                try
                {
                    return c.Resolve<SimpleService>();
                }
                catch (Exception)
                {
                }
                var factory = c.Resolve<SimpleService.Factory>();
                return factory.Invoke(Guid.Empty);
            });
            builder.RegisterType<SomeDependency>().AsSelf();
            builder.RegisterType<MyWorker>().AsSelf();
            return builder.Build();
        }

        private static void Main(string[] args)
        {
            var container = PrepareContainer();
            using (var scope = container.BeginLifetimeScope())
            {
                scope.Resolve<MyWorker>();
            }

            Console.ReadLine();
        }
    }

    public class SimpleService : IService
    {
        private Guid id;

        public SimpleService(Guid id)
        {
            this.id = id;
        }

        public delegate SimpleService Factory(Guid id);
    }

    public class SomeDependency
    {
        private IService service;

        public SomeDependency(IService service)
        {
            this.service = service;
        }
    }
}

You don't need:

  • ComplexService
  • Either of the ISimpleService or IComplexService interfaces
  • IServiceFactory or ServiceFactory
  • The ISomeDependency interface
  • Any of the InstancePerLifetimeScope settings

Once that's all simplified down, it's much easier to see into what's going on by doing a little debugging.

Something important to know is that for each resolve operation (scope.Resolve<MyWorker>()) there is an activation stack. Each thing that needs to be resolved in a chain during an operation gets pushed onto the stack. If the same thing tries to get activated (created/instantiated/etc.) twice in the same stack, it's seen as a circular dependency.

In a working scenario (where the try/catch is commented out) the activation stack gets reset for each dependency. It looks like:

  • MyWorker
    • IService (the dependency for MyWorker ctor)
      • SimpleService.Factory (the factory in the lambda registration)
    • SomeDependency (the dependency for MyWorker ctor)
      • IService (the dependency for MyWorker ctor)
        • SimpleService.Factory (the factory in the lambda registration)

IService appears twice, but not in the same general hierarchy, so there's no circular dependency.

In a failing scenario (as seen above, with the try/catch enabled) the activation stack for the resolve context isn't getting reset because it's assumed that if a DependencyResolutionException occurs (the resolution fails) that the current resolve operation will be aborted.

  • MyWorker
    • IService (the dependency for MyWorker ctor)
      • SimpleService (the direct Resolve call in the lambda registration)
        • SimpleService.Factory (the factory in the lambda registration... WHAT?! this is because the activation stack is still going - it's the same logical resolve operation which started from MyWorker and you're resolving on the same context)
      • SomeDependency (the dependency for MyWorker ctor... but the activation stack is corrupt because of the try/catch)
        • IService (the dependency for MyWorker ctor - uh oh, IService shows up in the same stack, soooo 💥)

By doing the try/catch, you're not allowing the resolve operation to abort; you're disengaging the safety mechanism.

You can see how the activation stack assumes it can push/pop based on the relative success or failure of a complete resolve op inside ResolveOperation.

It looks like I can switch ResolveOperation to put a try/finally around the handling of the activation stack to allow the stack to properly reset.

try
{
    var instance = activation.Execute();
    _successfulActivations.Add(activation);

    return instance;
}
finally
{
    _activationStack.Pop();

    if (_activationStack.Count == 0)
        CompleteActivations();

    --_callDepth;
}

I'll work on that, however, I would recommend that you also implement a workaround to keep yourself safe from some of the side effects here - instead of resolving the try/catch stuff from the context, you can resolve from the same lifetime scope in a different resolve op:

builder.Register<IService>(c =>
{
  try
  {
    // Resolve an ILifetimeScope, which will give you the current scope,
    // then resolve the service from that. It should still allow sharing component
    // lifetimes, but will start a new resolve operation instead of chaining onto
    // the current op. Note this will _also_ possibly break the built-in circular
    // dependency detection, so it will be on you to choose if this is the right
    // solution to go with.
    return c.Resolve<ILifetimeScope>().Resolve<SimpleService>();
  }
  catch (Exception)
  {
  }
  var factory = c.Resolve<SimpleService.Factory>();
  return factory.Invoke(Guid.Empty);
});

@tillig tillig self-assigned this Oct 24, 2018
@tillig tillig added the bug label Oct 24, 2018
@tillig tillig changed the title Circular Dependency and lambda registration issue try/catch in Lambda Registration Doesn't Reset the Activation Stack Oct 24, 2018
@tillig
Copy link
Member

tillig commented Oct 24, 2018

As it turns out, the example can be even further simplified by removing the generated factory. Instead of calling the factory at the end of the lambda, just return new SimpleService(Guid.Empty).

@tillig tillig closed this as completed in 9d78afc Oct 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants