What's hot ? (and I mean really ...) - scroll down for more
1).  Code Templating - advanced usage of delegates & generics: my slides & demos are available for download! CodeProject article is also available.

2).  My series "TDD in the eyes of a simpleminded" is in progress(including code!): preface, part1, part2, Q&A 1, Manual Stub .vs. Mock Stub

3).  TDD Workshop: SeeCompass v0.1 and v0.2 are out.
# Friday, January 27, 2006
# Best Practice: verify the safe cleanup of your unmanaged code

After elaborating on the topic on my first post (see comments), it's time to wrap the all thing up.
As I've noted, the purpose was to brought up some points of interest about developing classes which wrap unmanaged code inside them. When I'm reviewing my teammates code, any usage of unmanaged code triggers me as I start thinking about all the scenarios of which the unmanaged code stays un-handled and therefore causes memory leaks in the application. One of the scenarios which can cause a memory leak was the one we've talked about(again, the code in the first post was only for teaching purpose, so this post will be more relevant), but this could be better handled by implementing "Dispose pattern" on the (wrapper)class:

public class CustomWriter : IDisposable
{
    private bool disposed = false; // Track whether Dispose has been called.
    private StreamWriter m_sw;

    public CustomWriter(string filePath, int numberOfLinesToWrite)
    {
        m_sw = new StreamWriter(filePath);

        // If we know that an exception may occur after initializing unmanaged code - try-catch is preferable.
        try
        {
            if (numberOfLinesToWrite < 0)
            {
                throw new ArgumentException("What's wrong with you ?!!? numberOfLinesToWrite can't be less than 0", "numberOfLinesToWrite");
            }

            // Assume that I'm using numberOfLinesToWrite here...
        }
        catch // equals to catch(Exception)
        {
            // Explicit disposal of the object - deterministic.
            Dispose();

            throw; // rethrow the exception without losing the stack trace.
        }
    }

    #region IDisposable Members

    // Implement IDisposable.
    // Do not make this method virtual !
    // A derived class should not be able to override this method.
    public void Dispose()
    {
        Dispose(true);
        // This object will be cleaned up by the Dispose method.
        // Therefore, you should call GC.SupressFinalize to
        // take this object off the finalization queue
        // and prevent finalization code for this object
        // from executing a second time.
        GC.SuppressFinalize(this);
    }

    private void Dispose(bool disposing)
    {
        // Check to see if Dispose has already been called.
        if(!disposed)
        {
            // If disposing equals true - Explicit disposal is called.
            // Dispose managed resources.
            if(disposing)
            {
                // clean up managed resources (if have any).
            }

            // Call the appropriate methods to clean up unmanaged resources here.
            m_sw.Close();

            disposed = true;
        }            
    }

    ~CustomWriter()
    {
        // nondeterministic
        // Do not re-create Dispose clean-up code here.
        // Calling Dispose(false) is optimal in terms of
        // readability and maintainability.
        Dispose(false);
    }


    #endregion
}

Summing up:

If you're developing a wrapper class for unmanaged code, make sure you implement the "Dispose pattern" properly.
Now, if you're a consumer of a wrapper class, make sure to call the Dispose method (Explicit Dispose) via using or try-finally{Dispose}.
 
Annotations:

  • Remember that the destructor\finalizer (~CustomWriter()) in C# is nondeterministic; This is an important one as the unmanaged code will stay unhandled until the GC(Garbage collector) will decide otherwise.
  • "Explicit Dispose" is your goal while using wrapper classes, especially if you're developing an ASP.NET application which is hosted by a single process (aspnet_wp.exe\w3wp.exe) which hosts many applications (AppDomains) as the nondeterministic nature of the (C#) class destructor\finalizer. In a console\Windows Forms application, each application has its own process, and shutting them down will call the objects destructors(if have any) thus making it harder to detect memory leaks.
  • Implementing a destructor\finalizer will harm your performance as the GC has to handle this class instances in a special manner, but this is still much better than memory leaks in your application - more about it at Joe's article (link at the end of my post).
  • If you're a developer - while writing the wrapper classes or using them, make sure that you call\implement the Dispose method as needed.
  • If you're a code reviewer - make sure to add the appropriate tests to you code review list, so you won't forget to check them.

More on the subject:

