NamedScope and garbage collection

Question

(This question was first asked in the Ninject Google Group, but I see now that Stackoverflow seems to be more active.)

I'm using the NamedScopeExtension to inject the same ViewModel into both the View and the Presenter. After the View have been released, memory profiling shows that the ViewModel is still retained by the Ninject cache. How can I make Ninject release the ViewModel? All ViewModels are released when the Form is Closed and Disposed, but I'm creating and deleting Controls using a Factory in the Form and would like the ViewModels be garbage collected to (the Presenter and View gets collected).

See the following UnitTest, using dotMemoryUnit, for an illustration of the problem:

using System;
using FluentAssertions;
using JetBrains.dotMemoryUnit;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Ninject;
using Ninject.Extensions.DependencyCreation;
using Ninject.Extensions.NamedScope;

namespace UnitTestProject
{
    [TestClass]
    [DotMemoryUnit(FailIfRunWithoutSupport = false)]
    public class UnitTest1
    {
        [TestMethod]
        public void TestMethod()
        {
            // Call in sub method so no local variables are left for the memory profiling
            SubMethod();

            // Assert
            dotMemory.Check(m =>
            {
                m.GetObjects(w => w.Type.Is<ViewModel>()).ObjectsCount.Should().Be(0);
            });
        }

        private static void SubMethod()
        {
            // Arrange
            var kernel = new StandardKernel();
            string namedScope = "namedScope";
            kernel.Bind<View>().ToSelf()
                  .DefinesNamedScope(namedScope);
            kernel.DefineDependency<View, Presenter>();
            kernel.Bind<ViewModel>().ToSelf()
                  .InNamedScope(namedScope);
            kernel.Bind<Presenter>().ToSelf()
                  .WithCreatorAsConstructorArgument("view");

            // Act
            var view = kernel.Get<View>();
            kernel.Release(view);
        }
    }

    public class View
    {
        public View()
        {
        }

        public View(ViewModel vm)
        {
            ViewModel = vm;
        }

        public ViewModel ViewModel { get; set; }
    }

    public class ViewModel
    {
    }

    public class Presenter
    {
        public View View { get; set; }
        public ViewModel ViewModel { get; set; }

        public Presenter(View view, ViewModel viewModel)
        {
            View = view;
            ViewModel = viewModel;
        }
    }
}

The dotMemory.Check assert fails and when analyzing the snapshot the ViewModel has references to the Ninject cache. I thought the named scope should be released when the View was released.

Regards, Andreas

Danit

Answer 1

TL;DR

