5

Memory leak when the, no objects gets disposed in the weakreferencecollection inside ninject, what am I doing wrong or is there a huge error in ninject?

When Kernal gets disposed then all the references gets disposed, from the weakreference collection, but when in this loop, even when there is no reference from code - the memory just explodes.

public class Program
{
    private StandardKernel kernel;

    private static void Main(string[] args)
    {
        new Program().Run();
    }

    public void Run()
    {
        kernel = new StandardKernel(new NinjectSettings() { LoadExtensions = false });
        kernel.Bind<Root>().ToSelf();
        kernel.Bind<Foo>().ToSelf().InCallScope();

        while (true)
        {
            Process();

            Thread.Sleep(500);

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

    public void Process()
    {
        Root root = kernel.Get<Root>();
        Root root2 = kernel.Get<Root>();
    }

    public class Root
    {
        private Foo _test;

        public Root(Foo foofac)
        {
            Id = Guid.NewGuid();
            _test = foofac;
        }

        protected Guid Id { get; set; }
    }

    public class Foo
    {
    }
}

Update

Ive tried with Named scope and even using a Factory like in the excample: http://www.planetgeek.ch/2012/04/23/future-of-activation-blocks/ but still in a memoryprofiler the weakreference collection is exploding over time, and this is not nice... My current test code is:

public class Program
{
    private StandardKernel kernel;

    private static void Main(string[] args)
    {
        new Program().Run();
    }

    public void Run()
    {
        kernel = new StandardKernel(new NinjectSettings() { LoadExtensions = false });
        kernel.Load<FuncModule>();

        kernel.Bind<IRootFactory>().ToFactory();
        var scopeParameterName = "scopeRoot";
        kernel.Bind<Root>().ToSelf().DefinesNamedScope(scopeParameterName);
        kernel.Bind<Foo>().ToSelf().InNamedScope(scopeParameterName);
        

        while (true)
        {
            Process();

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

            Thread.Sleep(500);
        }
    }

    public void Process()
    {
        Root root = kernel.Get<IRootFactory>().CreateRoot();
        Root root2 = kernel.Get<IRootFactory>().CreateRoot();
    }

    public class Root
    {
        private Foo _test;

        public Root(Foo foofac)
        {
            Id = Guid.NewGuid();
            _test = foofac;
        }

        protected Guid Id { get; set; }
    }

    public class Foo
    {
    }

    public  interface IRootFactory
    {
        Root CreateRoot();
    }
}
Community
  • 1
  • 1
eble
  • 88
  • 1
  • 6

2 Answers2

1

It takes time and is not intended to be deterministic - see the impl code for reality and the Cache and Collect article for the spirit.

For a start in your example you should be waiting to wait for the Ninject Releasing mechanism to get triggered (off a timer IIRC). One thing it def does not do is synchronously Dispose and Release the second a GC happens.

Bottom line is you're better off doing the Release deterministically via IResolutionRoot.Release or named scopes allied to InRequestScope.

Community
  • 1
  • 1
Ruben Bartelink
  • 59,778
  • 26
  • 187
  • 249
  • And there is no difference in using named scopes instead of call scope, in this senario – eble May 28 '13 at 10:40
  • @eble Is it on `IKernel` ? Bottom line is, it *is* there and it it what you want. And with the current stable NuGets, Release of a root object will also release other objects created as part of the same compose. The problem is that this relies on a bug (see a recent answer by me in the Ninject tag). Hence the cleanest (and future proof) is to CreateNamedScope() and Dispose it to deterministically clear things. I have a[n unreleased] stack of tests for the CreateNamedScope stuff - it def works - Trust me, I've spent a little time on this! – Ruben Bartelink May 28 '13 at 12:06
  • @eble Just in case it isnt clear - Ninject definitely makes **no** guarantee to Dispose or release on any schedule except via a) Disposing a Named Scope b) a `Release` call c) the wiring that makes lets people register Request Processors and then hook in a call to clear for a request (aka InRequestScope as shown in my other answer) – Ruben Bartelink May 28 '13 at 12:09
  • Okay, but it must be an error in Ninject, that after im done with the Root as a Context its not disposed, evenn the GarbageCollectionCachePruner(thats called every 30 sec) in ninject can't dispose the weakreferences because somehow they are alive, so some where they are still referenced. Im thinking of using the BeginBlock method, witch seems to do the work - but then in my code I always need make a UnitOfWork thing, and then theres more work as intended, just to make sure ninject releases them, witch should have been automatically when the context object is not referenced anymore! – eble May 28 '13 at 12:28
  • @eble The vagaries of when the nondeterministic Disposal happens, I don't much care for - as far as I'm concerned it was just a silly idea - e.g. WCF and IIS contexts dont have correct Memory Pressure/caching to use as scopes. [Don't use Activation Blocks - they are deprecated](http://www.planetgeek.ch/2012/04/23/future-of-activation-blocks/). Seriously, just go with my CreateNamedScope a la my other answer - it's tested, proven and not deprecated. The key aspects of that code were a) pseudocoded b) reviewed by Remo Gloor. And I have it in a production rig that screams loudly if it doesnt work – Ruben Bartelink May 28 '13 at 13:34
  • do you have the tests for CreateNamedScope stuff that I can check out? Thanks for the greate replies, sounds like you have tried some things and learned :) – eble May 28 '13 at 13:49
  • Look at my updated code, here I used Named Scope and a factory as in the example you gave, this still don't clean up. – eble May 28 '13 at 15:04
  • @eble Sorry GTG until later. You are not a) Creating b) Disposing a Named Scope in your code. Come back when you're doing that 1000 times in a loop and seeing 1000 not Disposed reachable items. Will try to dig out tests but dont have direct unit tests for it itself (they will eventually be part of a Ninject Pull Request in fantasu land) – Ruben Bartelink May 28 '13 at 16:27
  • Well I see this as an big issue with ninject, then using Named scope and the root object, where you DefinesNamedScope and that object is created, it should automaticly call CreateNamedScope and then dispose it self. Now when I have a service, that runs infinite, I have to somewhere get the IKernel, Create and Dispose the named scope inside my logic, and not just set the scopes up in a bootstrapper... This is not what I want – eble May 29 '13 at 08:35
  • @eble You can view stuff whatever way you like, the key things are: DefinesNamedScope and stuff are about scoping. If someoene isnt doing explicit `Release`s or `Dispose`ing Scopes, random things happen. I know that relying on said random things didnt work for me, so I use the following Scopes:- Transient: no leaks Singleton: no leaks, goes away when Kernel is Disposed Request: no leaks, goes away at end of request (either via `EndRequest` or my `using( ... CreateNamedScope()`). Before that I relied on [WCF] `InScope(OperationContext.Current)` with no cleanup - banng, leaks – Ruben Bartelink May 29 '13 at 15:36
  • @eble I am not surprised neither of your code fragments work the way you want and I would not use them. My proposed approaches a) work for fundamental reasons b) have been for years c) are used by others intentionally – Ruben Bartelink May 29 '13 at 15:38
  • @eble Here's a gist illustrating the learnings and verifying my impl (https://gist.github.com/bartelink/5671464) – Ruben Bartelink May 29 '13 at 16:06
1

Not that I recommend using it, but GC.Collect() is an asynchronous mechanism. Calling it only triggers a GC to occur, it does not wait for it to finish. You would probably want to call GC.WaitForPendingFinalizers() if you were that concerned about it.

Erik Funkenbusch
  • 92,674
  • 28
  • 195
  • 291
  • Your right, still have a problem even with WaitForPendingFinalizers – eble May 28 '13 at 10:26
  • @eble For clarity, The GC etc. will turn an internal `WeakReference` within Ninject released, but its up to its impl when it actually does something in reaction to this – Ruben Bartelink May 28 '13 at 11:59
  • No it don't if you debug using my code, the WeakRefereces is still alive. – eble May 28 '13 at 12:33
  • 2
    You can see this in the GarbageCollectionCachePruner.PruneCacheIfGarbageCollectorHasRun – eble May 28 '13 at 12:34