From 9d78afcd2ee27620aefc8a47e583be7d4ff3681d Mon Sep 17 00:00:00 2001 From: Travis Illig Date: Wed, 24 Oct 2018 15:35:26 -0700 Subject: [PATCH] Resolved #929: Resetting resolve op activation stack on succeed or fail to allow try/catch in lambda registrations. --- .../Core/Resolving/ResolveOperation.cs | 25 ++++-- .../Core/Resolving/ResolveOperationTests.cs | 76 +++++++++++++++++++ 2 files changed, 93 insertions(+), 8 deletions(-) diff --git a/src/Autofac/Core/Resolving/ResolveOperation.cs b/src/Autofac/Core/Resolving/ResolveOperation.cs index 317528308..71b5f50ef 100644 --- a/src/Autofac/Core/Resolving/ResolveOperation.cs +++ b/src/Autofac/Core/Resolving/ResolveOperation.cs @@ -121,17 +121,26 @@ public object GetOrCreateInstance(ISharingLifetimeScope currentOperationScope, I var handler = InstanceLookupBeginning; handler?.Invoke(this, new InstanceLookupBeginningEventArgs(activation)); - var instance = activation.Execute(); - _successfulActivations.Add(activation); - - _activationStack.Pop(); + try + { + var instance = activation.Execute(); + _successfulActivations.Add(activation); - if (_activationStack.Count == 0) - CompleteActivations(); + return instance; + } + finally + { + // Issue #929: Allow the activation stack to be popped even if the activation failed. + // This allows try/catch to happen in lambda registrations without corrupting the stack. + _activationStack.Pop(); - --_callDepth; + if (_activationStack.Count == 0) + { + CompleteActivations(); + } - return instance; + --_callDepth; + } } public event EventHandler CurrentOperationEnding; diff --git a/test/Autofac.Test/Core/Resolving/ResolveOperationTests.cs b/test/Autofac.Test/Core/Resolving/ResolveOperationTests.cs index 85baa0463..45c5c9d65 100644 --- a/test/Autofac.Test/Core/Resolving/ResolveOperationTests.cs +++ b/test/Autofac.Test/Core/Resolving/ResolveOperationTests.cs @@ -38,6 +38,36 @@ public void ActivatingArgsSuppliesParameters() Assert.Equal(provided, passed); } + [Fact] + public void ActivationStackResetsOnFailedLambdaResolve() + { + // Issue #929 + var builder = new ContainerBuilder(); + builder.RegisterType().AsSelf(); + builder.Register(c => + { + try + { + // This will fail because ServiceImpl needs a Guid ctor + // parameter and it's not provided. + return c.Resolve(); + } + catch (Exception) + { + // This is where the activation stack isn't getting reset. + } + + return new ServiceImpl(Guid.Empty); + }); + builder.RegisterType().AsSelf(); + builder.RegisterType().AsSelf(); + var container = builder.Build(); + + // This throws a circular dependency exception if the activation stack + // doesn't get reset. + container.Resolve(); + } + [Fact] public void AfterTheOperationIsFinished_ReusingTheTemporaryContextThrows() { @@ -150,5 +180,51 @@ public void PropertyInjectionPassesNamedParameterOfTheInstanceTypeBeingInjectedO var instanceType = capturedparameters.Named(ResolutionExtensions.PropertyInjectedInstanceTypeNamedParameter); Assert.Equal(existingInstance.GetType(), instanceType); } + + private interface IService + { + } + + // Issue #929 + // When a resolve operation fails in a lambda registration the activation stack + // doesn't get reset and incorrectly causes a circular dependency exception. + // + // The ComponentConsumer takes IService (ServiceImpl) and a Dependency; the + // Dependency also takes an IService. Normally this wouldn't cause an issue, but + // if the registration for IService is a lambda that has a try/catch around a failing + // resolve, the activation stack won't reset and the IService will be seen as a + // circular dependency. + private class ComponentConsumer + { + private Dependency _dependency; + + private IService _service; + + public ComponentConsumer(IService service, Dependency dependency) + { + this._service = service; + this._dependency = dependency; + } + } + + private class Dependency + { + private IService _service; + + public Dependency(IService service) + { + this._service = service; + } + } + + private class ServiceImpl : IService + { + private Guid _id; + + public ServiceImpl(Guid id) + { + this._id = id; + } + } } }