In Delphi 7 a the creation of an object was like this:
A := TTest.Create;
try
...
finally
A.Free;
end;
However in a blog article Marco Cantù says that in Embercadero they use
A1 := nil;
A2 := nil;
try
A1 := TTest.Create;
A2 := TTest.Create;
...
finally
A2.Free;
A1.Free;
end;
Was something changed in the logic of try finally block during version upgrades? The second example seems like a typical error to me!
Both are acceptable patterns. And this is not something that's changed.
First let's cover the one you're familiar with and why it's correct.
{ Note that here as a local variable, A may be non-nil, but
still not refer to a valid object. }
A := TTest.Create;
try
{ Enter try/finally if and only if Create succeeds. }
finally
{ We are guaranteed that A was created. }
A.Free;
end;
In the above: If A had been assigned after try, then there's a possibility that Create could fail and jump here. This would attempt to Free an object from an undefined location in memory. It can lead to an Access Violation or unstable behaviour. Note that the compiler would also give a warning that on A.Free; that A might be uninitialised. This is because of the possibility of jumping to the finally block before A is assigned due to an exception in the constructor.
So why is Marco's code acceptable?
A1 := nil; { Guarantees A1 initialised *before* try }
A2 := nil; { Guarantees A2 initialised *before* try }
try
A1 := TTest.Create;
A2 := TTest.Create;
...
finally
{ If either Create fails, A2 is guaranteed to be nil.
And Free is safe from a nil reference. }
A2.Free;
{ Similarly, if A1's Create fails, Free is still safe.
And if A1's create succeeds, but A2's fails: A1 refers to a valid
object and can be destroyed. }
A1.Free;
end;
Note that Marco's code relies on some subtleties of the behaviour of Free(). See the following Q&A for more information:
The purpose behind the technique is to avoid nested try..finally blocks which can get messy. E.g.
A1 := TTest.Create;
try
A2 := TTest.Create;
try
{...}
finally
A2.Free;
end;
finally
A1.Free;
end;
Marco's code reduces nesting levels, but requires 'pre-initialisation' of the local references.
Victoria has raised a caveat that if the destructor for A2 fails in Marco's code, then A1 will not be Freed. This would be a certain memory leak. However, I'd argue that as soon as any destructor fails:
So the best advice I can offer is: take care to ensure the correctness of your destructors.
There is one important addition to Craig's answer and explanation why using single try..finally block is also fine.
A1 := nil;
A2 := nil;
try
A1 := TTest.Create;
A2 := TTest.Create;
...
finally
A2.Free;
A1.Free;
end;
Potential problem with above code is that if A2 destructor raises or causes an exception A1 destructor will not be called.
From that standpoint above code is broken. But, the whole Delphi memory management is built on top of premise that destructors should never ever raise or cause an exception. Or in other words, if there is a code in destructor that can cause an exception, destructor must handle that exception on site and not allow it to escape.
What is the problem with destructors raising exceptions?
Raising exception in destructor will break calling destructor chain. Depending on the code, inherited destructors may not be called and they will not be able to perform proper cleanup resulting in memory or resources leak.
But even more important fact is that even if you have a single destructor that causes an unhandled exception, the FreeInstance method that releases object instance memory allocated on the heap will not be called and you will leak that object instance's memory.
That means following code will leak TTest instance heap memory if A.Free contains code that will cause an exception.
A := TTest.Create;
try
...
finally
A.Free;
end;
The same is valid for nested try...finally blocks. If any of the destructors causes unhandled exception memory will be leaked.
While nested try...finally blocks will leak less memory than single try...finally blocks they will still cause the leak.
A1 := TTest.Create;
try
A2 := TTest.Create;
try
...
finally
A2.Free;
end;
finally
A1.Free;
end;
You can use as many try...finally blocks as you wish, or you can even use interfaces and automatic memory management, but an exception raising (causing) destructor will always leak some memory. Period.
How about BeforeDestruction?
The same rule that applies to destructors applies to BeforeDestruction method. Unhandled exception in BeforeDestruction will break object release process and destructor chain as well as FreeInstance will not be called resulting in memory leak.
Of course, proper handling of any exceptions inside BeforeDestruction method or destructors implies that you must make sure that all code that is responsible for any kind of cleanup, including calling inherited methods, that absolutely must be executed is executed during exception handling process.
We can certainly argue how much is some code broken, point is it is broken. All above examples will cause memory leak if any of the destructors causes an unhandled exception. And the only way such code can be properly fixed is by fixing broken destructors.
What exactly is handling exceptions?
Handling exceptions is done within try...except block. Any exception that is caught with that block and is not re-raised is handled. On the other hand try...finally blocks are used for cleanup (executing code that absolutely must run even in case of exception) and not for handling exceptions.
For instance if you have some code in BeforeDestruction or destructor doing string to integer conversion that code can raise EConvertError. You can catch that exception with try...except block and handle it there not letting it escape and cause havoc.
destructor TFoo.Destroy;
var
x: integer;
begin
try
x := StrToInt('');
except
on E: EConvertError do writeln(E.ClassName + ' handled');
end;
inherited;
end;
If there is some cleanup code you must perform you can also use try...finally block inside and make sure that any cleanup code executes properly.
destructor TFoo.Destroy;
var
x: integer;
begin
try
try
x := StrToInt('');
finally
writeln('cleanup');
end;
except
on E: EConvertError do writeln(E.ClassName + ' handled');
end;
inherited;
end;
Another way of handling exceptions - is preventing them in the first place. Perfect example is calling Free on your inner fields instead of calling Destroy. That way destructors can handle partially constructed instances and perform proper cleanup. If the FBar is nil FBar.Free will do nothing, but FBar.Destroy would raise an exception.
destructor TFoo.Destroy;
begin
FBar.Free;
inherited;
end;
How not to handle exceptions during destruction process
Don't go around writing try...except blocks in every destructor you have ever written. Not every single line of code can cause an exception, nor absolutely all exceptions everywhere should be eaten up.
Exceptions are exceptional events that may occur in some code under specific circumstances, but that does not mean you cannot recognize code that may cause an exceptions and protect it.
Also, wrapping up all code with try...except block will not keep you safe. You have to deal with exceptions within each destructor.
For instance if FBar destructor can cause an exception then you must handle that exception within TBar destructor. Wrapping it up in exception handler inside TFoo destructor will leak FBar instance because its destructor is flawed and it will not release FBar heap memory.
destructor TFoo.Destroy;
begin
// WRONG AS THIS LEAKS FBar instance
try
FBar.Free;
except
...
end;
inherited;
end;
This is proper handling of exception that may be raised in TBar destructor
destructor TBar.Destroy;
begin
try
// code that can raise an exception
except
...
end;
inherited;
end;
destructor TFoo.Destroy;
begin
FBar.Free;
inherited;
end;
