Named scope throws ScopeDisposedException when using "Release" configuration

[ghost]

Using named scopes in code built using "Release" configuration can cause scope objects to be disposed of and garbage collected prematurely, which in turn causes ScopeDisposedException to be thrown. This issue does not manifest itself when using "Debug" configuration.

Here's a sample binding configuration that allows this to happen:

Bind<IMainProcessor>().To<MainProcessor>().DefinesNamedScope("TheScope");
Bind<ISecondaryProcessor>().To<SecondaryProcessor>();
Bind<IDataFactory>().ToFactory();
Bind<IData>().To<Data>().InNamedScope("TheScope");

Main method retrieves MainProcessor via Kernel.Get<IMainProcessor>(). ISecondaryProcessor is passed to MainProcessor via constructor injection, and IDataFactory is passed to SecondaryProcessor via constructor injection. Then, inside SecondaryProcessor, IData objects are obtained using factory method.

If GC occurs while SecondaryProcessor works on its IData object(s) in a loop, GarbageCollectionCachePruner will kick in and collect the named scope reference object. This results the subsequent request to retrieve IData using factory methods to fail, and ScopeDisposedException to be thrown.

Changing the binding setup by moving the DefinesNamedScope("TheScope") declaration to the SecondaryProcessor's binding seems to work in this case, but it's not a viable solution, as multiple SecondaryProcessor instances, that need to share the same IData object, might need to be created.

A Visual Studio solution illustrating this problem can be downloaded from here: http://s000.tinyupload.com/index.php?file_id=60787475607892861020
Behavior can be reproduced by first building the solution using "Release" configuration, and then running the executable without debugger attached (e.g. using windows explorer).

[b3rnhard]

As we were having the same issue, I was looking into what you posted here, trying to reproduce it with my own solution - yours has been scraped from tinyupload. When you're saying "IData objects are obtained using factory method" you should be saying "IData object", as for this scope there should only be one IData instance per IMainProcessor instance. Can you please reupload your VS solution or modify my example below, so that I can have a look at it to find out if it is the same issue that our company is dealing with? What I have been trying out without attached debugger and with Release build configuration did not yield any ObjectDisposedException. Our application crashed only once with said exception and we could not reproduce it.
This is what I tried just now:

class Program {
        static void Main(string[] args) {
            var k = new StandardKernel();
            k.Bind<IMainProcessor>().To<MainProcessor>().DefinesNamedScope("TheScope");
            k.Bind<ISecondaryProcessor>().To<SecondaryProcessor>();
            k.Bind<IDataFactory>().ToFactory();
            k.Bind<IData>().To<Data>().InNamedScope("TheScope");

            var mainProcessor = k.Get<IMainProcessor>();
            mainProcessor.SecondaryProcessor.CreateDatas();
        }
    }

    public interface IDataFactory {
        IData Create();
    }

    public interface IData {}

    public class Data : IData {

        private static int instanceCounter = 0;
        private int instance;

        public Data() {
            instance = instanceCounter++;

            Console.Out.WriteLine("Creating new Data {instance}");
        }
    }

    public class SecondaryProcessor : ISecondaryProcessor {
        public IDataFactory DataFactory { get; set; }
        public SecondaryProcessor(IDataFactory dataFactory) {
            DataFactory = dataFactory;
        }

        public void CreateDatas() {
            while (true) {
                DataFactory.Create();
            }
        }
    }

    public interface ISecondaryProcessor {
        void CreateDatas();
    }

    public class MainProcessor : IMainProcessor {
        public ISecondaryProcessor SecondaryProcessor { get; set; }
        public MainProcessor(ISecondaryProcessor  secondaryProcessor) {
            SecondaryProcessor = secondaryProcessor;
        }
    }

    public interface IMainProcessor {
        ISecondaryProcessor SecondaryProcessor { get; set; }
    }

I used the required extensions Factory, NamedScope and ContextPreservation, my packages.config looks like this:

<packages>
  <package id="Castle.Core" version="3.2.0" targetFramework="net452" />
  <package id="Ninject" version="3.2.2.0" targetFramework="net452" />
  <package id="Ninject.Extensions.ContextPreservation" version="3.2.0.0" targetFramework="net452" />
  <package id="Ninject.Extensions.Factory" version="3.2.1.0" targetFramework="net452" />
  <package id="Ninject.Extensions.NamedScope" version="3.2.0.0" targetFramework="net452" />
</packages>

[bartuszekj]

I have re-uploaded the test project here: http://s000.tinyupload.com/index.php?file_id=60787475607892861020 - I can reproduce the issue every single time using this solution.

Make sure you do not use the vshost binary (do not use VS environment to run this). This bug is reproducible when running the standalone .exe file built using "Release" configuration.

[b3rnhard]

Thanks for reuploading.

[bartuszekj]

Please let me know if you happen to encounter the same problem again. I hope we can bring a developer's attention to this issue.

[b3rnhard]

Could this be an explanation of that problem?
http://stackoverflow.com/questions/16307915/ninject-request-scope-has-already-been-disposed/16496527#16496527

[bartuszekj]

I am not sure, but I don't think so. Remo's answer indicates that in such case, the parent object is ready for deactivation as well, but in the test solution that I have uploaded, the parent object's work is not done as the for loop would normally keep executing.

[BrunoJuchli]

I'm unable to access the uploaded repro (because our proxy prevents me from accessing any file sharing services :( ). So just a quick question:
Are you holding on to a strong reference of IMainProcessor (for the duration of any ISecondaryProcessor doing work / IData being requested from the kernel)?

[b3rnhard]

I just changed the uploaded solution from bartuszekj to this:

namespace NamedScopeTest
{
    internal static class Program
    {
        private static IMainProcessor processor;

