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

The finally and except will both trigger, the order is up to you. It depends on what you want to do in your finally or except block. Do you want to free something that is used in the except block? Place finally around the except block.

Answer 2

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.

Answer 3

It all depends if the code in your finally block can raise an exception itself (then it needs to be protected by an upper level try except), or if you need something in your exception handling that should be freed later (then it needs to be freed in an upper level finally block).

All that means that sometimes you can even have some code like:

  try
    try
      try
        CouldCauseError(X);
      except
        HandleErrorWith(X);
      end;
    finally
      FreeAndNil(X); // and/or any resource cleanup
    end;
  except
    CatchAllError;
  end;