Skip to content

GetAllInstances returns unexpected results after GetInstance for concrete type #610

@SpiridonovAlexMoeDelo

Description

@SpiridonovAlexMoeDelo

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:

  1. Exact type matches - always include
  2. Interface inheritance - both types are interfaces (e.g., ICollection : IEnumerable)
  3. Class inheritance - requested type is a class and registered type derives from it
  4. 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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions