Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Child container resources are never released #142

Closed
eValker opened this issue Aug 11, 2023 · 6 comments
Closed

Child container resources are never released #142

eValker opened this issue Aug 11, 2023 · 6 comments

Comments

@eValker
Copy link

eValker commented Aug 11, 2023

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
Copy link
Contributor

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

@z4kn4fein
Copy link
Owner

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
Copy link
Author

eValker commented Aug 16, 2023

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
Copy link
Owner

z4kn4fein commented Aug 16, 2023

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
Copy link
Owner

@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
Copy link
Author

eValker commented Aug 17, 2023

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

image

image

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants