Skip to content

Commit

Permalink
CopyOnWriteRegistry: Pick the correct registry to dispose.
Browse files Browse the repository at this point in the history
When the registry hasn't been touched, nothing needs to be disposed. Only when
the registry has been modified, and hence a new (write) registry allocated,
should the registry be disposed. This is determined by the _writeRegistry
field being non-null.

Fixes Resharper warnings in that class.

Issue autofac#746
  • Loading branch information
afabre committed Jul 23, 2017
1 parent eb27003 commit d505042
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 11 deletions.
21 changes: 10 additions & 11 deletions src/Autofac/Core/Registration/CopyOnWriteRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,9 @@ internal class CopyOnWriteRegistry : IComponentRegistry

public CopyOnWriteRegistry(IComponentRegistry readRegistry, Func<IComponentRegistry> createWriteRegistry)
{
if (readRegistry == null) throw new ArgumentNullException(nameof(readRegistry));
if (createWriteRegistry == null) throw new ArgumentNullException(nameof(createWriteRegistry));

_readRegistry = readRegistry;
_createWriteRegistry = createWriteRegistry;
this.Properties = new FallbackDictionary<string, object>(readRegistry.Properties);
_readRegistry = readRegistry ?? throw new ArgumentNullException(nameof(readRegistry));
_createWriteRegistry = createWriteRegistry ?? throw new ArgumentNullException(nameof(createWriteRegistry));
Properties = new FallbackDictionary<string, object>(readRegistry.Properties);
}

private IComponentRegistry Registry => _writeRegistry ?? _readRegistry;
Expand All @@ -69,7 +66,9 @@ public CopyOnWriteRegistry(IComponentRegistry readRegistry, Func<IComponentRegis

public void Dispose()
{
_readRegistry?.Dispose();
// The _readRegistry doesn't need to be disposed if it still points to the initial registry.
// Only the potentially allocated registry, containing additional registrations, needs to be disposed.
_writeRegistry?.Dispose();
}

public bool TryGetRegistration(Service service, out IComponentRegistration registration)
Expand Down Expand Up @@ -101,8 +100,8 @@ public IEnumerable<IComponentRegistration> RegistrationsFor(Service service)

public event EventHandler<ComponentRegisteredEventArgs> Registered
{
add { WriteRegistry.Registered += value; }
remove { WriteRegistry.Registered -= value; }
add => WriteRegistry.Registered += value;
remove => WriteRegistry.Registered -= value;
}

public void AddRegistrationSource(IRegistrationSource source)
Expand All @@ -116,8 +115,8 @@ public void AddRegistrationSource(IRegistrationSource source)

public event EventHandler<RegistrationSourceAddedEventArgs> RegistrationSourceAdded
{
add { WriteRegistry.RegistrationSourceAdded += value; }
remove { WriteRegistry.RegistrationSourceAdded -= value; }
add => WriteRegistry.RegistrationSourceAdded += value;
remove => WriteRegistry.RegistrationSourceAdded -= value;
}
}
}
14 changes: 14 additions & 0 deletions test/Autofac.Test/Core/Registration/CopyOnWriteRegistryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,5 +48,19 @@ public void RegistrationsMadeByUpdatingAChildScopeDoNotAppearInTheParentScope()
Assert.True(childScope.IsRegistered<object>());
Assert.False(container.IsRegistered<object>());
}

[Fact]
public void OnlyRegistrationsMadeOnTheRegistryAreDisposedWhenTheRegistryIsDisposed()
{
var componentRegistration = Mocks.GetComponentRegistration();
var readOnlyRegistry = new ComponentRegistry();
readOnlyRegistry.Register(componentRegistration);
var cow = new CopyOnWriteRegistry(readOnlyRegistry, () => new ComponentRegistry());
var nestedComponentRegistration = Mocks.GetComponentRegistration();
cow.Register(nestedComponentRegistration);
cow.Dispose();
Assert.False(componentRegistration.IsDisposed);
Assert.True(nestedComponentRegistration.IsDisposed);
}
}
}
51 changes: 51 additions & 0 deletions test/Autofac.Test/Mocks.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using System;
using System.Collections.Generic;
using System.Reflection;
using Autofac.Core;
using Autofac.Core.Activators.Reflection;

namespace Autofac.Test
Expand All @@ -21,6 +23,11 @@ public static Startable GetStartable()
return new Startable();
}

public static MockComponentRegistration GetComponentRegistration()
{
return new MockComponentRegistration();
}

internal class MockConstructorFinder : IConstructorFinder
{
public ConstructorInfo[] FindConstructors(Type targetType)
Expand All @@ -46,5 +53,49 @@ public void Start()
StartCount++;
}
}

internal class MockComponentRegistration : IComponentRegistration
{
public void Dispose()
{
IsDisposed = true;
}

public bool IsDisposed { get; set; }

public Guid Id { get; }

public IInstanceActivator Activator { get; }

public IComponentLifetime Lifetime { get; }

public InstanceSharing Sharing { get; }

public InstanceOwnership Ownership { get; }

public IEnumerable<Service> Services { get; } = new Service[0];

public IDictionary<string, object> Metadata { get; }

public IComponentRegistration Target { get; }

public event EventHandler<PreparingEventArgs> Preparing;

public void RaisePreparing(IComponentContext context, ref IEnumerable<Parameter> parameters)
{
}

public event EventHandler<ActivatingEventArgs<object>> Activating;

public void RaiseActivating(IComponentContext context, IEnumerable<Parameter> parameters, ref object instance)
{
}

public event EventHandler<ActivatedEventArgs<object>> Activated;

public void RaiseActivated(IComponentContext context, IEnumerable<Parameter> parameters, object instance)
{
}
}
}
}

0 comments on commit d505042

Please sign in to comment.