Skip to content

Commit

Permalink
Check if parent scopes are disposed when checking scope IsDisposed
Browse files Browse the repository at this point in the history
  • Loading branch information
tillig authored Dec 22, 2019
2 parents 2c63e85 + 3a8ef4c commit 852cafd
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 9 deletions.
21 changes: 17 additions & 4 deletions src/Autofac/Core/Lifetime/LifetimeScope.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public class LifetimeScope : Disposable, ISharingLifetimeScope, IServiceProvider
private readonly object _synchRoot = new object();
private readonly ConcurrentDictionary<Guid, object> _sharedInstances = new ConcurrentDictionary<Guid, object>();
private object? _anonymousTag;
private LifetimeScope? parentScope;

internal static Guid SelfRegistrationId { get; } = Guid.NewGuid();

Expand All @@ -70,8 +71,8 @@ public class LifetimeScope : Disposable, ISharingLifetimeScope, IServiceProvider
protected LifetimeScope(IComponentRegistry componentRegistry, LifetimeScope parent, object tag)
: this(componentRegistry, tag)
{
ParentLifetimeScope = parent ?? throw new ArgumentNullException(nameof(parent));
RootLifetimeScope = ParentLifetimeScope.RootLifetimeScope;
parentScope = parent ?? throw new ArgumentNullException(nameof(parent));
RootLifetimeScope = parentScope.RootLifetimeScope;
}

/// <summary>
Expand Down Expand Up @@ -281,7 +282,7 @@ public object ResolveComponent(ResolveRequest request)
/// <summary>
/// Gets the parent of this node of the hierarchy, or null.
/// </summary>
public ISharingLifetimeScope? ParentLifetimeScope { get; }
public ISharingLifetimeScope? ParentLifetimeScope => parentScope;

/// <summary>
/// Gets the root of the sharing hierarchy.
Expand Down Expand Up @@ -354,6 +355,7 @@ protected override void Dispose(bool disposing)

// ReSharper disable once InconsistentlySynchronizedField
_sharedInstances.Clear();
parentScope = null;
}

base.Dispose(disposing);
Expand All @@ -376,6 +378,7 @@ protected override async ValueTask DisposeAsync(bool disposing)

// ReSharper disable once InconsistentlySynchronizedField
_sharedInstances.Clear();
parentScope = null;
}

// Don't call the base (which would just call the normal Dispose).
Expand All @@ -384,10 +387,20 @@ protected override async ValueTask DisposeAsync(bool disposing)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void CheckNotDisposed()
{
if (IsDisposed)
if (IsTreeDisposed())
throw new ObjectDisposedException(LifetimeScopeResources.ScopeIsDisposed, innerException: null);
}

/// <summary>
/// Gets a value indicating whether this or any of the parent disposables have been disposed.
/// </summary>
/// <returns>true if this instance of any of the parent instances have been disposed.</returns>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private bool IsTreeDisposed()
{
return IsDisposed || (parentScope is object && parentScope.IsTreeDisposed());
}

/// <summary>
/// Gets the service object of the specified type.
/// </summary>
Expand Down
7 changes: 3 additions & 4 deletions src/Autofac/Core/Lifetime/LifetimeScopeResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/Autofac/Core/Lifetime/LifetimeScopeResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@
<value>The tag '{0}' has already been assigned to a parent lifetime scope. If you are using Owned&lt;T&gt; this indicates you may have a circular dependency chain.</value>
</data>
<data name="ScopeIsDisposed" xml:space="preserve">
<value>Instances cannot be resolved and nested lifetimes cannot be created from this LifetimeScope as it has already been disposed.</value>
<value>Instances cannot be resolved and nested lifetimes cannot be created from this LifetimeScope as it (or one of its parent scopes) has already been disposed.</value>
</data>
<data name="SelfConstructingDependencyDetected" xml:space="preserve">
<value>The constructor of type '{0}' attempted to create another instance of itself. This is not permitted because the service is configured to only allowed a single instance per lifetime scope.</value>
Expand Down
56 changes: 56 additions & 0 deletions test/Autofac.Specification.Test/Lifetime/DisposalTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,62 @@ public void TwoLayersOfExternalRegistration_OnDisposingInnerLayer_OuterLayerRema
Assert.True(rootInstance.IsDisposed);
}

[Fact]
public void DisposalOfContainerPreventsResolveInLifetimeScope()
{
var containerBuilder = new ContainerBuilder();
containerBuilder.Register(c => 1);

var container = containerBuilder.Build();

var scope = container.BeginLifetimeScope();

container.Dispose();

Assert.Throws<ObjectDisposedException>(() =>
{
scope.Resolve<int>();
});
}

[Fact]
public void DisposalOfParentScopePreventsResolveInNestedScope()
{
var containerBuilder = new ContainerBuilder();
containerBuilder.Register(c => 1);

var container = containerBuilder.Build();

var outerScope = container.BeginLifetimeScope();
var innerScope = outerScope.BeginLifetimeScope();

outerScope.Dispose();

Assert.Throws<ObjectDisposedException>(() =>
{
innerScope.Resolve<int>();
});
}

[Fact]
public void DisposalOfContainerPreventsResolveInNestedScope()
{
var containerBuilder = new ContainerBuilder();
containerBuilder.Register(c => 1);

var container = containerBuilder.Build();

var outerScope = container.BeginLifetimeScope();
var innerScope = outerScope.BeginLifetimeScope();

container.Dispose();

Assert.Throws<ObjectDisposedException>(() =>
{
innerScope.Resolve<int>();
});
}

private class A : DisposeTracker
{
}
Expand Down

0 comments on commit 852cafd

Please sign in to comment.