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
126 changes: 126 additions & 0 deletions src/Lamar.Testing/Bugs/Bug_descriptor_lifetime_for_fluent_use.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
using System.Linq;
using Microsoft.Extensions.DependencyInjection;
using Shouldly;
using Xunit;

namespace Lamar.Testing.Bugs;

// Repro for a bridge issue between Lamar's fluent registration API and the
// IServiceCollection snapshot Lamar exposes (since ServiceRegistry : IServiceCollection).
//
// When a Lamar Instance is added to the registry via the extension at
// src/Lamar/Scanning/Conventions/ServiceCollectionExtensions.cs:42-45:
//
// public static void Add(this IServiceCollection services, Instance instance)
// {
// services.Add(new ServiceDescriptor(instance.ServiceType, instance));
// }
//
// the descriptor constructor used is ServiceDescriptor(Type, object) which
// hard-codes ServiceLifetime.Singleton (it's the only MS-DI overload that
// accepts a pre-existing instance). So every For<T>().Use<TImpl>() /
// For<T>().Use(factory) / For<T>().Add<TImpl>() / ...Scoped() / ...Transient()
// call writes a ServiceDescriptor whose Lifetime is Singleton, regardless of
// what the underlying Lamar Instance says.
//
// Lamar itself still honors Instance.Lifetime when resolving, so apps that
// only ever go through Container.GetInstance<T>() never see this. But anything
// that reads the IServiceCollection snapshot directly (e.g. Wolverine 5+'s
// HTTP endpoint code-gen, JasperFx's ServiceCollectionServerVariableSource)
// sees Singleton and treats the registration accordingly. That's the
// "Use existing HttpContext ServiceProvider for opaque scoped services"
// scenario tracked at JasperFx/wolverine#1610 — for users of Lamar via
// Wolverine HTTP it manifests as captive dependencies: a single resolved
// instance baked into a singleton-lifetime generated handler class.
//
// This file demonstrates the underlying mismatch as plain xUnit tests against
// Lamar alone, no Wolverine reference needed.
public interface ILifetimeBridgeThing { }
public class LifetimeBridgeThing : ILifetimeBridgeThing { }

public class Bug_descriptor_lifetime_for_fluent_use
{
// Convention-scan registrations (WithDefaultConventions()) are NOT affected:
// DefaultConventionScanner.ScanTypes calls the (serviceType, implType, lifetime)
// descriptor ctor directly at DefaultConventionScanner.cs:28, so the descriptor
// carries an honest lifetime. The bug below is specific to the fluent API paths
// that route through the Add(IServiceCollection, Instance) extension.

// ------- Fluent paths: descriptor.Lifetime is silently Singleton ----------

[Fact]
public void For_Use_writes_Transient_lifetime_to_descriptor()
{
// ServiceRegistry.For<T>() returns an InstanceExpression with
// ServiceLifetime.Transient as its default (ServiceRegistry.cs:96).
// The Instance Lamar adds also has Lifetime = Transient. But the
// ServiceDescriptor surfaced to IServiceCollection consumers will
// be Singleton.
var registry = new ServiceRegistry();
registry.For<ILifetimeBridgeThing>().Use<LifetimeBridgeThing>();

var descriptor = registry.LastOrDefault(d => d.ServiceType == typeof(ILifetimeBridgeThing));
descriptor.ShouldNotBeNull();
descriptor.Lifetime.ShouldBe(ServiceLifetime.Transient);
}

[Fact]
public void For_Use_Scoped_writes_Scoped_lifetime_to_descriptor()
{
// .Scoped() updates instance.Lifetime to Scoped (see
// InstanceExtensions.Scoped in ServiceRegistry.cs:23). The descriptor
// still ends up Singleton.
var registry = new ServiceRegistry();
registry.For<ILifetimeBridgeThing>().Use<LifetimeBridgeThing>().Scoped();

var descriptor = registry.LastOrDefault(d => d.ServiceType == typeof(ILifetimeBridgeThing));
descriptor.ShouldNotBeNull();
descriptor.Lifetime.ShouldBe(ServiceLifetime.Scoped);
}

[Fact]
public void For_Use_factory_Scoped_writes_Scoped_lifetime_to_descriptor()
{
// The lambda-factory overload — same bug. This is the exact shape
// that AMI uses to register IAsyncDocumentSession against RavenDB
// and that triggers captive sessions in Wolverine HTTP handlers.
var registry = new ServiceRegistry();
registry.For<ILifetimeBridgeThing>().Use(_ => new LifetimeBridgeThing()).Scoped();

var descriptor = registry.LastOrDefault(d => d.ServiceType == typeof(ILifetimeBridgeThing));
descriptor.ShouldNotBeNull();
descriptor.Lifetime.ShouldBe(ServiceLifetime.Scoped);
}

[Fact]
public void For_Add_writes_Transient_lifetime_to_descriptor()
{
// The multi-impl pipeline form. Same bridge path.
var registry = new ServiceRegistry();
registry.For<ILifetimeBridgeThing>().Add<LifetimeBridgeThing>();

var descriptor = registry.LastOrDefault(d => d.ServiceType == typeof(ILifetimeBridgeThing));
descriptor.ShouldNotBeNull();
descriptor.Lifetime.ShouldBe(ServiceLifetime.Transient);
}

// ------- Sanity: container resolution honours intent (the consolation) ---

[Fact]
public void Container_resolution_still_honours_intended_lifetime()
{
// Lamar's own container plan uses Instance.Lifetime correctly, so this
// passes today. Documenting it to make clear the bug is purely at the
// IServiceCollection-snapshot layer.
var container = new Container(x => x.For<ILifetimeBridgeThing>().Use<LifetimeBridgeThing>().Scoped());

using var scope1 = container.GetNestedContainer();
using var scope2 = container.GetNestedContainer();
var a1 = scope1.GetInstance<ILifetimeBridgeThing>();
var a2 = scope1.GetInstance<ILifetimeBridgeThing>();
var b1 = scope2.GetInstance<ILifetimeBridgeThing>();

a1.ShouldBeSameAs(a2); // same scope -> same instance
a1.ShouldNotBeSameAs(b1); // different scope -> different instance
}
}
3 changes: 2 additions & 1 deletion src/Lamar.Testing/IoC/ServiceRegistryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using Lamar.IoC;
using Lamar.IoC.Frames;
using Lamar.IoC.Instances;
using Lamar.Scanning.Conventions;
using Microsoft.Extensions.DependencyInjection;
using Shouldly;
using StructureMap.Testing.Widget;
Expand All @@ -22,7 +23,7 @@ public void can_add_custom_instance()