        public static void Main()
        {
            // Simulate heavy load, collect frequently
            var task = new Task(() =>
            {
                while (true)
                {
                    Thread.Sleep(500);
                    GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced);
                    GC.WaitForPendingFinalizers();
                }
            });

            task.Start();

            var settings = new NinjectSettings {CachePruningInterval = new TimeSpan(0, 0, 5)};
            IKernel kernel = new StandardKernel(settings, new NinjectBindingsModule());

            PrintLoadedModules(kernel);

            processor = kernel.Get<IMainProcessor>();
            processor.Process();
        }

        private static void PrintLoadedModules(IKernel kernel)
        {
            Console.WriteLine("Loaded Ninject Kernel modules:");
            foreach (var module in kernel.GetModules())
                Console.WriteLine("{0}", module.Name);
            Console.WriteLine();
        }
    }
}

I. e. I introduced a static field which holds the IMainProcessor instance. With this and without debugger and with release configuration, the problem does not occurr anymore. As soon as I switch it back to the fieldless version, I get the ScopeDisposedException. Does that mean, IMainProcessor is cleaned up?

[b3rnhard]

It seems to be a JIT thing, not a Ninject issue:
http://stackoverflow.com/questions/30416520/how-local-variable-usage-infomation-is-maintained-in-net-clr-source-code

[BrunoJuchli]

@b3rnhard
Thanks that is very interesting information.
As a matter of fact ninject periodically checks whether the object is still alive. If there's no-one holding on to to the MainProcessor the GC will collect it. Apparently the jit compiler thinks "no-one uses this object anymore, let's get rid of the reference". The GC collects it. Ninject detects that the object is dead and disposes the scope. Thus the ScopeDisposedException.

This also means something like Console.WriteLine(mainProcessor) at the end of the method would fix the issue (poor man's workaround ;-)).

---

processing is not async, is it?

[b3rnhard]

Yes, any need for the mainProcessor instance AFTER all scope usages keeps the mainProcessor instance long enough so that no ScopeDisposedException is thrown. A not so obvious example for this would be a change in the MainProcessor implementation itself:

public sealed class MainProcessor : IMainProcessor
{
    private readonly ISecondaryProcessor _secondaryProcessor;
    private int i = 15;

    public MainProcessor(ISecondaryProcessor secondaryProcessor)
    {
        _secondaryProcessor = secondaryProcessor;
    }

    public void Process()
    {
        _secondaryProcessor.Process();
        Console.Out.Write(this.i); // AFTER _secondaryProcessor.Process() is finished, <this> is still required
    }
}

[bartuszekj]

Thank you @b3rnhard and @BrunoJuchli!

So it looks like GC is picking that object up almost immediately, and adding a destructor indeed confirms this:

~MainProcessor()
{
    Console.WriteLine("~MainProcessor()");
}

It also looks like the fact that this is only happening when running a "Release"-built code, might be explained in one of the comments here: https://stackoverflow.com/questions/90871/debug-visual-studio-release-in-net

Another big difference is that the GC behavior is somewhat different in that the JIT compiler will insert calls to GC.KeepAlive() as appropriate/needed in order to support debugging sessions.

This makes sense and explains why isn't the MainProcessor instance being immediately disposed of when running the "Debug" configuration. I think in this case, the best solution is to add an explicit GC.KeepAlive() call at the end of Main():

public static void Main()
{
    // Simulate heavy load, collect frequently
    var task = new Task(() =>
    {
        while (true)
        {
            Thread.Sleep(500);
            GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced);
            GC.WaitForPendingFinalizers();
        }
    });

    task.Start();

    var settings = new NinjectSettings {CachePruningInterval = new TimeSpan(0, 0, 5)};
    IKernel kernel = new StandardKernel(settings, new NinjectBindingsModule());

    PrintLoadedModules(kernel);

    var processor = kernel.Get<IMainProcessor>();
    processor.Process();
    GC.KeepAlive(processor); // Add this
}

This indeed works and lets "Release" code finish gracefully.

I think this ticket can now be closed.

[b3rnhard]

This should work, too:

public sealed class MainProcessor : IMainProcessor
{
    private readonly ISecondaryProcessor _secondaryProcessor;

    public MainProcessor(ISecondaryProcessor secondaryProcessor)
    {
        _secondaryProcessor = secondaryProcessor;
    }

    public void Process()
    {
        _secondaryProcessor.Process();
        GC.KeepAlive(this); // added
    }
}

That way, the MainProcessor implementation itself contains everything to work properly and you don't have to remember to put the GC.KeepAlive() outside after every call to MainProcessor.Process()

[bartuszekj]

Yes, you are absolutely right, @b3rnhard. Your approach is definitely a better one. Thank you.

[b3rnhard]

You're welcome. Learned something, too. 👍

[b3rnhard]

Further investigation on our project showed that:

• the NamedScopeParameter's scope object IsDisposed == true although it is still very much alive and not GC'd.
• the object that is configured with DefinesNamedScope() is also alive
• A ScopeDisposedException is thrown. At the same time when this request happened the garbage collector just ran - I am using the aggressive version from the solution provided by OP, the log statement is shown milliseconds before the crash. It seems as if the DisposeNotifyingObject has been actively disposed (instead of being finalized by the GC) although it should not have.

I believe that OPs test solution did not expose the issue that is still existing in Ninject

[bartuszekj]

Thanks for the update. Any chance you can provide a mock-up solution with this setup that you have just described, so that I can confirm that I can reproduce that too?

[b3rnhard]

It wasn't a Ninject bug after all, "just" a design inconvenience of Ninject. I've added a repo to Github that explains and demonstrates the behavior here: https://github.com/b3rnhard/NinjectNamedScopesStrangeness