Condividi tramite

When to call Dispose

  • Articolo

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:

  • The question of when to call Dispose is just a small piece of the Dispose puzzle. For information about correctly implementing the Dispose pattern, see the updated Dispose guidelines on Joe Duffy’s blog.
  • As a quick refresher, calling Dispose(void) releases resources deterministically (as opposed to nondeterministic cleanup at finalization).

The Debate

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.

Who won?

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.

Resource Categories

1. Named/shared OS resources

  • Examples: files, sockets, named pipes, memory mapped files
  • Failure to call Dispose: likely to have visible side effects. Even if the resource is wrapped with a SafeHandle or class has a finalizer, problems can manifest as the resource being unavailable for some period of time
  • When to call Dispose: if you own it, Dispose it

2. Other/unnamed resources

  • Examples: native fonts, controls, bitmaps (native memory)
  • Failure to call Dispose: only has visible side effects when large amounts are used. These resources have higher limits than resource category 1. In “typical” use, limits are not hit*
  • When to call Dispose: only if limit is likely to be hit. Rely on GC cleanup for typical use

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.

What’s next?

Whatever distinction we eventually use, API docs should explicitly call out IDisposables that must be Disposed. 

Any comments?

Comments

  • Anonymous
    November 05, 2008
    PingBack from http://mstechnews.info/2008/11/when-to-call-dispose/

  • Anonymous
    November 06, 2008
    Hi Kim, There's another Dispose-related special case: "IDisposable is not implemented correctly". IDisposable on ClientBase<T> (from WCF) is implemented in a way that can sometimes cause it to throw (which violates the "Dispose should never throw" guideline for implementing the dispose pattern). The official guideline for WCF clients (http://msdn.microsoft.com/en-us/library/aa355056.aspx) now recommends that the "using" statement not be used (and thus Dispose not be called), and instead suggests a complicated series of multiple catch blocks. In a forum post (http://social.msdn.microsoft.com/Forums/en-US/wcf/thread/b95b91c7-d498-446c-b38f-ef132989c154), Brian McNamara wrote, "You can argue (and some of us did) that we should have removed [IDisposable] from those two classes [ServiceHost and ClientBase]", indicating that perhaps the implementation of IDisposable on these WCF classes was a mistake.

  • Anonymous
    November 06, 2008
    As I'm sure you know, there is a cost to letting the finalizer clean up finalizable objects. (Chris Brumme goes into some reasons at http://blogs.msdn.com/cbrumme/archive/2004/02/20/77460.aspx, which include promotion of objects, or possibly object graphs, to the next GC generation, and the current CLR only having one finalizer thread, which can limit scalability.) Thus, I would suggest adding another reason to call Dispose to category #2: When the cost of finalization is measurably affecting performance or memory use.

  • Anonymous
    November 06, 2008
    I would argue that it's WinForm that's at fault for not using the IDisposable contract correctly. They should simply have called their method something else, since not calling Dispose in this case is non-observable.

  • Anonymous
    November 06, 2008
    .NET What's New in the BCL in .NET 4.0 NetMon API &#8211; Capture, Parse and and Capture File Access

  • Anonymous
    November 06, 2008
    .NETWhat'sNewintheBCLin.NET4.0NetMonAPI–Capture,ParseandandCaptureFileAccess(wi...

  • Anonymous
    November 06, 2008
    Hi Bradley, It was nice to meet you at PDC last week. I wasn't aware of the WCF ServiceHost and ClientBase examples. I read the forum post and it's interesting that matching up Close and Dispose (after the fact) was involved in this behavior. It's indeed a common expectation that they do the same. As I talked about in another post, I believe we'd have fewer Dispose headaches if we'd never given the guidance of hiding Dispose in favor of domain-specific names. Finalization cost is an interesting point. It will be important to clarify that for category 2, we're just saying that calling Dispose isn't necessary from a correctness standpoint, as opposed to "don't call Dispose". There are cases where users would want to. Thanks, Kim

  • Anonymous
    November 06, 2008
    Hi Ziv, Unfortunately not calling Dispose could be observable in extreme cases. Best example I can think of would be a targeted stress test. :) These are cases where it's hard to hit the limits, but we want to make sure apps that happen to do this are aware of the problem. Thanks, Kim

  • Anonymous
    November 12, 2008
    The comment has been removed

  • Anonymous
    November 16, 2008
    A typical ambiguous case can be found with readers and writers (StreamReader, StreamWriter, BinaryReader, BinaryWriter...). They implement IDisposable just in case the have ownership on the underlying stream, but the documentation is not clear about it. When you need to keep the stream open (to use several differents writers, or when using a memory stream), you must use Flush and not Close (= Dispose). My opinion is that you should always call Flush, and then call Dispose only if you need to get rid of the underlying stream. A bit confusing...