You should read this article by Joe Duffy, a technical program manager of the CLR team, about "Dispose, Finalization, and Resource Management". 
* The article is quite long and hard to follow, but I'm sure that a quick scan can teach you a couple of things.
.NET | Articles
Posted by Oren Ellenbogen 
27/01/2006 01:33, Israel time UTC+02:00,     Comments [1]  |  Related posts:
Microsoft CCR: clean way to write parallel code in .Net
Making WCF Proxy useable
Bring lock back until 12, it has a busy day tomorrow
How to set a free TDD-enabled environment
How to mock static class or static member for testing
Writing Thread Safety tests for instance data
Sunday, January 29, 2006 12:35:10 PM (Jerusalem Standard Time, UTC+02:00)
If I were you I would make this pattern into a class. A pattern is usually a general "way" of doing things - if you are referring to a recurring structure of code lines this would best be encapsulated by a class, making it easier to implement and much more important - easier to change.

I thought of something that looks like this:

public class Disposer
{
public interface INeedDisposing : IDisposable
{
void cleanUpManagedResouces();
void cleanUpUnmanagedResources();
}

INeedDisposing _inNeedOfDisposal;
bool _disposed = false;

public Disposer(INeedDisposing inNeedOfDisposal)
{
if (inNeedOfDisposal == null)
throw new ArgumentNullException("inNeedOfDisposal");
_inNeedOfDisposal = inNeedOfDisposal;
}

public void disposeNow(bool calledFromDisposeMethod)
{
if (!_disposed)
{
if (calledFromDisposeMethod)
{
_inNeedOfDisposal.cleanUpManagedResouces();
}

_inNeedOfDisposal.cleanUpUnmanagedResources();

_disposed = true;
}
if (calledFromDisposeMethod)
{
GC.SuppressFinalize(_inNeedOfDisposal);
}
}
}

public abstract class DisposableBase : Disposer.INeedDisposing
{
Disposer _disposer;
public DisposableBase()
{
_disposer = new Disposer(this);
}
public virtual void cleanUpManagedResouces() { }
public virtual void cleanUpUnmanagedResources() {}

public void Dispose()
{
_disposer.disposeNow(true);
}
~DisposableBase()
{
_disposer.disposeNow(false);
}
}

The Disposer class actually encapsulates the disposal behavior and can be used on its own, the DisposableBase abstract base class makes the use of Disposer even easier if you can derive your disposable class form it (it also describes the proper usage of Disposer).

Here are two ways to implement CustomWriter using the Disposer and DisposerBase classes:

public class CustomWriter : IDisposable, Disposer.INeedDisposing
{
private StreamWriter m_sw;
Disposer _disposer;

public CustomWriter(string filePath, int numberOfLinesToWrite)
{
_disposer = new Disposer(this);
m_sw = new StreamWriter(filePath);

using (this)
{
if (numberOfLinesToWrite < 0)
{
throw new ArgumentException("What's wrong with you ?!!? numberOfLinesToWrite can't be less than 0", "numberOfLinesToWrite");
}
}
}
public void Dispose()
{
_disposer.disposeNow(true);
}
~CustomWriter()
{
_disposer.disposeNow(false);
}
public void cleanUpManagedResouces()
{
}
public void cleanUpUnmanagedResources()
{
m_sw.Close();
}
}

public class CustomWriter1 : DisposableBase
{
private StreamWriter m_sw;
public CustomWriter1(string filePath, int numberOfLinesToWrite)
{
m_sw = new StreamWriter(filePath);

using (this)
{
if (numberOfLinesToWrite < 0)
{
throw new ArgumentException("What's wrong with you ?!!? numberOfLinesToWrite can't be less than 0", "numberOfLinesToWrite");
}

// Assume that I'm using numberOfLinesToWrite here...
}
}
public override void cleanUpUnmanagedResources()
{
m_sw.Close();
}
}

So all the developer has to do is be either implement the Disposer.INeedDisposing interface or inherit from the DisposableBase class. If the first option is chosen (maybe the class already inherits from something else) there are the Dispose method and destructor which require attention.
The code reviewer should come up with the right unit test to automatically check for proper disposal of unmanaged resources and rid himself or herself of boring code reviewing sessions...
Yoni
Comments are closed.