z4kn4fein / stashbox

A lightweight, fast, and portable dependency injection framework for .NET-based solutions.
https://z4kn4fein.github.io/stashbox
MIT License
140 stars 10 forks source link

Child container resources are never released #142

Closed eValker closed 1 year ago

eValker commented 1 year ago

Hi, while playing with the library I found an unexpected behavior: resources of child containers are never released and they are being keep in the service's memory even after being disposed.

All problematic resources are being referenced in disposables field of the ResolutionScope class.

I prepared sample code that reproduces the issue.

using Stashbox;
using Stashbox.Configuration;

var container = new StashboxContainer(c =>
{
    c.WithRegistrationBehavior(Rules.RegistrationBehavior.PreserveDuplications);
});

while (true)
{
    var scope = container.CreateChildContainer();

    RegisterToContainer(container);

    var x = scope.Resolve<IAnimalRepository>();

    GC.KeepAlive(x);

    scope.Dispose();

    GC.Collect();
    GC.WaitForPendingFinalizers();
    GC.Collect();
}

Console.ReadKey();

static void RegisterToContainer(IStashboxContainer container)
{
    container.Register<AnimalRepository>(c => c.WithScopedLifetime().AsImplementedTypes());

    //another trial
    container.RegisterDecorator<CachedAnimalRepository>(c => c.AsImplementedTypes().When(t => t.Type.IsAssignableTo(typeof(IAnimalRepository))));
}

internal interface IDataRepository : IDisposable
{
    void CommonMethod()
    {
    }

    void IDisposable.Dispose()
    {
    }
}

internal interface IAnimalRepository : IDataRepository
{
}

internal sealed class AnimalRepository : IAnimalRepository
{
}

internal sealed class CachedAnimalRepository : IAnimalRepository
{
    public CachedAnimalRepository(IAnimalRepository service)
    {
    }
}

After a while we have hundreds of the AnimaRepository instances in the memory: image

For testing reasons I have cleaned manually disposables field of a main container using a VS debugger and after that all disposed resources were collected by GC.

I cannot reproduce this issue while using scopes instead of child containers.

It would be nice to have it fixed because this may lead to memory leak :)

schuettecarsten commented 1 year ago

I wonder why you do the registration on the main container and not on the child container (scope).

z4kn4fein commented 1 year ago

Hi @eValker, thank you for reaching out!

As @schuettecarsten pointed out, the issue with your test is that in your loop you register the services into the main container, which never gets disposed. You dispose only the child container that doesn't contain any services, so it won't dispose anything.

The test would be correct if you pass the child container to the registration method like: RegisterToContainer(scope);.

Let me know what you think!

eValker commented 1 year ago

Hello, Sure, you are both right :) I meant to do registrations on the child container, but I was testing different things and I accidentally pasted wrong sample code. However it does not change anything - issue remains the same - child containers are not being properly released. Main root scope (on the main container) keeps references to the child containers and to all services instances inside those containers. Because of that GC cannot collect them, so they remains in the memory.

Changed sample code:

using Stashbox;
using Stashbox.Configuration;

var container = new StashboxContainer(c =>
{
    c.WithRegistrationBehavior(Rules.RegistrationBehavior.PreserveDuplications);
});

while (true)
{
    var childContainer = container.CreateChildContainer();

    RegisterToContainer(childContainer);

    var x = childContainer.Resolve<IAnimalRepository>();

    GC.KeepAlive(x);

    childContainer.Dispose();

    GC.Collect();
    GC.WaitForPendingFinalizers();
    GC.Collect();
}

Console.ReadKey();

static void RegisterToContainer(IStashboxContainer container)
{
    container.Register<AnimalRepository>(c => c.WithSingletonLifetime().AsImplementedTypes());

    //another trial
    container.RegisterDecorator<CachedAnimalRepository>(c => c.AsImplementedTypes().When(t => t.Type.IsAssignableTo(typeof(IAnimalRepository))));
}

internal interface IDataRepository : IDisposable
{
    void CommonMethod()
    {
    }

    void IDisposable.Dispose()
    {
    }
}

internal interface IAnimalRepository : IDataRepository
{
}

internal sealed class AnimalRepository : IAnimalRepository
{
}

internal sealed class CachedAnimalRepository : IAnimalRepository
{
    public CachedAnimalRepository(IAnimalRepository service)
    {
    }
}

And memory state after a while of running: image image

As you can see, even after closing and disposing child container, we have over 2k instances of the AnimalRepository in the memory. Proper behavior IMO is that we should have single instance of that service.

z4kn4fein commented 1 year ago

Ah, I see the issue now. Somehow, my brain skipped your explanation about the disposables (sorry for that), and yes, that field is the bad guy here. Thank you for pointing it out! I'm going to work on the fix.

z4kn4fein commented 1 year ago

@eValker I've released a new version v5.11.1 with the fix, could you please check that it works now at your end as expected? Thanks!

eValker commented 1 year ago

Thank you @z4kn4fein for the fast fix :) I can confirm that now child containers are properly released.

image

image

image