3

General overview

I need to bind with a native API that has 4 mains functions:

void ActivateEngine();
int CreateModule();
void DestroyModule(int id);
void TerminateEngine();

And the documentation states that ActivateEngine and TerminateEngine should surround any call to CreateModule and DestroyModule. That is usage should be something like:

void foo()
{
    ActivateEngine();

    int module1 = CreateModule();
    int module2 = CreateModule();

    ...

    DestroyModule(module2);
    DestroyModule(module1);

    TerminateEngine();
}

To do this I have created two .NET objects, namely Engine and Module both binded to the native API using the DllImport attribute.

Engine object acts as singleton and is binded to ActivateEngine and TerminateEngine.

Module object is used to create many instances within the Engine and is binded to CreateModule and DestroyModule in the native API.

Encountered issue

I have implemented things in a way that users can create Modules directly without carrying too much about the Engine or about the lifetime of the objects (i.e. I don't [And I don't want to] force users to dispose objects when no longer used).

To do this I have used a list of WeakReference in Engine object that points to all created Modules.

See my simplified code here.

The issue is that when application's end, finalizer are called in non determistic way and WeakReference targets are already null even if finalizer of Module has not been called yet and that parameter trackResurrection is set to true.

In my case the code logs the following:

ActivateEngine() ...
CreateModule() ==> 0 ...
CreateModule() ==> 1 ...
DestroyModule(1) ...
  Ooops ... Can't dispose registered module because WeakReference to it is already null ...
  Ooops ... Can't dispose registered module because WeakReference to it is already null ...
TerminateEngine() ...
DestroyModule(0) ...

Which of course is inappropriate order.

Question

How can force all Module to be finalized before the Engine ?

I truly don't want to force end-users to call Dispose method on Module objects and I also don't want to keep strong references to created Module so that objects can disappear automatically when no longer referenced in the code. Example:

 processing
 {
     var module = new Module();
     ...
 }

 foo()
 {
     processing();
     GC.Collect(); // If using strong references 'module' is gonna be kept alive (that's not smart)  
 }

I have looked to following threads using ConditionalWeakTable:

But I don't understand how this can help in my situation.

Community
  • 1
  • 1
