Why is IDisposable implementation designed the way it is [closed]_问答_开发者

Closed. This question needs to be more focused. It is not currently accepting answers.

Want to improve this question? Update the question so it focuses on one problem only by editing this post.

Closed 4 years ago.

Improve this question

Let's take a look at the infamous IDisposable interface:

[ComVi开发者_如何学Csible(true)]
public interface IDisposable
{
    void Dispose();
}

and a typical implementation, as recommended by MSDN (I omitted the check if current object has already been disposed):

public class Base : IDisposable
{
    protected virtual void Dispose(bool disposing)
    {
        if (disposing)
        {
            // release managed
        }
        // release unmanaged
        disposed = true;
    }

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

    ~Base()
    {
        Dispose(false);
    }
}

public class Derived : Base
{
    protected override void Dispose(bool disposing)
    {
        base.Dispose(disposing);
        if (disposing)
        {
            // release managed
        }
        // release unmanaged
        disposed = true;
    }
}

Problem is: I think this implementation is counter-intuitive. And it is also significantly different in base and derived class. Derived class is supposed to assume that base class implemented IDisposable properly and then override Dispose(bool), which is not even a part of the original interface.

I have to admit, I came up with this question because I usually ask junior programmers to implement IDisposable on a job interview. If they don't exactly know how it's supposed to be done, they come up with something close to this:

public class Base : IDisposable
{
    public virtual void Dispose()
    {
        // release managed and unmanaged
        GC.SuppressFinalize(this);
    }

    ~Base()
    {
        // release unmanaged
    }
}

public class Derived : Base
{
    public override void Dispose()
    {
        // release managed and unmanaged
        base.Dispose();
    }

    ~Derived()
    {
        // release unmanaged
    }
}

To me, this implementation is more clear and more consistent. Of course, the bad thing is that we have to release unmanaged resources in two different places, but the important point is that probably over 99% custom classes do not have anything unmanaged to dispose, so they won't need a finalizer anyway. I can't explain to a junior programmer why MSDN implementation is better because I don't really understand it myself.

So I'm wondering, what led to such unusual design decisions (making derived class to override a different method than the one in the interface and making him think about unmanaged resources which it most probably doesn't contain). Any thoughts on this matter?

I usually take out the guesswork for derived classes. Here's my .snippet file:

#region IDisposable pattern
/// <summary>
/// Dispose of (clean up and deallocate) resources used by this class.
/// </summary>
/// <param name="fromUser">
/// True if called directly or indirectly from user code.
/// False if called from the finalizer (i.e. from the class' destructor).
/// </param>
/// <remarks>
/// When called from user code, it is safe to clean up both managed and unmanaged objects.
/// When called from the finalizer, it is only safe to dispose of unmanaged objects.
/// This method should expect to be called multiple times without causing an exception.
/// </remarks>
protected virtual void Dispose(bool fromUser)
    {
    if (fromUser)   // Called from user code rather than the garbage collector
        {
        // Dispose of managed resources (only safe if called directly or indirectly from user code).
        try
            {
      DisposeManagedResources();
            GC.SuppressFinalize(this);  // No need for the Finalizer to do all this again.
            }
        catch (Exception ex)
            {
            //ToDo: Handle any exceptions, for example produce diagnostic trace output.
            //Diagnostics.TraceError("Error when disposing.");
            //Diagnostics.TraceError(ex);
            }
        finally
            {
            //ToDo: Call the base class' Dispose() method if one exists.
            //base.Dispose();
            }
        }
    DisposeUnmanagedResources();
    }
/// <summary>
/// Called when it is time to dispose of all managed resources
/// </summary>
  protected virtual void DisposeManagedResources(){}
/// <summary>
/// Called when it is time to dispose of all unmanaged resources
/// </summary>
  protected virtual void DisposeUnmanagedResources(){}
/// <summary>
/// Dispose of all resources (both managed and unmanaged) used by this class.
/// </summary>
public void Dispose()
    {
    // Call our private Dispose method, indicating that the call originated from user code.
    // Diagnostics.TraceInfo("Disposed by user code.");
    this.Dispose(true);
    }
/// <summary>
/// Destructor, called by the finalizer during garbage collection.
/// There is no guarantee that this method will be called. For example, if <see cref="Dispose"/> has already
/// been called in user code for this object, then finalization may have been suppressed.
/// </summary>
~$MyName$()
    {
    // Call our private Dispose method, indicating that the call originated from the finalizer.
    // Diagnostics.TraceInfo("Finalizer is disposing $MyName$ instance");
    this.Dispose(false);
    }
#endregion

So I'm wondering, what led to such unusual design decisions (making derived class to override a different method than the one in the interface and making him think about unmanaged resources which it most probably doesn't contain). Any thoughts on this matter?

The main issue is that IDisposable was defined AFTER the framework was already designed and in existence. It's meant to handle a situation that managed code is trying to avoid - so it's really an edge case, if a very common one. ;)

