Skip to content

Dispose

marksvc edited this page Feb 15, 2020 · 3 revisions

In the past we have been hit several times by improper dealing with disposable objects. This often causes hanging tests on some machines.

How does it work?

Add the following line to the Dispose(bool) method in the lowest possible base class you have control over.

System.Diagnostics.Debug.WriteLineIf(!fDisposing, "***** Missing Dispose() call for " + GetType() + ". *****");

The "Missing Dispose" message will be output when an object gets called from the finalizer, i.e. we don't call the Dispose() method.

However, since this is run during garbage collection where the order in which the objects get finalized is kind of random, we can't rely on this message. It might happen that the Console object is already disposed when we try to output the message, in which case we don't get the message.

Why should I care?

The contract for the IDisposable interface states that you have to call Dispose() on a object that implements IDisposable.

Most often nothing bad happens if you don't call Dispose(), but there are two dangers:

  1. The order in which objects get finalized is non-deterministic which might lead to problems on a different machine even if it works on your machine

  2. another developer might add some code to the class that now requires the dispose message to be called (e.g. a COM object that holds on to some other objects). Since the class already implements IDisposable he assumes that it should be sufficient to dispose the new member variable in the Dispose(bool) method.

In any case this might result in hanging tests, and/or the application not exiting properly which makes it impossible for the user to restart the app.

What do I need to do?

Please don't just comment the "Missing Dispose" call since they help to find real problems - even if the amount of lines you get seems annoying. The end user who gets a release build won't see these messages.

If you get a "Missing Dispose() call for ClassName" in the output of a test, then you have to get rid of the message by finding the place where the object should be disposed. See below for some debugging tips. There shouldn't show up any missing dispose messages when running tests at the time you check in.

If you get the "Missing Dispose" message when running the application, you should verify the code you modified to make sure any disposable object you create gets disposed properly. Ideally you would fix the problems so that you no longer get any missing dispose messages, but a certain number of dispose messages in the output when running the application are acceptable.

Rules

  • If a class has (owns) disposable member variables it has to implement the IDisposable interface and call Dispose() on its member variables

  • Try to avoid unnecessary implementations of IDisposable

  • Make sure that every disposable object has exactly one owner who is responsible for disposing the object. If multiple classes have a member variable that reference the same object, only one of the classes should own the object and dispose it in its Dispose(bool) method. The other classes should only reference it and not dispose it.
    This can get a little tricky if the classes that reference the same object have different life expectancies, i.e. if one class gets disposed while another class is still around. This requires some careful design to find the correct owner, but IMO it makes the code clearer since responsibilities are clearly defined.

  • Add a comment for methods/constructors that take a disposable object as parameter (especially if it gets stored for later use) whether this class takes this object as reference or wants to take ownership of it, i.e. is responsible for disposing it.

  • If you create an object that needs to stay around but you don't want to create a separate member variable for it, consider putting it in the System.ComponentModel.Container components member variable (if your class derives from System.Component) or in a SIL.Utils.DisposableObjectsSet member variable

  • Be careful when passing a managed object that implements IDisposable to a COM object. If the COM object stores the pointer to that managed object, the COM object needs to be released when the managed object gets disposed (or at least the COM object needs to replace the reference to the managed object). Otherwise this might cause a deadlock when the GC tries to collect the objects.

Tips for debugging

It's sometimes hard to find the correct object that doesn't get disposed if several instances of a class get created. I found the code snippet below helpful. Besides the missing dispose message it shows the instance number of the object that didn't get disposed. This allows to set a breakpoint in the constructor at the n-th call which allows you to see where exactly the object gets created.

public MyClass: IDisposable
{
    private static int Count;
    private int Number;

    public MyClass()
    {
        Number = ++Count;
        System.Diagnostics.Debug.WriteLine("Creating " + GetType() + " number " + Number);
    }

#if DEBUG
    ~MyClass()
    {
        Dispose(false);
    }
#endif

    public bool IsDisposed { get; private set; }

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

    protected virtual void Dispose(bool fDisposing)
    {
        System.Diagnostics.Debug.WriteLineIf(!fDisposing, "***** Missing Dispose() call for " + GetType() + ". *****");
        System.Diagnostics.Debug.WriteLine("Disposing " + GetType() + " number " + Number);

        if (fDisposing)       
        {
        }

        IsDisposed = true;
    }
}

Investigating with ObjectCreationPath

It can also be helpful to find where an object was created by adding this field to the object:

private string ObjectCreationPath = System.Environment.StackTrace;

And then in the object's Dispose(bool) method, add:

System.Diagnostics.Debug.WriteLineIf(!disposing, "Object creation path:" + System.Environment.NewLine + ObjectCreationPath);

This will cause the object to print a stack trace of how it was created, if it is finalized before being disposed.

Investigating with undisposed-fody

See https://github.com/ermshiperete/undisposed-fody

Notes

Note that a missing dispose might be caused by a variable being assigned to twice. For example, if

m_window = new XWindow();

happens twice, then a corresponding

m_window.Dispose() 

is not enough.

If you can not reproduce missing dispose errors using your IDE, you may be able to from the Terminal:

$ Build/run-in-environ mono --debug Bin/NUnit/bin/nunit-console.exe Output/Debug/xWorksTests.dll -labels -run=SIL.FieldWorks.XWorks
Clone this wiki locally