A recent internal email thread unearthed extreme differences of opinion about when Dispose(void) should be called on an IDisposable. This led to a long discussion and a realization that -- while it seems like we’ve said everything there is to say about Dispose -- it’s time for some more Dispose guidance. This blog summarizes our initial thoughts about when you should call Dispose. Input was provided by Jeffrey Richter, Mike Boilen, Brian Grunkemeyer, Joe Duffy, and Shawn Farkas. We'd like to hear your feedback as well.
Before diving in, some context:
Let’s look at the different opinions.
“Always call Dispose” camp
The people in this camp, which included me, have been burned by cases in which failing to call Dispose can lead to bugs. For example, failure to call Dispose on a FileStream can lead to hard-to-spot bugs where the file is temporarily unavailable. Even worse, failure to explicitly dispose some .NET crypto classes can lead to an exception thrown on the finalizer thread. Based on these and other examples, we concluded the pit of success is to always call Dispose.
“Avoid calling Dispose” camp
Jeffrey Richter was the lone voice in this camp, but he was up to the challenge. He pointed out that many IDisposables are not as clear cut as FileStream, Socket, etc. For example, a Winforms app has IDisposables that are fonts, controls, etc. For these, explicit cleanup isn’t necessary in mainstream scenarios. Calling Dispose on each of these would be incredibly tedious – similar to (but not as bad as) C++ destructor style of cleanup.
Jeffrey also provided an example where Dispose shouldn't be called. The IAsyncResults returned by FileStream.BeginRead and BeginWrite have a WaitHandle member, which implements IDisposable. Jeffrey said some users think that they should aggressively fetch and dispose this WaitHandle. This can obviously have bad consequences if done prematurely, but it has another problem. The WaitHandle is lazily allocated, so fetching it just to dispose it causes an unnecessary allocation (i.e. negatively impacts performance).
He pointed out similar APIs where there is confusion over whether to call Dispose; in general, these are APIs in where there is ambiguity about who owns the IDisposable.
Well, everyone...or no one. (Actually, probably Jeffrey, given that I didn't think he could budge my opinion at all.)
In any case, from our discussion, it was obvious we haven’t enunciated clear guidance about when to call Dispose.
Given the already confusing state of Dispose, we’d like to keep this guidance as simple as possible. Previously (before that email thread), I thought we wanted to tell users to always call Dispose, since doing so prevents bad side effects described above. The IAsyncResult / unnecessary allocation example could be solved by telling users not to aggressively fetch and dispose members. (In fact, this needs to be advertised no matter what.) This guidance is very simple. So why complicate things?
Jeffrey’s point about the impact of this guidance on WinForms-like apps is crucial. Having to call Dispose on each font and control in a WinForms app could significantly impact coding patterns and would be viewed as tedious. It’s not _as_ tedious as C++ destructors (since at least managed memory is handled), but it’s still a lot of bookkeeping that would be nice to avoid, as long as it's safe.
Proposed guidance about when to call Dispose (draft)
The simplest story we found combining these two concerns (correctness and usability) was to divide IDisposables into resource categories and give specific guidance for each category. The key observation, provided by Mike Boilen, is that you should call Dispose when failing to do so can lead to visible side effects. This approach covers most of our Dispose concerns; special cases are listed in the next section.
1. Named/shared OS resources
2. Other/unnamed resources
To keep this simple for users, we could flag in the docs the classes that you must call Dispose on (i.e. resource category 1). Guidance for resource category 2 is left vague at the moment; the intent for now is to get feedback about whether this approach addresses usability concerns, while remaining as simple as possible.
*Scenarios that expect to stress the resource may hit limits and should consider calling Dispose.
Other Dispose-related special cases
Unfortunately we have to complicate the story. These will also have to be handled on a case-by-case basis.
1. Classes with very different lifetimes: We’d like to recommend not holding strong references to objects that have shorter lifetime
2. Impersonation-related problems: Dispose must be called for crypto classes in which finalizer thread runs under different identity and process is ripped.
3. Bad/degenerate cases: if we suggest not to call Dispose for resource category 2, and a class using one of those resources has not handled cleanup via safehandles or finalizers, then we’re causing a new problem. Do we even care about this?
4. Ambiguous resource type: for some APIs that return an IDisposable, it’s not obvious what kind of resource is wrapped. If failure to call Dispose for any of the resources may lead to observable side effects, the docs must call this out.
5. Ambiguous ownership: for some APIs that return an IDisposable, it’s not clear whether the method allocated the IDisposable and you own the single reference, or you’re referencing a shared instance. Ownership ambiguity will require clarification in docs. In any case, you shouldn’t fetch IDisposable members and dispose them, as in the IAsyncResult case.
Whatever distinction we eventually use, API docs should explicitly call out IDisposables that must be Disposed.