Delphi Exception Handling - How to clean up properly?

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
    Ha开发者_JS百科ndleError;
  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.

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.

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.

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;

At first your code looks a little bit strange. I miss the creation of X.

X := CreateAnX
try
  DoSomeThing(X);
finally
  FreeAndNil(x);
end;

It is important. Because if you have code like this

// don't do something like this
try
  X := CreateAnX
  DoSomeThing(X);
finally
  FreeAndNil(x);
end;

you can be lucky and it works. But if the construction fails you can be "lucky" and get an access violation or you have bad luck and get an access violation some times later at an completely different code position.

An alternative could be

X := nil;
try
  X := CreateAnX
  DoSomeThing(X);
finally
  FreeAndNil(x);
end;

Where to use except depends on what is your intention. When you want to catch every exception and know all calling code clean its problems (using try finally) then an outer except-block is the way to go

try
  X := CreateAnX
  try
    DoSomeThing(X);
  finally
    FreeAndNil(x);
  end;
except
  on e: Exception do
    LogException(e)
end;      

But always think about it when you want catch all errors. As an example (and I often see it wrong) don't do it in an Indy OnExecute-handler this way. There you must use something like this

try
  X := CreateAnX
  try
    DoSomeThing(X);
  finally
    FreeAndNil(x);
  end;
except
  on EIdException do
    raise;
  on e: Exception do
    LogException(e)
end;      

If you expect an exception because you throw it or (as an example) a conversation can fail, look for the most inner position to catch the error:

X := CreateAnX
try
  DoSomeThing(X);
  try
    i := StrToInt(X.Text);
  except
    on EConvertError do
      i := 0;
  end;       
finally
  FreeAndNil(x);
end;

don't do it this way

X := CreateAnX
try
  try
    DoSomeThing(X);
    i := StrToInt(X.Text);
  except
    on EConvertError do
      i := 0;
  end;       
finally
  FreeAndNil(x);
end;

Do this only if you expect an EConvertError in DoSomeThing too. If DoSomeThing throws an EConvertError and you don't expect it, your code has a serious problem which need to be corrected. In this situation ensure that the user can save his work (perhaps as a copy, because his work could be damaged) and ensure you get the info about the problem.

At least try except is for handling Exceptions not for hiding them.