How to implement IDisposable properly

Question

I've seen so much C# code in my time as a developer that attempt to help the GC along by setting variables to null or calling Dispose() on classes (DataSet for example) within thier own classes Dispose() method that I've been wondering if there's any need to implement it in a managed environment.

Is this code a waste of time in its design pattern?

class MyClass : IDisposable 
{
    #region IDisposable Members

    public void Dispose() 
    {
        otherVariable = null;
        if (dataSet != null)
        {
            dataSet.Dispose();
        }
    }

    #endregion
}

Solution 1

The GC does not call .Dispose() (It does, however, call the finalize ~MyClass() method, which you can provide a call to the Dispose() method to have resources automatically managed when the GC decides to clean up your class).

You must always provide a way of disposing internal resources such as DataSets to code that uses your classes (and ensure you actually call .Dispose() or wrap the constructor in a using). Using IDisposable on your classes that use internal resources is highly recommended.

From MSDN:

The primary use of this interface is to release unmanaged resources. The garbage collector automatically releases the memory allocated to a managed object when that object is no longer used. However, it is not possible to predict when garbage collection will occur. Furthermore, the garbage collector has no knowledge of unmanaged resources such as window handles, or open files and streams.

public void Dispose()
{
    otherVariable = null;
    if (dataSet != null)
    {
        dataSet.Dispose();
        dataSet = null;
    }
}

Solution 2

No, Dispose methods are not a waste of time.

The dispose pattern is there to allow a caller to clean up a class as soon as they have finished with it, rather than waiting for the GC to collect it. The delay doesn't matter much for plain heap memory, which is why basic classes like String don't implement it. What Dispose is useful for however is cleaning up unmanaged resources. Somewhere internally, the Dataset class is using an unmanaged resource, so it provides a dispose method to allow you to let it know when that unmanaged resource can be released.

If the pattern has been followed correctly Dataset will also have a finalizer (or some subclass will) which means that if you didn't dispose of it manually, eventually the GC would run, the finalizer would get called and the unmanaged resource would be cleaned up that way. This unmanaged resource might be important though, imagine if it was a file lock, or a database connection, you don't really want to hang around waiting for the GC to run before you can reuse your database connection. Dispose provides a deterministic way of cleaning up resources when they are finished rather than relying on the non-deterministic GC.

As for setting variables to null in a dispose method. It nearly all cases it would be pointless. setting a variable to null removes a reference to that variable, which will make it eligible for garbage collection (if that's the last reference), but as you are disposing of the class anyway, you are likely to be going out of scope for the containing class so the internal class will become eligible for collection anyway.

If you have member variables inside your class that are disposable that you created (not just references you hold), then you should always call dispose on them from your own class's dispose method, but don't bother setting them to null.

Solution 3

Not entirely. If you have member variables which are disposable, then you probably should dispose of it like that. Your object may live longer than the scope of the work it is doing as the garbage collector is not guaranteed to run at any particular time.

Setting managed variables to null is a waste of time though. The object won't get GC'd any faster.

Comments

[supercat]

The purpose of IDisposable isn't to destroy resources--it's to release them. When a FileStream opens a file, it requests that the OS prevent anyone else from using the file until further notice; calling Dispose on the FileStream doesn't destroy the file--to the contrary, it makes it usable by other entities. The GC operates on the principle that there should be no rush to destroy things when there's space to hold them, but once code has no more use for a FileStream, it should try to make the file available again to other code as soon as possible.

[CharithJ]

@supercat: Not quite sure what you are trying to say. I guess FileStream Dispose does release the file handler and let the garbage collector to clear managed instances. It's sole purpose if not to make it usable for others.If you don't open the file exclusively, it may be able to use by others anyway.

[supercat]

The reason GC alone isn't sufficient for things like FileStream isn't that they always holds a file lock in a fashion that interferes with other entities that need to use the file, but that they sometimes do, and it's generally easier to release resources when they're no longer needed, without regard for whether contention would actually exist, than to identify cases where contention won't exist and abandon resources without releasing them in such cases. Note that the sometimes doesn't have to be very "often" to make IDisposable worthwhile. Even if abandoning a file stream...

[supercat]

...would work 99% of the time, it may be easier to have code call Dispose on file streams 100% of the time when they are no longer needed than to make code which generally doesn't dispose of them work 100% of the time.