registry.For<IWidget>().Use(custom);

registry.Single().ImplementationInstance.ShouldBe(custom);
registry.Single().LamarInstance().ShouldBe(custom);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public void add_type_on_an_empty_set()

var widgetDescriptor = services.Single(x => x.ServiceType == typeof(IWidget));
widgetDescriptor.ServiceType.ShouldBe(typeof(IWidget));
widgetDescriptor.ImplementationInstance.As<Instance>().ImplementationType.ShouldBe(typeof(AWidget));
widgetDescriptor.LamarInstance().ImplementationType.ShouldBe(typeof(AWidget));
}

[Fact]
Expand All @@ -42,7 +42,7 @@ public void add_type_on_new_implementation_type()
services.AddType(typeof(IWidget), typeof(MoneyWidget));

services.Where(x => x.ServiceType == typeof(IWidget))
.Select(x => x.ImplementationInstance).OfType<Instance>().Select(x => x.ImplementationType)
.Select(x => x.LamarInstance()).Where(x => x != null).Select(x => x.ImplementationType)
.ShouldHaveTheSameElementsAs(typeof(AWidget), typeof(MoneyWidget));
}

Expand All @@ -58,7 +58,7 @@ public void do_not_add_twice()
services.AddType(typeof(IWidget), typeof(MoneyWidget));

services.Where(x => x.ServiceType == typeof(IWidget))
.Select(x => x.ImplementationInstance).OfType<Instance>().Select(x => x.ImplementationType)
.Select(x => x.LamarInstance()).Where(x => x != null).Select(x => x.ImplementationType)
.ShouldHaveTheSameElementsAs(typeof(AWidget), typeof(MoneyWidget));
}
}
5 changes: 3 additions & 2 deletions src/Lamar.Testing/ServiceRegistryTester.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Linq;
using Lamar.IoC.Instances;
using Lamar.Scanning.Conventions;
using Microsoft.Extensions.DependencyInjection;
using Shouldly;
using StructureMap.Testing.Widget;
Expand All @@ -17,7 +18,7 @@ public void for_use()

var descriptor = registry.Single();

