How to properly dispose of potentially lost classes in case of exceptions?
I recently started using the full set of Code Analysis rules in several of our projects. One of the warnings that made me take a closer look at classes IDisposable
is CA2000 . I have many cases where it complains that some disposable objects were not removed in every exception path.
So the question is, what is the more correct way of handling object disposal in case of exceptions in the rest of the method, before returning? For example, this method:
public MyDisposable GetMyDisposable()
{
var disposable = new MyDisposable();
disposable.MethodThatCanThrowExceptions();
return disposable;
}
A warning like this would be raised because if the method MethodThatCanThrowExceptions
does throw an exception, the caller will not receive a one-time instance, and thus no one will be able to dispose of it before the garbage collector starts running.
The sample I used is something like this:
public MyDisposable GetMyDisposable()
{
var disposable = new MyDisposable();
try
{
disposable.MethodThatCanThrowExceptions();
}
catch
{
disposable.Dispose();
throw;
}
return disposable;
}
But I saw some people mention using a block finally
instead of a catch, with a boolean value to signal that there was a problem in the rest of the code:
public MyDisposable GetMyDisposable()
{
bool ok;
var disposable = new MyDisposable();
try
{
disposable.MethodThatCanThrowExceptions();
ok = true;
return disposable;
}
finally
{
if (!ok) disposable.Dispose();
}
}
Between the two, what is the best way to handle this scenario? Is there another, possibly simpler way to handle this situation? I have programmed a bit in C ++ and this language usually uses a class to handle these one-off situations called Scope Guard. From what I've seen, this template supports a method Release
, so you can wrap the code with a delete behavior, but undo the actual delete in any state. In C #, I assume it will look like this:
public MyDisposable GetMyDisposable()
{
using (var disposable = new MyDisposable())
{
disposable.MethodThatCanThrowExceptions();
return release disposable;
}
}
I know such a thing doesn't exist in C #, but it would be nice if there was something equivalent or clean.
source to share
In this particular case (which is rather unusual, I may add), you are passing the responsibility for removing the object from this method to its caller when you return a disposable resource.
Obviously, returning an object already located would be meaningless to the caller, since they can never use it. This precludes the latter option here.
You want to get rid of the object in case you cannot initialize it correctly, which means deleting it in case there is an exception, which is exactly what your first sentence does in the easiest way, your second approach works exactly the same as your first, it's just not as straightforward as it's done.
Using a block finally
(usually hidden inside using
for simplicity) is appropriate when there is a method responsible for disposing of the resource, and where the lifetime of the disposable resource matches the scope of the variable you store it. This tends to be the case for the vast majority of uses of disposable resources, not just that.
source to share
Many languages require annoyingly awkward constructs to detect when exceptions occur without catching them. This usually results in a trade-off between using awkward but semantically accurate code to get around this limitation, not refraining from catching an exception that cannot be dealt with, or tolerating the semantic inaccuracy of catching an exception that cannot be handled and then refactoring it. My personal preference is to use semantically correct but inconvenient code until language developers can write semantically correct code correctly, but many others choose to tolerate the semantic sloppiness of catching and reverse engineer exceptions unconditionally.
Note that there is some benefit to consistently avoiding this interception and re-throwing behavior, but not much will be achieved by avoiding this behavior in multiple places if the rest of one code abounds in it. Tolerant syntactic awkwardness for maintaining semantic cleanliness is IMHO, that's good, but if things are already semantically messy, writing code that is syntactically awkward but can't eliminate the underlying mess might not be worth it.
Of course, if the .NET Maintainers can let the code do the right thing in a syntactically pure fashion, the whole issue will be moot.
source to share