CitizenInsane
  • 4,755
  • 1
  • 25
  • 56
  • Can you check the simplified [code link](https://gist.github.com/CitizenInsane/802bb4bd61c708f3402e) you have provided. – Rob Jan 07 '14 at 13:31
  • @Rob Sorry I modified the link to the [following](https://gist.github.com/CitizenInsane/06d898e3e3c0a380d6a7) (question has been edited to new the link) – CitizenInsane Jan 07 '14 at 13:39
  • 1
    Very unclear how you hope a WeakReference is going to be useful. You'll at least need a *counter*, a simple static int. Increment it for every CreateModule() call, decrement it for every DetroyModule() call. When it reaches 0, you can safely call TerminateEngine(). – Hans Passant Jan 07 '14 at 14:26
  • I would strongly urge you to actually make the objects `IDisposable`. Disposing is *designed* for deterministic clearing of resources. Finalization is, by design, non-deterministic. You don't even know *that* it will be called on a given object, let alone the order it will be called in. Having disposable objects makes this problem trivial as long as the users actually dispose of the objects. – Servy Jan 07 '14 at 17:31
  • @Servy ... yes ... "as long as users actually dispose the objects" ... that truly is the issue, "the users" ... :) – CitizenInsane Jan 07 '14 at 17:34
  • @CitizenInsane It's pretty well known to C# programmers in general that if you aren't disposing of disposable resources properly you're going to have problems. It's a reasonable expectation to have. A solution like what you're looking for is very likely to end up with bugs in it, even if it works in some of the common cases, leading to situations where your users *can't rely on it working* no matter what they do, which is a major problem. – Servy Jan 07 '14 at 17:35

4 Answers4

3

More of a workaround for your particular situation than a solution to the general problem:

Distribute the obligation to terminate the engine over both the Engine singleton and the Module objects.

Create a shared counter that you update via Interlocked methods (or native equivalents). This could be a static volatile int field or piece of unmanaged memory.

Theintshould count the number of 'references' to the engine your app maintains. Increment the counter atomically in every constructor and decrement it in every finalizer. The one finalizer that reduces the counter to zero also calls TerminateEngine() (and frees the shared counter)

The Engine object must also count as a 'reference' in case your user lets all Module objects get garbage collected but then starts creating new modules. Otherwise, the engine would be destroyed to early.

Christian Klauser
  • 4,416
  • 3
  • 31
  • 42
  • Thanks @Christian, I found a solution using WeakEvents but this is nice workaround I could have used for current situation. – CitizenInsane Jan 07 '14 at 17:17
3

I don't think you can approach this the way you want. The order of finalisation is non-deterministic, so it's not possible to know if your singleton Engine or a Module is going to be finalised first.

I would suggest you remove your Engine singleton (keep Engine as a static class if you like, but don't allow any instances and only use it for static methods to perform engine initialisation and termination) and the registration of modules and use a static atomic counter along the lines of the answer by @Christian-Klauser (incremented in the Module constructor and decremented in the finalizer). When the atomic counter goes from 0 to 1 then you can activate the engine (via a call to an internal static Engine method), and likewise terminate the engine when the module count goes to 0.

I would also suggest you ask your users to use the using mechanism when using your Module class.

Rob
  • 1,472
  • 12
  • 24
2

As the other answers and comments have said you need to implement some form of reference counting. Here is my attempt at doing it (I was working on this when you posted your answer), It still uses a singleton Engine (there is no requirement now to do so, you could make it a static class with minimal changes), however callers need to call AddRefrence() and ReleaseRefrence() to let the engine know if it needs to setup or tear down the API when the count hits 1 or 0 respectively.

Module supports releasing it's reference on calling Dispose() or when the class is finalized.

using System.Threading;

namespace FinalizerOrder
{
    using System;
    using System.Collections.Generic;
    using System.Diagnostics;

    class Engine
    {
        private Engine()
        {
            //ActivateEngine() is no longer called here.
        }

        private readonly static Engine _singleton = new Engine(); //Now that the constructor is empty we can initialize immediately.
        private readonly static object _syncLock = new object();
        private static volatile int _counter = 0;

        public static Engine Singleton
        {
            get { return _singleton; }
        }

        public void AddRefrence()
        {
            lock (_syncLock)
            {
                _counter++;
                if (_counter < 0)
                    throw new InvalidOperationException("ReleaseRefrence() was called more times than AddRefrence()");

                if(_counter == 1)
                    Debug.WriteLine("ActivateEngine() ...");
            }
        }

        public void ReleaseRefrence()
        {
            lock (_syncLock)
            {
                _counter--;

                if (_counter < 0)
                    throw new InvalidOperationException("ReleaseRefrence() was called more times than AddRefrence()");

                if (_counter == 0)
                {
                    Debug.WriteLine("TerminateEngine() ...");
                }
            }
        }
    }

    class Module : IDisposable
    {
        public Module()
        {
            Engine.Singleton.AddRefrence();

            _id = _counter++;
            Debug.WriteLine("CreateModule() ==> {0} ...", _id);

        }

        private readonly int _id;
        private static int _counter;

        ~Module()
        {
            Dispose(false);   
        }

        public void Dispose()
        {
            Dispose(true);
            GC.SuppressFinalize(this);
        }

        private bool _disposed = false;

        protected void Dispose(bool disposing)
        {
            if(_disposed)
                return;

            _disposed = true;                

            if (disposing)
            {
                //Nothing to do here, no IDisposeable objects.
            }

            Debug.WriteLine("DestroyModule({0}) ...", _id);
            Engine.Singleton.ReleaseRefrence();
        }
    }

    internal class Program
    {
        private static void Main()
        {
            Test();
            GC.Collect(3, GCCollectionMode.Forced);
            GC.WaitForPendingFinalizers();
            Test();

        }

        private static void Test()
        {
            var module1 = new Module();
            var module2 = new Module();

            GC.KeepAlive(module2);
            GC.KeepAlive(module1);
        }
    }
}
Scott Chamberlain
  • 124,994
  • 33
  • 282
  • 431
  • Thanks for writing solution using reference counting ... and sorry to have post in between ... This is anyway good and simple workaround to the issue. – CitizenInsane Jan 07 '14 at 17:42
  • How is this ensuring that the modules are released on the reverse order that they were created? – Servy Jan 07 '14 at 17:51
  • @Servy In the OP I only see that `ActivateEngine()` must be called before the first call of `CreateModule()` and that `TerminateEngine()` must be called after `DestroyModule(int)` has been called for every created instance. I don't see anything about the order of `DestroyModule(int)` between `ActivateEngine()` and `TerminateEngine()` mattering. – Scott Chamberlain Jan 07 '14 at 17:54
  • Oh, and in your application creating a module, disposing of it, and then creating another results in using a disposed `Engine`. – Servy Jan 07 '14 at 17:56
  • @Servy It does not, the order would be `ActivateEngine()`, `CreateModule()`, `DestroyModule(0)`, `TerminateEngine()`, `ActivateEngine()`, `CreateModule()`. So it terminates the engine when the last Module is distroyed then it creates a new engine when the next module comes in (I tested this in my test code at the bottom, calling `Test()` twice between a GC cleanup) – Scott Chamberlain Jan 07 '14 at 17:58
1

I ended-up using FastSmartWeakEvent pattern.

This is general solution and it is easy to read/understand.

See updated sample code here.

CitizenInsane
  • 4,755
  • 1
  • 25
  • 56
  • Damned ... after working with other test-cases this is pitfall ... WeakEvents are WeakReferences ... they lead to unpredictable finalizer order too ... Christian's proposal and @Scott 's implementation are definitely the most appropriate ones (worked for all test-cases so far). – CitizenInsane Jan 07 '14 at 23:46