var instance = descriptor.ImplementationInstance.ShouldBeOfType<ConstructorInstance<AWidget, IWidget>>();
var instance = descriptor.LamarInstance().ShouldBeOfType<ConstructorInstance<AWidget, IWidget>>();
instance.ImplementationType.ShouldBe(typeof(AWidget));

descriptor.ServiceType.ShouldBe(typeof(IWidget));
Expand All @@ -31,7 +32,7 @@ public void forsingleton_use()

var descriptor = registry.Single();

var instance = descriptor.ImplementationInstance.ShouldBeOfType<ConstructorInstance<AWidget, IWidget>>();
var instance = descriptor.LamarInstance().ShouldBeOfType<ConstructorInstance<AWidget, IWidget>>();
instance.ImplementationType.ShouldBe(typeof(AWidget));
instance.Lifetime.ShouldBe(ServiceLifetime.Singleton);

Expand Down
35 changes: 32 additions & 3 deletions src/Lamar/IoC/Instances/Instance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using JasperFx.Core;
using JasperFx.Core.Reflection;
using Lamar.IoC.Frames;
using Lamar.Scanning.Conventions;
using Microsoft.Extensions.DependencyInjection;

namespace Lamar.IoC.Instances;
Expand Down Expand Up @@ -34,13 +35,40 @@ protected Instance(Type serviceType, Type implementationType, ServiceLifetime li

public bool IsExplicitlyNamed { get; set; }

// Set when ServiceCollectionExtensions.Add wraps the Instance in a
// LamarServiceDescriptor. Together they let the Lifetime setter (below) keep the
// descriptor's surface-area Lifetime in sync with this Instance — needed because
// the MS-DI ServiceDescriptor is immutable, so any Lifetime change requires
// replacing the descriptor in the registry.
internal IServiceCollection ServiceCollection { get; set; }
internal LamarServiceDescriptor LamarDescriptor { get; set; }


public Type ServiceType { get; }
public Type ImplementationType { get; }

public int Hash { get; set; }

public ServiceLifetime Lifetime { get; set; } = ServiceLifetime.Transient;
private ServiceLifetime _lifetime = ServiceLifetime.Transient;

/// <summary>
/// Intended lifetime of this Instance. Mutating this value also refreshes
/// the backing LamarServiceDescriptor in the registry so IServiceCollection
/// consumers (Wolverine code-gen, MS-DI validation, etc.) see the current
/// lifetime. All Lifetime mutations — fluent .Scoped()/.Transient()/.Singleton(),
/// ReferencedInstance.createPlan's Lifetime = _inner.Lifetime, internal
/// adjustments — route through this single setter.
/// </summary>
public ServiceLifetime Lifetime
{
get => _lifetime;
set
{
if (_lifetime == value) return;
_lifetime = value;
this.ReplaceLamarDescriptor();
}
}

public string Name
{
Expand Down Expand Up @@ -147,9 +175,10 @@ public static Instance For(ServiceDescriptor service)
#endif


if (service.ImplementationInstance is Instance i)
var lamarInstance = service.LamarInstance();
if (lamarInstance != null)
{
return i;
return lamarInstance;
}

if (service.ImplementationInstance != null)
Expand Down
51 changes: 51 additions & 0 deletions src/Lamar/LamarServiceDescriptor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
using System;
using Lamar.IoC.Instances;
using Microsoft.Extensions.DependencyInjection;

namespace Lamar;

/// <summary>
/// ServiceDescriptor variant that carries a Lamar Instance alongside its
/// intended ServiceLifetime. This solves a long-standing mismatch: the
/// stock ServiceDescriptor(Type, object) constructor — which is the only
/// MS-DI overload that accepts a pre-existing object payload — hard-codes
/// Lifetime = Singleton, forcing every fluent registration (For<T>().Use<TImpl>(),
/// .Use(factory), .Add<TImpl>(), and their .Scoped()/.Transient() variants)
/// to appear as a Singleton to anything inspecting the IServiceCollection
/// snapshot. That's the root cause behind Wolverine HTTP code-gen capturing
/// resolved Lamar registrations as singleton fields on generated handler
/// classes; see JasperFx/wolverine#1610 and #537 for visible symptoms.
///
/// Descriptor shape: we delegate to the (Type, Func, ServiceLifetime) base
/// constructor, which makes the descriptor look like an "opaque lambda
/// factory" to external consumers. This is the shape Wolverine's docs say
/// triggers service-location at runtime — exactly what we want, because
/// service location calls back into Lamar's IServiceProvider, which can
/// see scan-only deps (whereas Wolverine's inline-construction codegen
/// cannot). The factory delegate itself is never invoked under Lamar
/// resolution: Lamar's <see cref="Instance.For" /> checks for the subclass
/// first and returns the typed Instance directly, bypassing the factory
/// branch. The factory body throws to make any incorrect direct invocation
/// loud and obvious.
/// </summary>
public sealed class LamarServiceDescriptor : ServiceDescriptor
{
public LamarServiceDescriptor(Instance instance)
: base(
instance.ServiceType,
FactoryShouldNotBeInvoked,
instance.Lifetime)
{
Instance = instance;
}

public Instance Instance { get; }

private static object FactoryShouldNotBeInvoked(IServiceProvider sp) =>
throw new InvalidOperationException(
"LamarServiceDescriptor's ImplementationFactory is a Wolverine/MS-DI codegen marker " +
"and should not be invoked directly. Resolution under Lamar goes through Instance.For " +
"(which prefers the typed Instance on the subclass) and never through this delegate. " +
"If you see this, you are resolving the descriptor outside a Lamar container — build " +
"the container via UseLamar(...) instead of stock MS-DI ServiceProvider.");
}
56 changes: 52 additions & 4 deletions src/Lamar/Scanning/Conventions/ServiceCollectionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,60 @@ public static ConnectedConcretions ConnectedConcretions(this IServiceCollection
}

/// <summary>
/// Add a registration via Lamar's intrinsic Instance type
/// Add a registration via Lamar's intrinsic Instance type.
///
/// Lamar smuggles its rich Instance metadata through ServiceDescriptor by
/// placing the Instance object in the ImplementationInstance slot. The
/// built-in MS-DI ServiceDescriptor(Type, object) constructor forces
/// Lifetime = Singleton, which lies to any IServiceCollection consumer
/// (Wolverine code-gen in particular) that reads descriptor.Lifetime to
/// decide how to source the dependency. We use a LamarServiceDescriptor
/// subclass so the descriptor carries the Lamar Instance AND the intended
/// lifetime; Lamar's own self-discovery treats it the same as a legacy
/// ImplementationInstance round-trip via the helper below.
/// </summary>
/// <param name="services"></param>
/// <param name="instance"></param>
public static void Add(this IServiceCollection services, Instance instance)
{
services.Add(new ServiceDescriptor(instance.ServiceType, instance));
var descriptor = new LamarServiceDescriptor(instance);
instance.ServiceCollection = services;
instance.LamarDescriptor = descriptor;
services.Add(descriptor);
}

/// <summary>
/// Called by <see cref="Instance.Lifetime" />'s setter when the lifetime changes.
/// Locates the Instance's existing LamarServiceDescriptor in the registry by
/// reference and replaces it with a fresh one reflecting current Instance state.
/// Matches by descriptor reference (not by ServiceType, which is what MS-DI's
/// own <c>IServiceCollection.Replace</c> extension does — wrong semantics for us
/// when multiple registrations share a ServiceType, e.g. <c>For&lt;T&gt;().Add&lt;&gt;()</c>
/// pipelines).
/// </summary>
internal static void ReplaceLamarDescriptor(this Instance instance)
{
var services = instance.ServiceCollection;
var current = instance.LamarDescriptor;
if (services == null || current == null) return;

var idx = services.IndexOf(current);
if (idx < 0) return;

var refreshed = new LamarServiceDescriptor(instance);
services[idx] = refreshed;
instance.LamarDescriptor = refreshed;
}

/// <summary>
/// Returns the Lamar Instance attached to <paramref name="descriptor" />, if any —
/// covering both the LamarServiceDescriptor subclass and the legacy
/// ImplementationInstance round-trip shape.
/// </summary>
internal static Instance LamarInstance(this ServiceDescriptor descriptor)
{
if (descriptor is LamarServiceDescriptor lsd) return lsd.Instance;
return descriptor.ImplementationInstance as Instance;
}

public static bool Matches(this ServiceDescriptor descriptor, Type serviceType, Type implementationType)
Expand All @@ -56,9 +103,10 @@ public static bool Matches(this ServiceDescriptor descriptor, Type serviceType,
return true;
}

if (descriptor.ImplementationInstance is Instance)
var lamarInstance = descriptor.LamarInstance();
if (lamarInstance != null)
{
return descriptor.ImplementationInstance.As<Instance>().ImplementationType == implementationType;
return lamarInstance.ImplementationType == implementationType;
}

return false;
Expand Down
Loading
Loading