The Black Box of .NET Headline Animator

The Black Box of .NET
Showing posts with label dispose. Show all posts
Showing posts with label dispose. Show all posts

Tuesday, March 13, 2012

Don't use 'using()' with a WCF proxy


If you're trying to be a conscientious developer and making sure that you cleanup your resources - great! You are writing 'using()' blocks around all of your disposable items - great...except when the disposable item is a WCF Client/Proxy! The using() statement and the try/finally effectively have the same IL:

    // The IL for this block is effectively the same as
    // the IL for the second block below
    using (var win = new Form())
    {
    }

   
    // This is the second block
    Form f = null;
    try
    {
        f = new Form();
    }
    finally
    {
        if (f != null)
        {
            f.Dispose();
        }
    }

Here's the IL for the 'using()' block above compiled in Release mode:

     IL_0000:  newobj     instance void [System.Windows.Forms]System.Windows.Forms.Form::.ctor()
     IL_0005:  stloc.0
     .try
     {
         IL_0006:  leave.s    IL_0012
     }  // end .try
     finally
     {
         IL_0008:  ldloc.0
         IL_0009:  brfalse.s  IL_0011
         IL_000b:  ldloc.0
         IL_000c:  callvirt   instance void [mscorlib]System.IDisposable::Dispose()
         IL_0011:  endfinally
     }  // end handler


Here's the IL for the second block (try/finally) compiled in Release mode:

     IL_0012:  ldnull
     IL_0013:  stloc.1
     .try
     {
         IL_0014:  newobj     instance void [System.Windows.Forms]System.Windows.Forms.Form::.ctor()
         IL_0019:  stloc.1
         IL_001a:  leave.s    IL_0026
     }  // end .try
     finally
     {
         IL_001c:  ldloc.1
         IL_001d:  brfalse.s  IL_0025
         IL_001f:  ldloc.1
         IL_0020:  callvirt   instance void [System]System.ComponentModel.Component::Dispose()
         IL_0025:  endfinally
     }  // end handler

As you can see, the IL is nearly identical.

Well this is all fine and good but let's get back to the issue with WCF.  The problem is that if an exception is thrown during disposal of the WCF client/proxy, the channel is never closed.  Now, in general, any exception that occurs during disposal of an object is indeed undesirable.  But, in the case of WCF, multiple channels remaining open could easily cause your entire service to fall on its face - not to mention what might eventually happen to your web server.

Here is an alternative solution that can be used:

    WCFProxy variableName = null;
    try
    {
        variableName = new WCFProxy();

        // TODO code here

        variableName.Close();
    }
// if you need to catch other exceptions, do so here...
    catch (Exception)
    {
        if (variableName != null)
        {
            variableName.Abort();
        }
        throw;
    }

MSDN does have a brief on this issue which you can read here - http://msdn.microsoft.com/en-us/library/aa355056.aspx

Share

Wednesday, March 2, 2011

How do you properly implement the IDisposable pattern?

This is the first post of a series that I'll be making on memory management in .NET.  The subsequent posts will cover guidelines for working with disposable objects, when to implement IDisposable, and maybe a few others.

I am often asked how to correctly implement the IDisposable pattern. There are many posts you'll see from a Google search that try to answer this question. IMHO, the pattern that you find here is the safest and most correct implementation of the pattern according to .NET Best Practices and my experience. I am confident saying this because of my extensive experience with memory leak debugging, internals of the CLR, code reviews, and implementation of best practices at very large, well-known clients who rely on solid applications.  You will find that my provided implementation of this pattern is slightly different than that which is posted on MSDN.  IMHO, it isn't sufficient.  You have to agree that a large number of samples and examples posted on MSDN are "hacky" and don't do things the way that they should be done in the "real world".

