simpleinjector / SimpleInjector

An easy, flexible, and fast Dependency Injection library that promotes best practice to steer developers towards the pit of success.
https://simpleinjector.org
MIT License
1.21k stars 155 forks source link

HybridLifestyle: fallbackLifestyle instance is created/initialized together with defaultLifestyle instance #975

Closed FDUdannychen closed 1 year ago

FDUdannychen commented 1 year ago

Here is a minimal test case:

interface IFoo { int Value { get; set; } }
class Foo : IFoo { public int Value { get; set; } }

[Fact]
public void HybridLifestyle()
{
    var container = new Container();
    container.Options.DefaultLifestyle = Lifestyle.Transient;
    container.Options.DefaultScopedLifestyle = new AsyncScopedLifestyle();

    container.Register<IFoo, Foo>(Lifestyle.CreateHybrid(Lifestyle.Scoped, Lifestyle.Singleton));

    container.RegisterInitializer<IFoo>(x =>
    {
        var scope = Lifestyle.Scoped.GetCurrentScope(container);

        if (scope != null && scope.GetItem("ConfigValue") is int v)
        {
            x.Value = v;
        }
    });

    IFoo foo1, foo2;

    using (var scope = AsyncScopedLifestyle.BeginScope(container))
    {
        scope.SetItem("ConfigValue", 1);
        foo1 = container.GetInstance<IFoo>();
    }

    foo2 = container.GetInstance<IFoo>();
    var same = ReferenceEquals(foo1, foo2); 
}

Finally, I got same=false that looks correct, but both foo1.Value and foo2.Value equal to 1. This is because the singleton instance is initialized together with the scope instance, so that they both have Foo.Value assigned by the initializer. If the singleton instance foo2 is initialized before scope instance foo1, I got foo1.Value=1 and foo2.Value=0, which makes more sense.

The root cause here is "if an object has a hybrid lifestyle, should the fallbackLifestyle instance be created/initialized together with defaultLifestyle instance". The answer for current design is "yes", while personally I think it makes more sense if they are created/initialized on demand.

dotnetjunkie commented 1 year ago

The behavior you are experiencing has nothing to do with the hybrid lifestyle but rather with how Simple Injector handles singletons. Singleton instances are constructed together with their compiled expression trees, as the final expression tree of a singleton registration is a ConstantExpression, which refers to a pre-initialized instance—that singleton instance.

Upon compilation, a hybrid lifestyle registration takes the expression trees of both underlying registrations and wraps it in a ConditionalExpression. But since in your case one of the lifestyles is a singleton, that ConditionalExpression wraps the ConstantExpression that references the initialized singleton instance.

This means that, even if the singleton is not resolved, it has to be initialized even if the compiled condition doesn't hit the singleton case.

By the way, I hope that your Lifestyle.CreateHybrid(Lifestyle.Scoped, Lifestyle.Singleton) lifestyle is just something you use for testing this scenario, because I'd say that using this in your production application is not something I would typically advice. What you are basically simulating here is behavior similar to that of Autofac, where a scoped instance is resolved from the container's 'root scope' in case there is no active scope. Simple Injector very explicitly doesn't allow this, because this can easily lead to bugs, such as multi-threading issues.

If you wish, can you elaborate on your specific case why you need this? Perhaps I can feedback on your design and provide better alternatives either for your design or for the Simple Injector registrations.

FDUdannychen commented 1 year ago

@dotnetjunkie I agree the design is probably not a good idea, my purpose is to refactor a legacy system, keeping existed configurations available. Each configuration file has two parts: the shared configs (defined in the top of the config file), and the per-job configs, something like this:

<Config Name="Config1" Value="Value1" />
<Config Name="Config2" Value="Value2" />
<Job1>
    <Config Name="Config1" Value="OverrideValue1" />
    <Config Name="Config3" Value="Value3" />
</Job1>
<Job2>...

In the application there is a ConfigReader : IConfigReader, in a job's context (there is a SimpleInjector scope), the ConfigReader is resolved to a scoped instance with corresponding configurations. If there is no job context, the ConfigReader is resolved to a singleton instance that only reads shared configs. That's why I want a hybrid lifestyle.

My original design is a ConfigReaderFactory where you can create ConfigReader from different configuration pieces. It works fine but due to some legacy code, I need to pass configurations deep through the method calls to make ConfigReaderFactory work, that's annoying. I want a ConfigReader that can be used anywhere in the code without worrying about configs anymore.

Any suggestion about this? Thanks.

dotnetjunkie commented 1 year ago

I'm sorry for my late response. I find it difficult to suggest anything here, as anything I'd suggest would probably be difficult to implement in your legacy system. I would suggest finding a solution that doesn't involve using hybrid lifestyles, for instance having a separate IJobConfigReader abstraction and implementation registered scoped, with the 'core' reader being the IConfigReader being registered as singleton, but that likely requires changing the way ConfigReader currently works, or how jobs are provided with a specific version, which might be too much to chew off for you.