This can be seen, btw, if you look at C++/CLI. It was designed after IDisposable, and as a result, implements IDisposable in a much more natural way (destructors [~ClassName]automatically become Dispose, and finalizers [!ClassName] are treated as finalizers).

The other issue is that IDisposable handles multiple situations. I wrote an entire blog series, walking through the different implementations that should be used when wrapping native code, encapsulating a class implementing IDisposable, and using it with factored types.

Technically, you only must implement the interface directly. The design decision to allow for a protected virtual void Dispose(bool disposing) method allows for extra flexibility that wouldn't be easily, and safely, handled in a public interface.

The answer to this and most other API design questions can be found in this book.

Framework Design Guidelines: Conventions, Idioms, and Patterns for Reusable .NET Libraries http://www.amazon.com/gp/product/0321545613?ie=UTF8&tag=bradabramsblo-20&link_code=wql&camp=212361&creative=380601

This is literally the set of rules Microsoft employees use to build .NET APIs. The rules are free (see below), but the book has the commentary that explains the rules. It really is a must have for .NET developers.

http://msdn.microsoft.com/en-us/library/b1yfkh5e.aspx

I'd say this is better:

public class DisposableClass : IDisposable {
  void IDisposable.Dispose() {
    CleanUpManagedResources();
    CleanUpNativeResources();
    GC.SuppressFinalize(this);
  }

  protected virtual void CleanUpManagedResources() { 
    // ...
  }
  protected virtual void CleanUpNativeResources() {
    // ...
  }

  ~DisposableClass() {
    CleanUpNativeResources();
  }
}

MSDN Magazine has an article about this pattern.

This doesn't quite answer the question, but you can use the following code snippet to implement the pattern.

<?xml version="1.0" encoding="utf-8" ?>
<CodeSnippets  xmlns="http://schemas.microsoft.com/VisualStudio/2005/CodeSnippet">
    <CodeSnippet Format="1.0.0">
        <Header>
            <Title>Dispose pattern</Title>
            <Shortcut>dispose</Shortcut>
            <Description>Code snippet for virtual dispose pattern</Description>
            <Author>SLaks</Author>
            <SnippetTypes>
                <SnippetType>Expansion</SnippetType>
                <SnippetType>SurroundsWith</SnippetType>
            </SnippetTypes>
        </Header>
        <Snippet>
            <Declarations>
                <Literal Editable="false">
                    <ID>classname</ID>
                    <ToolTip>Class name</ToolTip>
                    <Default>ClassNamePlaceholder</Default>
                    <Function>ClassName()</Function>
                </Literal>
            </Declarations>
            <Code Language="csharp">
                <![CDATA[
        ///<summary>Releases unmanaged resources and performs other cleanup operations before the $classname$ is reclaimed by garbage collection.</summary>
        ~$classname$() { Dispose(false); }
        ///<summary>Releases all resources used by the $classname$.</summary>
        public void Dispose() { Dispose(true); GC.SuppressFinalize(this); }
        ///<summary>Releases the unmanaged resources used by the $classname$ and optionally releases the managed resources.</summary>
        ///<param name="disposing">true to release both managed and unmanaged resources; false to release only unmanaged resources.</param>
        protected virtual void Dispose(bool disposing) {
            if (disposing) {
                $end$$selected$
            }
        }]]>
            </Code>
        </Snippet>
    </CodeSnippet>
</CodeSnippets>

My understanding is that the entire reason for IDisposable is to release unmanaged resources, so I'm confused why you're stating "99% custom classes do not have anything unmanaged to dispose" - if you're implementing IDisposable it should be because you have unmanaged resources.

MSDN IDisposable

One problem I see with your implementation is that there is potention for the derived class not to call the base class Dispose method. In that case, the GC.SuppressFinalize might not get called when it should, and you would end up also calling the Finalizer. I like Will's solution to ensure that GC.SuppressFinalize is called. The recommended way by MSDN has a similiar feel and ensures that GC.SuppressFinalize is called if the object is Disposed by the developer.

A useful feature of the recommended IDisposable pattern is that it allows a consistent pattern for derived types extending types that implement IDisposable, independent of whether the base type exposes a public parameterless method called Dispose. It's really not too bad a pattern, if one regards the parameter as a dummy which is used simply to give the protected method a different signature from a parameterless Dispose(); the biggest weakness is that the protection against redundant Dispose is not performed in a thread-safe fashion.

A not-so-useful feature of the recommended IDisposable pattern is that it encourages the the usage of finalizers/destructors in many cases where they aren't appropriate. Very seldom should a class which derives from anything other than System.Object have a finalizer for cleanup (classes may have finalizers for purposes of logging a failure to dispose properly). If a class holds references to many managed objects, and also holds an unmanaged resource, the unmanaged resource should be moved out into its own wrapper class, turning it into a managed resource. That wrapper class can either derive from something like SafeHandle, or it can derive from Object and define a finalizer/destructor; either course of action will eliminate the need for a finalizer/destructor in the main class.