Delphi Exception Handling - How to clean up properly?

Question

I'm looking at some code in an application of ours and came across something a little odd from what I normally do. With exception handling and cleanup, we (as well as many other programmers out there, I'm sure) use a Try/Finally block embedded with a Try/Except block. Now I'm used to the Try/Except inside the Try/Finally like so:

Try
  Try
    CouldCauseError(X);
  Except
    HandleError;
  end;
Finally
  FreeAndNil(x);
end;

but this other block of code is reversed as so:

Try
  Try
    CouldCauseError(X);
  Finally
    FreeAndNil(x);
  end;
Except
  HandleError;
end;

Looking around the web, I'm seeing folks doing this both ways, with no explanation as to why. My question is, does it matter which gets the outside block and which gets the inside block? Or will the except and finally sections get handled no matter which way it is structured? Thanks.

Answer 1

One difference is that try..finally..except is potentially vulnerable to an exception masking situation.

Imagine that an exception occurs in CouldCauseError(). Then imagine that the attempt to FreeAndNIL(X) in the finally causes a further exception. The original exception (quite possibly which lead to the instability leading to the FreeAndNIL() exception) is lost. The except handler is now handling the "downstream" exception that occured after the original one.

try..except..finally avoids this of course and should be preferred for this reason (deal with exceptions as close as possible to their source).

The other way to handle a simple case such as this (a single object being cleaned) is to include the cleanup both in the normal flow and in the exception handler:

try
  CouldCauseError(X);
  FreeAndNil(x);
except
  HandleError;
  FreeAndNil(x);
end;

This looks a little scary at first ("I need to be SURE that FreeAndNIL(X) is called, so I HAVE TO HAVE A FINALLY!!") but the only way that the first FreeAndNIL() might not be called is if there is an exception and if there is an exception you are FreeAndNIL()ing as well anyway, and it makes the order of cleanup in the event of an exception a little clearer (in the sense of removing noise that to some extent has to be "filtered" out in order to understand what is going on).

But, I personally do not like it - if you change code in either the exception handler or the normal flow you risk breaking the cleanup behaviour, but depending on the code around such a block, and the size of the block itself, the reduction in "noise" can be argued to be justified in some cases, for the sake of simplification.

However, this relies on the fact that FreeAndNIL() is actually "NILThenFree()"... X is NIL'd before it is Free'd, so if an exception occurs in the FreeAndNIL(X) in the normal flow, then X will be NIL when the exception handler catches the exception raised by X.Free, so it will not attempt to "double-free" X.

Whatever you decide, I hope that helps.