short answer: add INotifyWhenDisposed to your View. Dispose the view. This will lead to ninject automatically disposing all stuff bound InNamedScope plus also ninject will un-reference these objects. This will lead to (eventual) garbage collection (unless you're hanging on to strong references elsewhere).

---

Why your implementation doesn't work

Ninject does not get informed when the view is released / get's disposed. That's why ninject has a timer running to check whether the scope-object is still alive (alive = not garbage collected). If the scope-object is not alive anymore it disposes/releases all the objects which were held in scope.

I believe the timer is set to 30seconds by default.

Now what does this mean exactly?

• if there's no memory pressure, the GC may take a long time until the scope-object is garbage collected (or he may not do it, ever)
• once the scope-object is garbage collected, it may take about 30 seconds for the scoped objects to be disposed and released as well
• once ninject released the scope objects, again, the GC may take a long time with collection of the object if there's no memory pressure.

Deterministically Releasing Scoped Objects

Now if you need the objects to be disposed/released immediately when the scope is released, you will need to add INotifyWhenDisposed to the scope object (also see here). With named scopes, you'll need to add this interface to the type which is bound with DefinesNamedScope - in your case the View.

According to Ninject.Extensions.NamedScope's integration tests this will suffice: see here

Note: The only thing which is really made deterministic by this is the disposal of scoped objects. In practice this will usually cut the time for garbage collection to occur significantly, too. However, if there's no memory pressure, again, the actual collection could still take a long time.

Implementing this should get the unit test to pass.

Note: if the root object is bound InCallScope then this solution does not work (ninject 3.2.2 / NamedScope 3.2.0). I think it's due to a bug with InCallScope but sadly i failed to report it (the bug) a few years back. I may just as well be mistaking, though.

---

Proof that implementing INotifyWhenDisposed in the root object will dispose children

public class View : INotifyWhenDisposed
{
    public View(ViewModel viewModel)
    {
        ViewModel = viewModel;
    }

    public event EventHandler Disposed;

    public ViewModel ViewModel { get; private set; }

    public bool IsDisposed { get; private set; }

    public void Dispose()
    {
        if (!this.IsDisposed)
        {
            this.IsDisposed = true;
            var handler = this.Disposed;
            if (handler != null)
            {
                handler(this, EventArgs.Empty);
            }
        }
    }
}

public class ViewModel : IDisposable
{
    public bool IsDisposed { get; private set; }

    public void Dispose()
    {
        this.IsDisposed = true;
    }
}

public class IntegrationTest
{
    private const string ScopeName = "ViewScope";

    [Fact]
    public void Foo()
    {
        var kernel = new StandardKernel();
        kernel.Bind<View>().ToSelf()
            .DefinesNamedScope(ScopeName);
        kernel.Bind<ViewModel>().ToSelf()
            .InNamedScope(ScopeName);

        var view = kernel.Get<View>();

        view.ViewModel.IsDisposed.Should().BeFalse();

        view.Dispose();

        view.ViewModel.IsDisposed.Should().BeTrue();
    }
}

It even works with DefineDependency and WithCreatorAsConstructorArgument

I don't have dotMemory.Unit but this checks whether ninject keeps a strong reference to the objects in its cache:

namespace UnitTestProject
{
    using FluentAssertions;
    using Ninject;
    using Ninject.Extensions.DependencyCreation;
    using Ninject.Extensions.NamedScope;
    using Ninject.Infrastructure.Disposal;
    using System;
    using Xunit;

    public class UnitTest1
    {
        [Fact]
        public void TestMethod()
        {
            // Arrange
            var kernel = new StandardKernel();
            const string namedScope = "namedScope";
            kernel.Bind<View>().ToSelf()
                .DefinesNamedScope(namedScope);
            kernel.DefineDependency<View, Presenter>();
            kernel.Bind<ViewModel>().ToSelf().InNamedScope(namedScope);

            Presenter presenterInstance = null;
            kernel.Bind<Presenter>().ToSelf()
                .WithCreatorAsConstructorArgument("view")
                .OnActivation(x => presenterInstance = x);

            var view = kernel.Get<View>();

            // named scope should result in presenter and view getting the same view model instance
            presenterInstance.Should().NotBeNull();
            view.ViewModel.Should().BeSameAs(presenterInstance.ViewModel);

            // disposal of named scope root should clear all strong references which ninject maintains in this scope
            view.Dispose();

            kernel.Release(view.ViewModel).Should().BeFalse();
            kernel.Release(view).Should().BeFalse();
            kernel.Release(presenterInstance).Should().BeFalse();
            kernel.Release(presenterInstance.View).Should().BeFalse();
        }
    }

    public class View : INotifyWhenDisposed
    {
        public View()
        {
        }

        public View(ViewModel viewModel)
        {
            ViewModel = viewModel;
        }

        public event EventHandler Disposed;

        public ViewModel ViewModel { get; private set; }

        public bool IsDisposed { get; private set; }

        public void Dispose()
        {
            if (!this.IsDisposed)
            {
                this.IsDisposed = true;
                var handler = this.Disposed;
                if (handler != null)
                {
                    handler(this, EventArgs.Empty);
                }
            }
        }
    }

    public class ViewModel
    {
    }

    public class Presenter
    {
        public View View { get; set; }
        public ViewModel ViewModel { get; set; }

        public Presenter(View view, ViewModel viewModel)
        {
            View = view;
            ViewModel = viewModel;
        }
    }
}

Lumumba

"In practice this will usually cut the time for garbage collection to occur significantly, too. However, if there's no memory pressure, again, the actual collection could still take a long time." dotMemory.Check causes garbage collection so test will show actual memory state – Yockey

Excellent explanation, but alas, this did not solve my problem. I added an OnActivation callback on the ViewModel binding and retrieved the scope object for inspection: It does not seem to be the View which acts as the scope object, but rather a DisposeNotifyingObject, which already implements INotifyWhenDisposed. – Danit

As a side note, I thought that explicitly releasing the view with Kernel.Release(), would clean up the named scope that the View binding defined. – Danit

If I dispose the scope object (the DisposeNotifyingObject mentioned above) the ViewModel is garbage collected and the unit test succeeds. – Danit

@AndreasAppelros yes you're right as the scope object ninject is not actually using the object bound with DefinesNamedScope(...). However, as the ninject integration test shows it'll still work if the object bound (View in your case) implements INotifyWhenDisposed. Did you give it a try? – Lumumba

@AndreasAppelros kernel.Release doesn't even call Dispose on the object. It just removes it from the cache. – Lumumba

@Lumumba Since View is in Call scope, kernel.Release() does actually call Dispose on the object. Just checked in the debugger to be sure. Thats true for INotifyWhenDisposed as well. Yes, I tried letting the View implement INotifyWhenDisposed, the ViewModel does not get GC:d. – Danit

@AndreasAppelros then what is/are the ViewModel's path/-s to the GCRoot? – Lumumba

This works! I hadn't implemented Dispose correctly (in my "real" project View is a UserControl, so Dispose is implemented in Component) first. – Danit