C# calling IDisposable.Dispose() vs making object null

Question

Consider the following code:

A. Toy Class

class Toy
{
    private string name;
    public string Name
    {
        get { return name; }
        set { name = value; }
    }

    private ToyAddon addOn;
    public ToyAddon AddOn
    {
        get { return addOn; }
        set { addOn = value; }
    }

    public void RemoveAddons()
    {
        /*
         * someone said: this could lead to possible memory leak
         * if this class implements IDisposable,
         * then we are not disposing any of the resource 
         * */
        //this.addOn = null; 

        /* instead use this */
        var disposableToyAddOn = this.addOn as IDisposable;
        if (disposableToyAddOn != null)
            disposableToyAddOn.Dispose();

        this.addOn = null;

    }
    public override string ToString()
    {
        return String.Format("{0}, {1}",
            this.name, 
            (this.AddOn == null) ? "" : addOn.AddOnName);
    }
}

B. Toy Addon

class ToyAddon
{
    private string addOnName;

    public string AddOnName
    {
        get { return addOnName; }
        set { addOnName = value; }
    }

}

C. Main Program

class Program
{
    static void Main(string[] args)
    {
        ToyAddon tAdd = new ToyAddon();
        tAdd.AddOnName = "Amazing AddOn";
        Toy t = new Toy();

        t.Name = "Amazing Toy";
        t.AddOn = tAdd;
        t.RemoveAddons();

        Console.WriteLine(t.ToString());


    }
}

Now it was suggested to me that check if a "having-a" object is implementing IDisposable then call the dispose method of it. (please check the comments in Toy class)

IMHO, if I make the reference null, then the object on heap would be marked for Collection by GC.

It would be appreciated if this reasoning can be made on, what happens on heap and stack, and role of GC in that if the class (as ToyAddOn ) implements IDisposable vs if it does not.

Answer 1

If you don't Dispose of a class manually the unmanaged resources (if any) it holds will only be cleaned up when the GC detects there is enough memory pressure to require a garbage collection and the object gets GCed and Finalized.

Not all things that require disposing add memory pressure, they could be holding on to things like Window Handles.

If the memory pressure never gets high enough it is possible that you never garbage collect those values before you run out of the resource the dispose released. You frequently see this when people put Bitmap's in a tight loop but do not dispose them, each bitmap uses a GDI+ handle and not does count toward the managed memory pressure and will not trigger a GC. This often cause people to get OutOfMemoryExceptions when they still have plenty of memory because the real thing they ran out of was GDI+ handles.

By explicitly disposing of your resources you don't need to "hope you get lucky" and have your object get Finalized before you run out of unmanaged resources that it was holding.

Stack and heap have nothing to do with this.

Answer 2

I agree with those three answers posted. Just to simplify the answer for your question :

/* * someone said: this could lead to possible memory leak * if this class implements IDisposable, * then we are not disposing any of the resource * */ //this.addOn = null;

What someone said is not quite applicable for your example. But it is absolutely correct, in general.

Assume Toy class is living much longer than ToyAddon. And ToyAddon class has an event subscription to Toy Class. If so, ToyAddon should unsubscribe from those subscription in its Dispose method. If you didn't call ToyAddon's Dispose method after removing it from Toy instance, ToyAddon instace will live in memory as long as Toy instance lives.

And also, even though you call Dispose in above case, still there is a reference to ToyAddon through 'addOn'. So it is required to set addOn to null in order to get rid of it completely.

IMHO, if I make the reference null, then the object on heap would be marked for Collection by GC.

Correct in your case, wrong in general as I explained above.

Answer 3

The pattern of try-casting something to IDisposable and disposing it if it implements that interface is highly indicative of a design flaw. If the type of reference code holds doesn't implement IDisposable, that's a very strong indication that either:

1. Holders of a reference of that type shouldn't be disposing it; even if derived types implement Disposable, responsibility for their cleanup should lie with the code that created instances of those types (and holds references of those types).

2. The base type should have implemented IDisposable (even if 99% of implementations would do nothing) because some instances will have lifetimes outside the control of anyone who knows whether or not they actually need cleanup. A prime example of this latter situations is IEnumerator<T>. Although IEnumerator omitted IDisposable, the need for it became apparent soon after release; when Microsoft added IEnumerator<T> they included IDisposable directly. Although most implementations of IEnumerator<T> can be abandoned without cleanup, there is no way that code that calls GetEnumerator on a collection has no way of knowing whether the implementation of IEnumerator<T> might be one of the rare ones that needs cleanup, and there's no practical way an IEnumerable<T> whose GetEnumerator method creates an IEnumerator<T> can be expected know when the client is done with it, so nobody but the client will be in a position to Dispose the IEnumerator<T>.

Determine whether any instances of ToyAddOn that implement Disposable are at all likely to have their lifetime end at a time when while the only references are held by code that has no idea whether they need cleanup. If so, make ToyAddOn implement Disposable. If not, leave cleanup to code that knows that it's required.

Answer 4

Now it was suggested to me that check if a "having-a" object is implementing IDisposable then call the dispose method of it.

IDisposable is a pattern that is used to free unmanaged resources that need to be explicitly released, as the GC isn't aware of them. Usually, a class holding unmanaged resources will also implement a finalizer, and for that fact it is actually pro-longing the lifetime of the object, more than necessary. Dispose in those situations usually invokes a call to GC.SupressFinalize, to remove the said object from the finalization queue.

It is usually a good pattern, if you have an IDisposable object, to implement IDisposable yourself in order to make sure the underlying resource gets disposed.

IMHO, if I make the reference null, then the object on heap would be marked for Collection by GC.

If the member you want to "null out" isn't a static member, then there is (usually) no reason to do so. The compiler is smart enough to optimize it away, and the GC is smart enough to know that there is no longer a reference to the variable and clean it up.

Edit:

As @ScottChamberlain points out, there are cases when a disposable object is still held a reference to, and hence the GC doesn't count it for GC. When disposing of it and nulling it out as well, you hint the GC that it is "ready for collection".