Should we implement IDisposable if one member is IDisposable?

Question

I think so. But take a look at a built-in class in ASP.NET:

public sealed class HttpPostedFile
{
    public Stream InputStream { get; }   // Stream implements IDisposable
    // other properties and methods
}

Suppose I have an instance of HttpPostedFile called file. Since there is no Dispose method to explicitly invoke, file.InputStream.Dispose() won't be invoked until it's destructed, which I think goes against the original intention of IDisposable. I think the correct implementation should contain a standard IDisposable implementation. So, if one of the members implements IDisposable, the class needs to implement it too.

What are your opinions? It seems to be a bit complicated.

Answer 1

In general, you should implement IDisposable if you own the resource represented by the property - see this question for a discussion on this subject.

I'd say that because HttpPostedFile is instantiated during processing of an HTTP request, it doesn't own the stream, and hence doesn't dispose it. The stream will be disposed when the HTTP request processing finishes.

Answer 2

If your class creates one or more IDisposable objects and holds the only references to them, then your class should almost certainly implement IDisposable and dispose the IDisposable objects it created. If one or more IDisposable objects will be passed into the constructor of your class, then you need to consider a few scenarios:

1. Your creator may want to keep using the IDisposable after you're done with it, and will certainly know when it's no longer needed (the semantics of your class would let him know you're done with it).
2. Your creator won't want to use the IDisposable after you're done with it, and may not know when you're going to be done with it.
3. Your class may be used in some circumstances corresponding to (1) above, and in some circumstances (2), but your creator will know in advance which circumstance applies.
4. Your creator can't predict whether he's going to want to keep using the object after you're done with it.

For scenario #1, there's no need for you to implement IDisposable, though it might not be a bad idea to implement a do-nothing IDisposable handler and have your consumers use it, in case another scenario applies in future.

For scenario #2, your object should take ownership of the IDisposable, and should Dispose it when done. I don't really like having objects take unconditional ownership of IDisposables; I prefer to implement things as in #3.

There are two ways of handling #3. The one I prefer is for your object to take a parameter (either a Boolean or an enum) along with the IDisposable, indicating whether it is supposed to take ownership of the IDisposable. Your class unconditionally implements IDisposable; the implementation disposes of any objects it has taken ownership of, but not those it hasn't. An alternative is to have two subclasses with a common base class - one subclass implements IDisposable and the other does not. I prefer the former pattern, because it allows for the addition of a method to replace an IDisposable with a new one (of which it may or may not take ownership). For example, if I were implementing a control with an Image property, I would have a SetImage method which with a parameter to specify whether the control should own the passed-in image; that method would Dispose the old image if it owned it, and could then either take ownership of the new image or not.

bool OwnMyImage;
Image MyImage;
void SetImage(Image NewImage, bool TakeOwnership)
{
    IDisposable oldDisposable; // Could reuse one variable for multiple IDisposables
    if (OwnMyImage)
    {
        oldDisposable = Threading.Interlocked.Exchange(MyImage, null);
        if (oldDisposable != null)
        {
            oldDisposable.Dispose();
        }
    }
    OwmMyImage = TakeOwnership;
    MyImage = NewImage;
}

Scenario #4 is complicated; the best way to handle it is probably for your object to implement IDisposable by raising a Disposed event. Your creator can use that event to do either Dispose the object if you were the last one using it, or adjust a flag or counter so that other code will know the object shouldn't be left undisposed on your behalf.

Answer 3

It depends.

Stream is also implemented by TextStream (possibly on top of StringBuilder), so no unmanaged resources are required.

HttpPostedFile may not use any unmanaged resources at all, so it's safe to postpone deconstruction until the garbage collector sees fit.