2015-03-20

Mandatory disposal vs. the "Dispose-disposing" abomination

This article started as a stackoverflow answer, and then I copied it over here to expand on it.

For a discussion of the same issue but in java-oriented terms, see this stackoverflow answer of mine: Is overriding Object.finalize() really bad? http://programmers.stackexchange.com/a/288724/41811

There is this practice which is unfortunately very prevalent in the C# world, of implementing object disposal using the ugly, clunky, inelegant, ill-conceived, and error prone idiom known as IDisposable-disposing. MSDN describes it in length, and lots of people swear by it, follow it religiously, write walls of text discussing precisely how it should be done and precisely how it works, and precisely how they arrived at this particular way of doing it, etc.

(Please note that what I am calling ugly here is not the object disposal pattern itself; what I am calling ugly is the particular idiom of implementing an extra Dispose method with a bool disposing parameter.)

This idiom was invented under the assumption that the invocation of IDisposable.Dispose() is something optional, or in any case something which might be OK to forget, in combination with the fact that it is impossible to guarantee that our objects' destructor will always be invoked by the garbage collector to clean up resources.  So, people tend to make their best effort to invoke their IDisposable.Dispose() methods, and in case they forget, they also give it one more try from within the destructor. You know, just in case.

But then your IDisposable.Dispose() might have both managed and unmanaged objects to clean up, but the managed ones cannot be cleaned up when IDisposable.Dispose() is invoked from within the destructor, because they have already been taken care of by the garbage collector at that point in time, so there is this need for a separate Dispose() method that accepts a bool disposing flag to know if both managed and unmanaged objects should be cleaned up, or only unmanaged ones.  In addition to that, you have to guard against disposing your unmanaged resources twice, so you have to either have a disposed member variable to keep track of whether IDisposable.Dispose() has been invoked, or, better yet, invoke GC.SuppressFinalize() from within IDisposable.Dispose() so that finalization can be skipped if the object is known to have been properly disposed.

Excuse me, but this is just insane.

I go by Einstein's axiom, which says that things should be as simple as possible, but not simpler. Clearly, we cannot omit the cleaning up of resources, so the simplest possible solution has to include at least that. The next simplest solution involves always disposing everything at the precise point in time that it is supposed to be disposed, without complicating things by bringing into the picture the destructor as an alternative fall back. So, that has to be it, according to my line of thinking.  I call it "Mandatory Disposal".

Now, strictly speaking, it is of course impossible to guarantee that every single programmer out there will always remember to make sure that IDisposable.Dispose() will be invoked, but what can be done, is that the destructor can be used to detect such omissions.  The crux of the matter is what we do once we have detected such an omission: we do not attempt to correct it; instead, we generate an error message.

It is very simple, really.  (Duh!)  All the destructor has to do is generate a log entry if it detects that the disposed flag of the IDisposable object was never set to true. So, the use of the destructor is not an integral part of our disposal strategy, but it is our quality assurance mechanism. And since this is a debug-mode only test, we can place our entire destructor inside an #if DEBUG block, so we never incur any destruction penalty in a production environment. The IDisposable-disposing idiom prescribes that GC.SuppressFinalize() should be invoked precisely in order to lessen the overhead of finalization, but with Mandatory Disposal it is possible to completely avoid all finalization in our production build.

What it boils down to is the eternal hard error doctrine vs. soft error doctrine argument:

The IDisposable-disposing idiom of object disposal is a soft error doctrine approach, allowing the programmer to forget to invoke IDisposable.Dispose() and attempting to somehow (magically) make things right in the end. My Mandatory Disposal idiom is a hard error doctrine approach, requiring that the programmer must always make sure that IDisposable.Dispose() gets invoked, under penalty of error.

The error is mitigated in this special case, from the standard assertion failure exception that the hard error doctrine usually calls for, to a mere "error"-level message in the debug log, since the error is detected during finalization, and by that time it is too late for fail-fast measures anyway.

The Mandatory Disposal mechanism works best if the DEBUG version of our application performs a full garbage disposal before quitting, so as to guarantee that all destructors will be invoked, and thus detect any IDisposable objects that we forgot to dispose.  Also, the Mandatory Disposal mechanism works best if we avoid using static references to objects, because such references prevent objects from being garbage collected.

Once you start using Mandatory Disposal, one issue you inevitably encounter is that you discover objects which you forgot to dispose of, but you do not know how to fix them because you have no idea where they were allocated.  There is a nice way of solving this problem without considerable complications, but this will be the subject of another post.

More on this issue, but in java-oriented terms, in this stackoverflow answer of mine: Is overriding Object.finalize() really bad? http://programmers.stackexchange.com/a/288724/41811

No comments:

Post a Comment