2
Vote

Injected IEnumerable<T> instances blow up when used in LINQ queries

description

Okay, bear with me on this one, it's hard to explain clearly but I'll try my best.
 
In the following scenario:
  1. Multiple bindings to the same interface exist
  2. The bindings are not transient (they are cached somehow, whether it's per-request or singleton or other)
  3. There are no cyclical dependencies
  4. A class is injected with an IEnumerable<T> of that type
  5. The class executes LINQ queries on that injected enumerable to, for example, separate it out into "categories" of instances
     
    Then: an ActivationException is thrown stating that cyclical dependencies are detected.
     
    Cause:
    Because Ninject uses LINQ to build up the resolved types, it is delay executed. When I execute "more LINQ" on that same list and then finally reference it, the LINQ infrastructure ends up executing the original LINQ query multiple times over the entire list. This isn't bad, except that an execution path inside Ninject.Activation.Context.Resolve() can return the resolved instance (if it was cached) without popping the requested binding off the "ActiveBindings" stack.
    It takes 3 or more "sub-queries" to make this happen because:
  6. First time through everything resolves correctly, instances are created and put into the cache, and the "ActiveBindings" stack is pushed/popped correctly.
  7. Second time through, the instances are pulled out of the cache and the ActiveBindings stack is left in a bad state (not popped correctly), but things still return correctly.
  8. Third (and more) time through the sanity check against the "ActiveBindings" stack fails (because of the previous iteration's bug) and the exception is thrown.
     
    Workaround:
    Instead of executing further LINQ queries on the IEnumerable<T> given to me by Ninject, I can call ".ToList()" on it to execute the LINQ, and then do further queries on that list. By then, the Ninject LINQ only executes once and resolves all the types correctly only once.
     
    Possible solutions (need advice from those more familiar with the internals of Ninject):
    Choice 1:
    In /Activation/Context.cs, the "public object Resolve()" method could be changed from
    if (reference.Instance != null)
    return reference.Instance
     
    to
     
    if (reference.Instance != null)
    {
    Request.ActiveBindings.Pop();
    return reference.Instance;
    }
     
    This would pop the binding off the "ActiveBindings" stack when it was returned from the cache, and would prevent the stack from getting into a bad state.
    Note that this solution wouldn't address or fix the "side note" I have at the bottom of this report.
     
    Choice 2:
    Before Ninject injects the IEnumerable<T>, IList<T>, or ICollection<T>, it could "execute" the results of the LINQ query, so the incoming values to the class would be resolved at that moment and not wait until I used them within the class itself, if I did more LINQ querying on the list. It's potentially slightly slower this way, but the net gain would be worth it, I think. Especially given that this would also resolve the "side note" below.
     
    Choice 3:
    Do both 1 and 2. :) This would probably clean it up the most, fixing both "bugs".
     
    Example code for reproduction is attached. Create a new console app and use the file attached.
     
    SIDE NOTE:
    A side note, if I declare the bindings as transient, then this works, but only because they are never cached. Which also means that for a given input enumerable it will create a new instance of the class each time I execute a LINQ query on it. So where I would normally expect the incoming IEnumerable<T> to contain a set, it would actually create potentially many extra instances.

file attachments

comments