-
-
Notifications
You must be signed in to change notification settings - Fork 120
Description
Description
GetAllInstances<IInterface>() returns implementations that were not registered for IInterface. When a concrete type is registered (e.g., container.Register<MyClass>()), it incorrectly appears in GetAllInstances<IMyInterface>() results even though MyClass was never registered as IMyInterface.
LightInject Version
- Tested on latest master branch (commit ae33c4b)
- Likely affects all recent versions
Minimal Reproduction
using LightInject;
using System;
using System.Linq;
public interface IMyInterface { }
public class MyClass : IMyInterface { }
class Program
{
static void Main()
{
var container = new ServiceContainer();
// Register ONLY as concrete type, NOT as interface
container.Register<MyClass>(new PerContainerLifetime());
// This should return EMPTY - nothing was registered for IMyInterface
var instances = container.GetAllInstances<IMyInterface>().ToArray();
Console.WriteLine($"Count: {instances.Length}");
// ACTUAL: Count: 1 (MyClass is returned)
// EXPECTED: Count: 0 (nothing registered for IMyInterface)
}
}Expected Behavior
GetAllInstances<IMyInterface>() should return empty because nothing was explicitly registered for IMyInterface.
To get MyClass via IMyInterface, it should be registered as:
container.Register<IMyInterface, MyClass>();Actual Behavior
GetAllInstances<IMyInterface>() returns MyClass even though it was only registered as container.Register<MyClass>().
Impact
This causes real problems in ASP.NET Web API integration:
// Our exception logger registered as concrete type
container.Register<UnhandledExceptionLogger>(new PerContainerLifetime());
// Also added to Web API services
httpConfiguration.Services.Add(typeof(IExceptionLogger), container.GetInstance<UnhandledExceptionLogger>());
// Later, DefaultServices.GetServices() calls:
// - DependencyResolver.GetServices(typeof(IExceptionLogger)) - returns UnhandledExceptionLogger (BUG!)
// - Plus the one we added via Services.Add()
// Result: Logger appears TWICE!Root Cause
In ServiceContainer.CreateEmitMethodForEnumerableServiceServiceRequest(), the variance check uses IsAssignableFrom too broadly:
// Current code (line ~4858):
emitMethods = emitters
.Where(kv => actualServiceType.GetTypeInfo().IsAssignableFrom(kv.Key.GetTypeInfo()))
// ...This matches ANY type that implements the interface, not just types with proper generic variance relationship.
Proposed Fix
Replace the IsAssignableFrom check with a more precise IsVariantMatch() method that only matches:
- Exact type matches - always include
- Interface inheritance - both types are interfaces (e.g.,
ICollection : IEnumerable) - Class inheritance - requested type is a class and registered type derives from it
- Generic variance - same generic type definition with compatible type arguments
It should exclude concrete classes that implement an interface but were not registered as that interface.
private static bool IsVariantMatch(Type requestedType, Type registeredType)
{
if (requestedType == registeredType)
return true;
if (!requestedType.GetTypeInfo().IsAssignableFrom(registeredType.GetTypeInfo()))
return false;
var requestedTypeInfo = requestedType.GetTypeInfo();
var registeredTypeInfo = registeredType.GetTypeInfo();
// Interface inheritance
if (requestedTypeInfo.IsInterface && registeredTypeInfo.IsInterface)
return true;
// Class inheritance
if (requestedTypeInfo.IsClass && registeredTypeInfo.IsClass)
return true;
// Generic variance
if (requestedTypeInfo.IsGenericType && registeredTypeInfo.IsGenericType)
{
if (requestedType.GetGenericTypeDefinition() == registeredType.GetGenericTypeDefinition())
return true;
}
// Concrete class implementing interface - NOT a variant match
return false;
}Then update the variance checks to use IsVariantMatch() instead of IsAssignableFrom().
Test Case
[Fact]
public void GetAllInstances_ShouldReturnEmpty_WhenConcreteTypeNotRegisteredForInterface()
{
var container = new ServiceContainer();
container.Register<MyClass>(new PerContainerLifetime());
var instances = container.GetAllInstances<IMyInterface>().ToArray();
Assert.Empty(instances); // Currently fails - returns MyClass
}
public interface IMyInterface { }
public class MyClass : IMyInterface { }Note on Existing Tests
The test Issue257.ShouldHandleMoreThanNineImplementationsForSameInterface relies on this buggy behavior - it registers concrete types and expects them to be returned via interface. This test should be updated to use proper registration:
// Instead of:
container.Register<Foo0>();
// Should be:
container.Register<IFoo, Foo0>("Foo0");Workaround
Until fixed, call GetAllInstances for interfaces before resolving any concrete types:
// "Warm up" - caches empty result before concrete types are discovered
_ = container.GetAllInstances<IExceptionLogger>();
// Now resolve concrete type
var logger = container.GetInstance<UnhandledExceptionLogger>();However, this is fragile and depends on call order.
p.s. description is generated by my AI assistant