When is it reasonable to check if this object has been disposed and throw ObjectDisposedException?

Question

In a class that implements IDisposable, when is it reasonable to check if the object has been disposed and throw ObjectDisposedException if it has? In all public methods and properties (except Dispose)? Sometimes? Never?

Cathern

Would you not simply wrap the object in a using statement? Thus the object being disposed when not needed automatically? – Minutiae

@Darren: Then you're assuming that a user of my disposable type behaves correctly. Seldom the case! :-) – Cathern

@Lazarus: GC has nothing to do with IDisposable/Dispose. The GC collects unreferenced objects; it doesn't dispose anything. – Importance

ok nice one.. so if anything goes wrong in dispose then how you will come to know whether object is disposed or not, is that exactly what you are looking for? – Harrier

@Lazarus: I think you misunderstand what dispose means. Dispose is nothing more than a method in an interface a class implements. It really has nothing to do with the GC or the CLR. – Rounders

All - Don't know where my brain was this morning... obviously not enough coffee. I feel very sheepish! :) – Linell

@Johann Gerell: Please don't forget to accept an answer if any helped you. – Rounders

Answer 1

If an object supports IsDisposed, that method itself should never throw; it would be proper for many other methods to throw if IsDisposed returns true, but the exception should be generated by those methods rather than by IsDisposed. One might have a utility method AssertNotDisposed which would throw if an object is disposed, but such behavior would be expected from a method with that name.

Otherwise, I would suggest that there are many cases where it's useful to have an object hold IDisposable object, and be able to Dispose the inner object while maintaining useful state. For example, an object whose function is to show and maintain a modeless dialog box to get information from a user might usefully keep a copy of the contents of the fields even after the box is closed. Such an object should provide a "Close" method which will Dispose the inner Disposable objects but maintain useful state. While it could also have a Dispose method which would call Close but also set a "NoLongerValid" flag that would cause field properties to throw, I don't think that would really add any value.

I will grant that many cases where an object can hold useful state after it has been disposed are indicative of a class which should perhaps be split. For example, the Font class should perhaps be split into a non-disposable FontInfo class (holding a description of a font, but not a GDI handle) and an IDisposable ReadyFont class (inheriting FontInfo, and encapsulating a GDI font object). Routines that use a font could check whether the object they were given was a FontInfo or a ReadyFont; in the former case, they could create a GDI font, use it, and release it; in the latter case, they could use the ReadyFont's GDI font object and release it. The creator of a ReadyFont would then be responsible for ensuring its cleanup.

As it is, I don't know if the system will try to use the GDI object associated with a control's Font property when rendering the control, but I know that it doesn't squawk if the Font is Disposed (even if it's Disposed before assigning it to the Font property!). Controls are certainly capable of creating new GDI fonts if necessary; I don't know whether they always create a new GDI font or if they only do so if the old one has been disposed. The former behavior would seem to be more performant, but unless coded carefully could cause problems if one thread tried to Dispose a Font while another thread was using it.

Palmate

Answer 2

You should implement that check only in methods that don't work on a disposed object.

For example:
If your class closes the database connections or file handles in Dispose, all methods that need those database connections or file handles need to check if the instance is already exposed.

Rounders

Answer 3

The only thing that is really specified is that public void Dispose() itself should not throw anything.

And any method that needs the (un)managed resources should throw.

That leaves, in my mind, only a few disputable cases:

• IsOpen, IsDisposed: I would not throw
• other IsSomeStatus: it depends.
• Length, Count, Position : I don't think a closed Stream has a Length, so throw

It becomes more difficult when the class combines (unrelated) functions like a Stream and a Collection. We just shouldn't do that.

Kassel

Answer 4

As you say, I would implement this check in all public methods and properties except Dispose and IsDisposed. This is the standard pattern to prevent a developer from using your type in the mistaken impression that it's still valid.

Defensive

Some discretion would be in order. How about IsDisposed and/or IsOpen members? – Kassel

I would throw on IsOpen. For IsDisposed, I wouldn't throw. – Defensive

Answer 5

Using a disposed object is a programming error you want to find as fast as possible.

The more places you check it the faster you will find the error.
You should definitely check it where a disposed object will cause some other kind of exception (i.e. null pointer) to not confuse the user.

In other places it depends on the application if it is worth the effort.

Cabriole