views:

171

answers:

4

I have a class (say MyClass) that uses (has as a private field) a TcpClient object. MyClass implements IDisposable calling TcpClient.Close in the Dispose method.

My question is should MyClass also implement a finalizer to call Dispose(bool Disposing) to free the TcpClient’s unmanaged resources in case MyClass.Dispose is not called by the calling code?

Thanks

A: 

Yes you should - Microsoft even recommends it.

Just remember that belt-and-suspenders code never gets you called into the office at 2:00AM :)

Andrew Hare
+4  A: 

No you shouldn't.

Because you should never call a method on an other object in a finalizer, it could have been finalized before your object.

The finalizer of your TcpClient will be called by the garbage collector, so let him do.

The pattern in Dispose is :

protected virtual void Dispose(bool disposing)
{
   if (disposing)
   { 
      // dispose managed resources (here your TcpClient)
   }

   // dispose your unmanaged resources 
   // handles etc using static interop methods.
}
Think Before Coding
+1  A: 

No you don't have to. TcpClient is a wrapper class around the unmanaged socket and there for it is managed the way it should be disposed. What you have done is enough.

Mohammadreza
+1  A: 

No you shouldn't.

From this excellent post:

Finalization is fundamentally different from ending an object’s lifetime. From a correctness point of view, there is no ordering between finalizers (outside of a special case for critical finalizers), so if you have two objects that the GC thinks are dead at the same time, you cannot predict which finalizer will complete first. This means you can’t have a finalizer that interacts with any finalizable objects stored in instance variables.

This is my reference implementation of the disposable/finalize pattern with comments explaining when to use what:

/// <summary>
    /// Example of how to implement the dispose pattern.
    /// </summary>
    public class PerfectDisposableClass : IDisposable
    {
        /// <summary>
        /// Type constructor.
        /// </summary>
        public PerfectDisposableClass()
        {
            Console.WriteLine( "Constructing" );    
        }

        /// <summary>
        /// Dispose method, disposes resources and suppresses finalization.
        /// </summary>
        public void Dispose()
        {
            Dispose( true );
            GC.SuppressFinalize(this);
        }

        /// <summary>
        /// Disposes resources used by class.
        /// </summary>
        /// <param name="disposing">
        /// True if called from user code, false if called from finalizer.
        /// When true will also call dispose for any managed objects.
        /// </param>
        protected virtual void Dispose(bool disposing)
        {
            Console.WriteLine( "Dispose(bool disposing) called, disposing = {0}", disposing );

            if (disposing)
            {
                // Call dispose here for any managed objects (use lock if thread safety required), e.g.
                // 
                // if( myManagedObject != null )
                // {
                //     myManagedObject.Dispose();
                //     myManagedObject = null;
                //  }
            }
        }

        /// <summary>
        /// Called by the finalizer.  Note that if <see cref="Dispose()"/> has been called then finalization will 
        /// have been suspended and therefore never called.
        /// </summary>
        /// <remarks>
        /// This is a safety net to ensure that our resources (managed and unmanaged) are cleaned up after usage as
        /// we can guarantee that the finalizer will be called at some point providing <see cref="Dispose()"/> is
        /// not called.
        /// Adding a finalizer, however, IS EXPENSIVE.  So only add if using unmanaged resources (and even then try
        /// and avoid a finalizer by using <see cref="SafeHandle"/>).
        /// </remarks>
        ~PerfectDisposableClass()
        {
            Dispose(false);
        }
    }
ng5000
Any comments on my *Perfect* (I don't believe that for an instant, but it's my best to date ;) class welcome.
ng5000