A couple of notes on implementing the IDisposable pattern:
  1. Simply adding a Dispose() method to a class doesn't implement the pattern (just because it compiles doesn't mean it's correct).  I've seen this mistake made many, many times - it's just not a good idea.
  2. Adding the code below isn't enough - you must also explicitly declare the class inheriting from IDisposable ->  MyDisposableClass : IDisposable.  This is very important since objects creating/managing instances of your class might decide to try to cast it to IDisposable and call Dispose or use a 'using()' statement. If your class isn't explicitly declared to implement IDisposable, the cast will fail and Dispose will never be called.
  3. Just because you implement IDisposable correctly doesn't mean it will get called "magically". What I mean is that the GC never calls Dispose for you - you have to explicitly call it on your class from the owning object.  There is a common misconception that this can somehow magically happen by the GC and that it's not important that you explicitly call it "because the GC cleans up for you" (WRONG!)
  4. Implementing the IDisposable pattern correctly is tricky and can easily lead to problems if not done properly.  This is why I've provided the code below.
  5. I use a code snippet with the exact code below to properly and consistently implement the pattern correctly.
  6. There is great debate on whether or not you should null out rooted references. It certainly never hurts to do so. Nulling these out does do something before GC runs - it removes the rooted reference to that object. The GC later scans its collection of rooted references and collects those that do not have a rooted reference.

    Think of this example when it is good to do so: you have an instance of type "ClassA" - let's call it 'X'. X contains an object of type "ClassB" - let's call this 'Y'. Y implements IDisposable, thus, X should do the same to dispose of Y. Let's assume that X is in Generation 2 or the LOH and Y is in Generation 0 or 1. When Dispose() is called on X AND that implementation nulls out the reference to Y, the rooted reference to Y is immediately removed. If a GC happens for Gen 0 or Gen 1, the memory/resources for Y is cleaned up but the memory/resources for X is not since X lives in Gen 2 or the LOH.
Here is the code for properly implementing the IDisposable pattern in a base class:


#region IDisposable base-class implementation
 
//TODO remember to make this class inherit from IDisposable -> MyDisposableClass : IDisposable
 
/// <summary>
/// Gets or sets a value indicating whether this instance is disposed.
/// </summary>
/// <value>
///  <c>true</c> if this instance is disposed; otherwise, <c>false</c>.
/// </value>
/// <remarks>Default initialization for a bool is 'false'</remarks>
private bool IsDisposed { getset; }
 
/// <summary>
/// Implementation of Dispose according to .NET Framework Design Guidelines.
/// </summary>
/// <remarks>Do not make this method virtual.
/// A derived class should not be able to override this method.
/// </remarks>
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.
 
    // Always use SuppressFinalize() in case a subclass
    // of this type implements a finalizer.
    GC.SuppressFinalize(this);
}
 
/// <summary>
/// Overloaded Implementation of Dispose.
/// </summary>
/// <param name="isDisposing"><c>true</c> to release both managed and unmanaged resources; 
/// <c>false</c> to release only unmanaged resources.</param>
/// <remarks>
/// <list type="bulleted">Dispose(bool isDisposing) executes in two distinct scenarios.
/// <item>If <paramref name="isDisposing"/> equals true, the method has been called directly
/// or indirectly by a user's code. Managed and unmanaged resources
/// can be disposed.</item>
/// <item>If <paramref name="isDisposing"/> equals <c>false</c>, the method has been called 
/// by the runtime from inside the finalizer and you should not reference
/// other objects. Only unmanaged resources can be disposed.</item></list>
/// </remarks>
protected virtual void Dispose(bool isDisposing)
{
    // TODO If you need thread safety, use a lock around these 
    // operations, as well as in your methods that use the resource.
    try
    {
        if (!this.IsDisposed)
        {
            // Explicitly set root references to null to expressly tell the GarbageCollector
            // that the resources have been disposed of and its ok to release the memory 
            // allocated for them.
            if (isDisposing)
            {
                // Release all managed resources here


                // Need to unregister/detach yourself from the events. Always make sure
                // the object is not null first before trying to unregister/detach them!
                // Failure to unregister can be a BIG source of memory leaks
                if (someDisposableObjectWithAnEventHandler != null)
                {                 
                    someDisposableObjectWithAnEventHandler.SomeEvent -= someDelegate;
                    someDisposableObjectWithAnEventHandler.Dispose();
                    someDisposableObjectWithAnEventHandler = null;
                }


                // If this is a WinForm/UI control, uncomment this code
                //if (components != null)
                //{
                //    components.Dispose();
                //}
            }
            // Release all unmanaged resources here  
            // (example)             if (someComObject != null && Marshal.IsComObject(someComObject))
            {
                Marshal.FinalReleaseComObject(someComObject);
                someComObject = null;
            }
        }
    }
    finally
    {
        this.IsDisposed = true;
    }
}

