Should IDisposable.Dispose() be made safe to call multiple times?

Question

Should implementations of IDisposable make Dispose() safe to call multiple times? Or the opposite? What approach to most .NET Framework classes take?

Specifically, is it safe to call System.Data.Linq.DataContext.Dispose() multiple times?

The reason I ask is because I am wondering if this extra protection is necessary:

public override void Dispose(bool disposing)
{
    // Extra protection...
    if (this.obj != null)
    {
        this.obj.Dispose();
        this.obj = null;
    }

    // Versus simply...
    this.obj.Dispose();

    base.Dispose(disposing);
}

when disposing IDisposable members of a class, or whether I should just call this.obj.Dispose() without concern for whether it has been previously called.

Plectognath

I prefer the if (!= null) syntax. You should also guard the aggregated object's dispose with the disposing flag. – Havre

If your object also has a finalizer, make sure you only call this.obj.Dispose() if 'disposing' is false. Otherwise, you might be accessing another object while from within the finalizer, which might already have been finalized (finalization order is undefined and there's no safe finalization order for circular references). If I was Microsoft, I'd rename that 'disposing' flag to 'disposedExplicitly' or 'notInFinalizer' :) – Thulium

Answer 1

You should be safe to call it more than once, though you should probably avoid it if you can.

From the MSDN page on IDisposable.Dispose():

If an object's Dispose method is called more than once, the object must ignore all calls after the first one. The object must not throw an exception if its Dispose method is called multiple times.

Incandescence

Answer 2

Yes, your implementations of IDisposable.Dispose() should tolerate being called multiple times. After the first call to Dispose(), all other calls can just return immediately.

I would prefer the first part of your code example, to dispose and null the local variables as you go.

Be aware that your .Dispose() may be called multiple times even if you implement Dispose and null patterns in your code. If multiple consumers hold a reference to the same disposable object, then that object's Dispose will probably be called multiple times as those consumers drop their references to it.

Theatricals

Answer 3

Objects should be tolerant to having Dispose called more than once, since--especially in the presence of exceptions--it may be difficult for cleanup code to know for certain which things have been cleaned up and which have not. Nulling out IDisposable fields as they are cleaned up (and being tolerant of ones that are already null) will make it easier to avoid redundant calls to Dispose, but it doesn't really cost anything to make objects tolerate multiple disposal, and it helps to avoid ickiness in some Exception-throwing situations.

Colwen

Answer 4

If an object is disposed you shouldn't be disposing of it a second time.This helps you to not prolong the life of the object in the Garbage Collector.

A pattern I use normally is this.

// A base class that implements IDisposable.
// By implementing IDisposable, you are announcing that
// instances of this type allocate scarce resources.
public class BaseClass: IDisposable
{
    /// <summary>
    /// A value indicating whether this instance of the given entity has 
    /// been disposed.
    /// </summary>
    /// <value>
    /// <see langword="true"/> if this instance has been disposed; otherwise, 
    /// <see langword="false"/>.
    /// </value>
    /// <remarks>
    /// If the entity is disposed, it must not be disposed a second
    /// time. The isDisposed field is set the first time the entity
    /// is disposed. If the isDisposed field is true, then the Dispose()
    /// method will not dispose again. This help not to prolong the entity's
    /// life in the Garbage Collector.
    /// </remarks>
    private bool isDisposed;

   /// <summary>
    /// Disposes the object and frees resources for the Garbage Collector.
    /// </summary>
    public void Dispose()
    {
        this.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);
    }

    /// <summary>
    /// Disposes the object and frees resources for the Garbage Collector.
    /// </summary>
    /// <param name="disposing">If true, the object gets disposed.</param>
    protected virtual void Dispose(bool disposing)
    {
        if (this.isDisposed)
        {
            return;
        }

        if (disposing)
        {
            // Dispose of any managed resources here.

        }

        // Call the appropriate methods to clean up
        // unmanaged resources here.
        // Note disposing is done.
        this.isDisposed = true;

    }

    // Use C# destructor syntax for finalization code.
    // This destructor will run only if the Dispose method
    // does not get called.
    // It gives your base class the opportunity to finalize.
    // Do not provide destructors in types derived from this class.
    ~BaseClass()
    {
        // Do not re-create Dispose clean-up code here.
        // Calling Dispose(false) is optimal in terms of
        // readability and maintainability.
        Dispose(false);
    }      
}

Cavender

How does disposing an object a second time prolong the lifetime of the object? All the garbage collector cares about is whether there's a reference left to it or not. Also, you code needlessly calls GC.SuppressFinalize(), claiming the object to be in the finalization queue, which it isn't, since it doesn't have a finalizer. The GC will not look for IDisposable in any way (which would be dangerous anyway because an explicit Dispose() can access methods on other objects, which the finalizer shouldn't do). – Thulium

@Cycon: I realised I hadn't posted a complete enough code sample following your comments. By prolong the life I mean it will persist in the finalization queue. I've updated to add finalization code and fix another small bug I spotted. – Cavender

Note that this pattern should ONLY be used in the case where you're actually dealing with unmanaged resources and you have logic that can be executed without calling referencing any other managed instances in the code. If all you're dealing with are other managed IDisposable instances that need to be cleaned up (or you're using IDisposable for some kind of language convenience like a using block), then you should not be doing anything with a finalizer. Just having a finalizer on your object alters the way the GC collects it (it has to be placed into the finalizer queue). – Incandescence