Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions src/Autofac/Builder/RegistrationData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public class RegistrationData
private bool _defaultServiceOverridden;
private Service _defaultService;

private readonly ICollection<Service> _services = new HashSet<Service>();
private readonly HashSet<Service> _services = [];

private IComponentLifetime _lifetime = CurrentScopeLifetime.Instance;

Expand All @@ -43,12 +43,15 @@ public IEnumerable<Service> Services
{
get
{
if (_defaultServiceOverridden)
if (!_defaultServiceOverridden && !_services.Contains(_defaultService))
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed the declared type of _services above so that it's clear this is not an O(N) operation.

{
return _services;
yield return _defaultService;
}

return _services.DefaultIfEmpty(_defaultService);
foreach (var service in _services)
{
yield return service;
}
}
}

Expand All @@ -65,11 +68,15 @@ public void AddServices(IEnumerable<Service> services)
throw new ArgumentNullException(nameof(services));
}

_defaultServiceOverridden = true; // important even when services is empty
var empty = true;
foreach (var service in services)
{
empty = false;
AddService(service);
}

// `As([])` clears the default service.
_defaultServiceOverridden = _defaultServiceOverridden || empty;
}

/// <summary>
Expand All @@ -83,7 +90,9 @@ public void AddService(Service service)
throw new ArgumentNullException(nameof(service));
}

_defaultServiceOverridden = true;
// `AutoActivateService` is internal plumbing; `AutoActivate()` isn't expected to modify user-visible
// services.
_defaultServiceOverridden = _defaultServiceOverridden || service is not AutoActivateService;
_services.Add(service);
}

Expand Down
20 changes: 5 additions & 15 deletions src/Autofac/RegistrationExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ public static IRegistrationBuilder<T, SimpleActivatorData, SingleRegistrationSty

rb.SingleInstance();

// https://github.com/autofac/Autofac/issues/1102
// Single instance registrations with any custom activation phases (i.e. activation handlers) need to be auto-activated,
// so that other behavior (such as OnRelease) that expects 'normal' object lifetime behavior works as expected.
rb.RegistrationData.AddService(new AutoActivateService());
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This simplification wasn't possible prior to the other fix, as it would have broken default service logic.


rb.RegistrationData.DeferredCallback = builder.RegisterCallback(cr =>
{
if (rb.RegistrationData.Lifetime is not RootScopeLifetime ||
Expand All @@ -79,21 +84,6 @@ public static IRegistrationBuilder<T, SimpleActivatorData, SingleRegistrationSty

activator.DisposeInstance = rb.RegistrationData.Ownership == InstanceOwnership.OwnedByLifetimeScope;

// https://github.com/autofac/Autofac/issues/1102
// Single instance registrations with any custom activation phases (i.e. activation handlers) need to be auto-activated,
// so that other behavior (such as OnRelease) that expects 'normal' object lifetime behavior works as expected.
if (rb.ResolvePipeline.Middleware.Any(s => s.Phase == PipelinePhase.Activation))
{
var autoStartService = rb.RegistrationData.Services.First();
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bug here is due to the possibility that there are multiple registrations for this service; resolving it won't guarantee that we end up with the same component (and in fact, some unrelated random thing could end up unexpectedly activated :-))


var activationRegistration = new RegistrationBuilder<T, SimpleActivatorData, SingleRegistrationStyle>(
new AutoActivateService(),
new SimpleActivatorData(new DelegateActivator(typeof(T), (c, p) => c.ResolveService(autoStartService))),
new SingleRegistrationStyle());

RegistrationBuilder.RegisterSingleComponent(cr, activationRegistration);
}

RegistrationBuilder.RegisterSingleComponent(cr, rb);
});

Expand Down
20 changes: 20 additions & 0 deletions test/Autofac.Specification.Test/Features/StartableTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,26 @@ public void Startable_WhenTheContainerIsBuilt_StartableComponentsThatDependOnAut
Assert.Equal(expectedStartCount, StartableDependency.Count);
}

[Fact]
public void AutoActivate_DoesNotHideDefaultSelfService()
{
var builder = new ContainerBuilder();
builder.RegisterType<MyComponent2>().AutoActivate();
using var container = builder.Build();
Assert.True(container.IsRegistered<MyComponent2>());
}

[Fact]
public void AutoActivate_RegisterInstanceActivationWorksWhenDefaultServiceOverloaded()
{
var instanceCount = 0;
var builder = new ContainerBuilder();
builder.RegisterInstance(new MyComponent2()).As<object>().OnActivated(_ => instanceCount++);
builder.RegisterType<object>();
builder.Build();
Assert.Equal(1, instanceCount);
}

private class ComponentTakesStartableDependency : IStartable
{
public ComponentTakesStartableDependency(StartableTakesDependency dependency, bool expectStarted)
Expand Down