//TODO Uncomment this code if this class will contain members which are UNmanaged
///// <summary>Finalizer for MyDisposableClass</summary>
///// <remarks>This finalizer will run only if the Dispose method does not get called.
///// It gives your base class the opportunity to finalize.
///// DO NOT provide finalizers in types derived from this class.
///// All code executed within a Finalizer MUST be thread-safe!</remarks>
//  ~MyDisposableClass()
//  {
//     Dispose( false );
//  }
#endregion IDisposable base-class implementation
 
Here is the code for properly implementing the IDisposable pattern in a derived class:

#region IDisposable derived-class implementation

/// <summary>
/// Gets or sets a value indicating whether this instance is disposed.
/// </summary>
/// <value>
///  <c>true</c> if this instance is disposed; otherwise, <c>false</c>.
/// </value>
/// <remarks>Default initialization for a bool is 'false'</remarks>
private bool IsDisposed { getset; }

/// <summary>
/// Overloaded Implementation of Dispose.
/// </summary>
/// <param name="isDisposing"><c>true</c> to release both managed and unmanaged resources; 
/// <c>false</c> to release only unmanaged resources.</param>
/// <remarks>
/// <list type="bulleted">Dispose(bool isDisposing) executes in two distinct scenarios.
/// <item>If <paramref name="isDisposing"/> equals true, the method has been called directly
/// or indirectly by a user's code. Managed and unmanaged resources
/// can be disposed.</item>
/// <item>If <paramref name="isDisposing"/> equals <c>false</c>, the method has been called

/// by the runtime from inside the finalizer and you should not reference
/// other objects. Only unmanaged resources can be disposed.</item></list>
/// </remarks>
protected override void Dispose(bool isDisposing)
{
    // TODO If you need thread safety, use a lock around these 
    // operations, as well as in your methods that use the resource.
    try
    {
        if (!this.IsDisposed)
        {
            // Explicitly set root references to null to expressly tell the GarbageCollector
            // that the resources have been disposed of and its ok to release the memory 
            // allocated for them.
            if (isDisposing)
            {
                // Release all managed resources here

                // Need to unregister/detach yourself from the events. Always make sure
                // the object is not null first before trying to unregister/detach them!
                // Failure to unregister can be a BIG source of memory leaks
                if (someDisposableObjectWithAnEventHandler != null)
                {                 
                    someDisposableObjectWithAnEventHandler.SomeEvent -= someDelegate;
                    someDisposableObjectWithAnEventHandler.Dispose();
                    someDisposableObjectWithAnEventHandler = null;
                }
                // If this is a WinForm/UI contrlol, uncomment this code
                //if (components != null)
                //{
                //    components.Dispose();
                //}
            }

            // Release all unmanaged resources here
 
 
            // (example)
            if (someComObject != null && Marshal.IsComObject(someComObject))
            {
                Marshal.FinalReleaseComObject(someComObject);
                someComObject = null;
            }
        }
    }
    finally
    {
        this.IsDisposed = true;

        // explicitly call the base class Dispose implementation
        base.Dispose(isDisposing);

    }
}
#endregion IDisposable derived-class implementation


Share