Skip to content

Commit

Permalink
Resolved #929: Resetting resolve op activation stack on succeed or fa…
Browse files Browse the repository at this point in the history
…il to allow try/catch in lambda registrations.
  • Loading branch information
tillig committed Oct 24, 2018
1 parent 3e34d5c commit 9d78afc
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 8 deletions.
25 changes: 17 additions & 8 deletions src/Autofac/Core/Resolving/ResolveOperation.cs
Expand Up @@ -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<ResolveOperationEndingEventArgs> CurrentOperationEnding;
Expand Down
76 changes: 76 additions & 0 deletions test/Autofac.Test/Core/Resolving/ResolveOperationTests.cs
Expand Up @@ -38,6 +38,36 @@ public void ActivatingArgsSuppliesParameters()
Assert.Equal(provided, passed);
}

[Fact]
public void ActivationStackResetsOnFailedLambdaResolve()
{
// Issue #929
var builder = new ContainerBuilder();
builder.RegisterType<ServiceImpl>().AsSelf();
builder.Register<IService>(c =>
{
try
{
// This will fail because ServiceImpl needs a Guid ctor
// parameter and it's not provided.
return c.Resolve<ServiceImpl>();
}
catch (Exception)
{
// This is where the activation stack isn't getting reset.
}
return new ServiceImpl(Guid.Empty);
});
builder.RegisterType<Dependency>().AsSelf();
builder.RegisterType<ComponentConsumer>().AsSelf();
var container = builder.Build();

// This throws a circular dependency exception if the activation stack
// doesn't get reset.
container.Resolve<ComponentConsumer>();
}

[Fact]
public void AfterTheOperationIsFinished_ReusingTheTemporaryContextThrows()
{
Expand Down Expand Up @@ -150,5 +180,51 @@ public void PropertyInjectionPassesNamedParameterOfTheInstanceTypeBeingInjectedO
var instanceType = capturedparameters.Named<Type>(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;
}
}
}
}

0 comments on commit 9d78afc

Please sign in to comment.