RegisterConstructorDependency issue with lifetimes

[zpqrtbnk]

The following test fails (latest version) because the args collection which is passed to the constructor dependency is empty. Can trace it to the constructor dependency being invoked during CreateDefaultDelegate i.e. before the delegate is even invoked, and at a moment arguments are not available.

var container = new ServiceContainer();
container.Register<IThing, Thing>(new PerContainerLifetime());
container.RegisterConstructorDependency((factory, info, args) 
    => args.Length == 0 ? ThingDep.BadDep : args[0] as ThingDep;
var thing = (IThing) container.GetInstance(typeof(IThing), new object[] { new ThingDep() });
Assert.IsNotNull(thing);
Assert.AreNotSame(ThingDep.BadDep, thing.Dep); // FAILS!

Is this a bug or a feature?

[seesharper]

Could you provide a full sample of what you are trying to do?

https://github.com/seesharper/LightInject/blob/master/contributing.md

[zpqrtbnk]

Sure. What I am trying to do: register & use a constructor dependency. I have defined the following elements:

public class ThingDep
{
  public static ThingDep BadDep { get; } = new ThingDep();
}

public interface IThing
{
  ThingDep Dep { get; }
}

public class Thing : IThing
{
  public Thing(ThingDep dep)
  {
    if (dep == null) throw new ArgumentNullException(nameof(dep));
    Dep = dep;
  }

  public ThingDep Dep { get; }
}

And successfully running the following test:

var container = new ServiceContainer();

container.Register<IThing, Thing>();

container.RegisterConstructorDependency((factory, info, args) 
  => args.Length == 0 ? ThingDep.BadDep : args[0] as ThingDep);

var thingDep = new ThingDep();
var thing = container.GetInstance<ThingDep, IThing>(thingDep);

Assert.AreSame(thing.Dep, thingDep);

Which is expected, if I understand the constructor dependency registration correctly. I am using this in an application to get transient instances of "things" which depend on a dependency that I supply, and others that are supplied by the container. Works.

Then I modified one single thing in the test: registering per-container:

container.Register<IThing, Thing>(new PerContainerLifetime());

Expected: test successful. Result: test fails.

I traced it to args being empty in the constructor dependency function, because in per-container mode, the constructor dependency is invoked during CreateDefaultDelegate i.e. before the delegate is even invoked, and at a moment arguments are not available.

It may be that constructor dependencies are not supposed to be used with per-container, but I could not find it documented anywhere.

Making more sense?

[seesharper]

It makes perfekt sense. Will look into this.

[zpqrtbnk]

Cool ;-) In fact I can see in the code that PerContainerLifetime triggers some interesting optimizations in the container, i.e. it's not always treated as a "standard" lifetime, but this can have a few (probably) unintended consequences...

[seesharper]

Good catch :) I am not sure if removing that optimization would get us out of the woods, but I will take a look at that as well.

We are actually touching in on something that is related to this very issue.

Consider this code:

container.Register<int, IFoo>((factory, value) => new FooWithOneParameter(value));
var instance = container.GetInstance<int, IFoo>(42);

The register method that lets you define runtime arguments does not allow to specify a lifetime. If we should decide to allow lifetimes for these registrations, the arguments array would probably be passed down to the actual lifetime implementation so that it could use the actual argument values to determine if a new instance should be created. This is something I have considered more than once, but left it alone mainly because of performance reasons. There might be a clever way though to make it work without hurting performance.

[replaysMike]

Was anything done about this issue? I just ran into the same exact scenario. I can probably work around it by structuring my app differently, but here's the consideration:

• I have a factory that produces a single instance (PerContainerLifetime) of a FactoryCreatedClass
• Each FactoryCreatedClass takes in a Config object, which is populated based on the type of class in the factory
• That configuration is injected on the first call to GetInstance<Config, FactoryCreatedClass>(config)

Now I can get around this by either passing all configurations for every class and register it normally for DI, or have a specific configuration class for each FactoryCreatedClass. But it would sure be nice if dependencies worked as they do when not specifying lifetime.

[seesharper]

I think you would be best off by working around injecting runtime data into your services. In fact, that is one of the few things I actually regret implementing. I have never used it myself and it is sort of an anti-pattern depending on how you see it 😄 I highly recommend reading this post by @dotnetjunkie that discusses this in more detail. He is also the coauthor of Dependency Injection in .Net.

[zpqrtbnk]

FWIW we (original poster) are moving away from using RegisterConstructorDependency for other solutions. In fact, using it with per-container instances is ... weirdish: you could call GetInstance<Config, FactoryCreatedClass>(config) twice, with two different config instances, and then what happens?

Probably, this does not need to be fixed, but documented ;-)