Cyclical dependency detection false positive with open generic service

[halter73]

The following code will throw a Ninject.ActivationException in the call to Get claiming that a "A cyclical dependency was detected between the constructors of two services."

Even though IGeneric<> is resolved twice, there isn't a cyclical dependency because IGeneric<> is activated with two different type parameters which changes the type of its dependency.

class Program
{
    static void Main(string[] args)
    {
        var kernel = new StandardKernel();

        kernel.Bind(typeof(IGeneric<>)).To(typeof(Generic<>));

        // Uncomment the following line to avoid the ActivationException
        //kernel.Bind<IGeneric<IServiceA>>().To<Generic<ServiceA>>();

        kernel.Bind<IServiceA>().To<ServiceA>();
        kernel.Bind<IServiceB>().To<ServiceB>();

        var service = kernel.Get<IGeneric<IServiceA>>();
    }
}

interface IGeneric<out T>
{
}

interface IServiceA
{
}

interface IServiceB
{
}

class Generic<T> : IGeneric<T>
{
    public Generic(T foo)
    {
    }
}

class ServiceA : IServiceA
{
    public ServiceA(IGeneric<IServiceB> bar)
    {
    }
}

class ServiceB : IServiceB
{
}

As indicated by the comment in the Main method, you can work around this exception by registering a closed generic service (i.e. IGeneric<IServiceA> or IGeneric<IServiceB>). If you do this, the call to Get will return an instance of Generic<IServiceA> as expected.

I think this is a bug because the closed generic registration should be subsumed by the open generic registration.

The full exception message reads as follows:

Ninject.ActivationException: Error activating IGeneric{IServiceB} using binding from IGeneric{T} to Generic{T}
A cyclical dependency was detected between the constructors of two services.

Activation path:
  3) Injection of dependency IGeneric{IServiceB} into parameter bar of construct or of type ServiceA
  2) Injection of dependency IServiceA into parameter foo of constructor of type Generic{IServiceA}
  1) Request for IGeneric{IServiceA}

Suggestions:
  1) Ensure that you have not declared a dependency for IGeneric{IServiceB} on any implementations of the service.
  2) Consider combining the services into a single one to remove the cycle.
  3) Use property injection instead of constructor injection, and implement IInitializable
     if you need initialization logic to be run after property values have been injected.

   at Ninject.Activation.Context.Resolve()
   at Ninject.KernelBase.<>c__DisplayClass15.<Resolve>b__f(IBinding binding)
   at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.MoveNext()
   at System.Linq.Enumerable.SingleOrDefault[TSource](IEnumerable`1 source)
   at Ninject.Planning.Targets.Target`1.GetValue(Type service, IContext parent)
   at Ninject.Planning.Targets.Target`1.ResolveWithin(IContext parent)
   at Ninject.Activation.Providers.StandardProvider.GetValue(IContext context, ITarget target)
   at Ninject.Activation.Providers.StandardProvider.<>c__DisplayClass4.<Create>b__2(ITarget target)
   at System.Linq.Enumerable.WhereSelectArrayIterator`2.MoveNext()
   at System.Linq.Buffer`1..ctor(IEnumerable`1 source)
   at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
   at Ninject.Activation.Providers.StandardProvider.Create(IContext context)
   at Ninject.Activation.Context.ResolveInternal(Object scope)
   at Ninject.Activation.Context.Resolve()
   at Ninject.KernelBase.<>c__DisplayClass15.<Resolve>b__f(IBinding binding)
   at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.MoveNext()
   at System.Linq.Enumerable.SingleOrDefault[TSource](IEnumerable`1 source)
   at Ninject.Planning.Targets.Target`1.GetValue(Type service, IContext parent)
   at Ninject.Planning.Targets.Target`1.ResolveWithin(IContext parent)
   at Ninject.Activation.Providers.StandardProvider.GetValue(IContext context, ITarget target)
   at Ninject.Activation.Providers.StandardProvider.<>c__DisplayClass4.<Create>b__2(ITarget target)
   at System.Linq.Enumerable.WhereSelectArrayIterator`2.MoveNext()
   at System.Linq.Buffer`1..ctor(IEnumerable`1 source)
   at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
   at Ninject.Activation.Providers.StandardProvider.Create(IContext context)
   at Ninject.Activation.Context.ResolveInternal(Object scope)
   at Ninject.Activation.Context.Resolve()
   at Ninject.KernelBase.<>c__DisplayClass15.<Resolve>b__f(IBinding binding)
   at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.MoveNext()
   at System.Linq.Enumerable.<CastIterator>d__b1`1.MoveNext()
   at System.Linq.Enumerable.Single[TSource](IEnumerable`1 source)
   at Ninject.ResolutionExtensions.Get[T](IResolutionRoot root, IParameter[] parameters)
   at NinjectIssue.Program.Main(String[] args) in Program.cs:line 21

[iappert]

@remogloor I implemented a fix for this issue. Could you review my changes? Link to commit: iappert@1d0121b

[danielmarbach]

Could you guys align on line endings and spacing or seperate changes from code formatting? Kinda hard to follow what changed otherwise

Am 04.09.2014 um 07:13 schrieb Ivan Appert [email protected]:

@remogloor I implemented a fix for this issue. Could you review my changes? Link to commit: iappert/Ninject@1d0121b

—
Reply to this email directly or view it on GitHub.

[iappert]

Sorry for that. Habbits...

I added the following test:

    [Fact]
    public void OpenGenericBindingsWithDifferentGenericParametersCanBeAggregated()
    {
        kernel.Bind(typeof(IGeneric<>)).To(typeof(Bla<>));
        kernel.Bind<IServiceA>().To<ServiceA>();
        kernel.Bind<IServiceB>().To<ServiceB>();

        var service = kernel.Get<IGeneric<IServiceA>>();

        service.Should().NotBeNull();
    }

[danielmarbach]

Just make your ctrl+e+f spree in seperate commit ;)

Am 04.09.2014 um 08:07 schrieb Ivan Appert [email protected]:

Sorry for that. Habbits...

I added the following test:

[Fact]
public void OpenGenericBindingsWithDifferentGenericParametersCanBeAggregated()
{
    kernel.Bind(typeof(IGeneric<>)).To(typeof(Bla<>));
    kernel.Bind<IServiceA>().To<ServiceA>();
    kernel.Bind<IServiceB>().To<ServiceB>();

    var service = kernel.Get<IGeneric<IServiceA>>();

    service.Should().NotBeNull();
}

—
Reply to this email directly or view it on GitHub.

[remogloor]

@iappert
I haven't verified but i think you will now ignore circular dependencies for generics completely. e.g.

A -> Foo<int> -> A 

will lead to a stack overflow instead of an activation exception.

[iappert]

@remogloor
In my opinion, it still detects circular dependencies since

  A -> Foo<B> -> B -> A

or

Foo<A> -> A -> B -> Foo<A> -> 

result in a circular dependency of A.
I added some additional tests for circular dependency detection, check out commit iappert@2cce594

Without my change, the example of @halter73 leads to a circular dependency because of the open generic binding, i.e.

 IGeneric<IServiceA> -> ServiceA -> IGeneric<IServiceB> -> ServiceB

Resolving IGeneric<IServiceB> throws an activation exception because the binding IGeneric<> has already been used for resolving IGeneric<IServiceA>.

[remogloor]

This will get you a stackoverflow exception:

    [Fact]
    public void DetectsCyclicDependenciesForGenericServiceRegisteredViaOpenGenericType2()
    {
        kernel.Bind(typeof(IGeneric<>)).To(typeof(GenericServiceWithGenericConstructor<>));

        Action act = () => kernel.Get<IGeneric<int>>();

        act.ShouldThrow<ActivationException>();
    }

    public class GenericServiceWithGenericConstructor<T> : IGeneric<T>
    {
        public GenericServiceWithGenericConstructor(IGeneric<T> arg)
        {
        }
    }

[iappert]

Agreed.

I haven't thought about this case. I will try to find another solution:-). thanks for your help.

[yishaigalatzer]

Any update on the status for this?

[yishaigalatzer]

We have made a fix where we don't register some of the services as open generics. The issue in ninject still exists and will break user code (or potentially other framework code)

[jupaol]

Any update on this?