I encountered a memory leak when an exception is raised during an object's
destruction: The object's memory is no not freed.
A real world example is TWriter from Classes.pas. It calles WriteBuffer during
Destroy, which can raise an exception caused by the underlying TStream if, for
example, the stream runs out of disk space.
Below follows a tiny example project to test. Both MemCheck and MemProof report
the memory leak.
Any clues anybody how this can be avoided?
Ralf
{ ---------------------------------------------------------------------------- }
program Destructor_Exception;
{$APPTYPE Console}
{.$DEFINE MemCheck}
uses
{$IFDEF MemCheck}
MemCheck, // http://v.mahon.free.fr/pro/freeware/memcheck
{$ENDIF}
SysUtils,
Dialogs;
type
TExceptionObject1 = class
destructor Destroy; override;
end;
TExceptionObject2 = class(TExceptionObject1)
destructor Destroy; override;
end;
destructor TExceptionObject1.Destroy;
begin
ShowMessage('1 Destructor called');
inherited;
end;
destructor TExceptionObject2.Destroy;
begin
try
{ Simulate an exception which could be caused by a stream writing error. }
raise Exception.Create('2');
finally
inherited;
end;
end;
var
eo: TExceptionObject2;
begin
{$IFDEF MemCheck}
MemChk;
{$ENDIF}
eo := TExceptionObject2.Create;
try
{ The following lines does not free the object.
We are left with a memory leak of 4 bytes. }
eo.Free;
except
{ Console applications don't have an automatic exception handler,
so Except is needed here to free the Exception object created. }
end;
end.
>I encountered a memory leak when an exception is raised during an object's
>destruction: The object's memory is no not freed.
>Any clues anybody how this can be avoided?
Don't let an exception occur in a destructor?<G> Seriously - that
makes sense - raising the exception is going to kick you all the way
back to an handler somewhere - and the code to release the memory is
never going to be executed. Trapping the exception will get you around
that - if you don't reraise the exception. So you could do something
like:
try
inherited;
clean up your own mess
except
on E: Exception do showmessage(E.Message);
end;
Needless to say - this does not completely solve the problem. Whatever
normally would have been cleaned up between the raising of the
exception and your "catching" it is not going to be done.
Good luck.
Kurt
No. There has been tons of literature on the subject of throwing exceptions
during finalization/destruction in a number of object-oriented languages
(e.g. Delphi, C++, Java, C#). All of them basically come to the conclusion
that there's really no solution to this and there should never be exceptions
thrown (and not handled) in a destructor. You will just have to make sure
that no exception leaves your destructor.
sm
destructor TWriter.Destroy;
begin
WriteBuffer;
inherited Destroy;
end;
WriteBuffer can raise an exception. If this really happens, the TWriter object
will not be properly freed. Should we consider this a bug?
The following solution would free the TWriter object, but would also suppress
the exception.
destructor TWriter.Destroy;
begin
try
WriteBuffer;
except
end;
inherited;
end;
I prefer this to your suggestion (below), because calling the inherited
destructor first can free memory which is essentially needed for the clean-up.
Ralf
>destructor TWriter.Destroy;
>begin
> WriteBuffer;
> inherited Destroy;
>end;
>
>WriteBuffer can raise an exception. If this really happens, the TWriter object
>will not be properly freed. Should we consider this a bug?
If that is correct (and I willl take your word for it) then I would
classify that as a bug. However, I might change that classification to
"operator error" or "works as designed" if it is unreasonable to
expect an exception in calling WriteBuffer.
>The following solution would free the TWriter object, but would also suppress
>the exception.
>
>destructor TWriter.Destroy;
>begin
> try
> WriteBuffer;
> except
> end;
> inherited;
>end;
Yes, it suppresses the exception and the object will be free'd. Now
what? Presumably some portion of the output did not get written. No
error indication has been given. This is Not Good [tm].
>I prefer this to your suggestion (below), because calling the inherited
>destructor first can free memory which is essentially needed for the clean-up.
Changing the order of your cleanup (calling WriteBuffer) and calling
inherited is fine - it depends on what needs to be cleaned up locally.
You might even have local stuff cleaned up both before and after the
inherted call. But I really think some error indication needs to be
given of the failure. So I would change your solution to:
destructor TWriter.Destroy;
begin
try
WriteBuffer;
except
on E: Exception do ShowMessage(E.Message);
end;
inherited;
end;
And possibly give some additional indication as to what should be done
next.
Good luck.
Kurt
I would say so, since WriteBuffer is specifically written to
raise an exception when the return value of Write indicates
that not all data could be written.
>
> The following solution would free the TWriter object, but would also suppress
> the exception.
>
> destructor TWriter.Destroy;
> begin
> try
> WriteBuffer;
> except
> end;
> inherited;
> end;
Try this instead (not tested):
destructor TWriter.Destroy;
begin
try
try
WriteBuffer;
finally
inherited Destroy;
end;
except
FreeInstance;
raise;
end;
end;
That should preserve your exception while freeing the object.
Karl
Forgot to mention:
You should not subclass TWriter when using this approach.
Karl
Thanks to you all!
Ralf
>Try this instead (not tested):
>
>destructor TWriter.Destroy;
>begin
> try
> try
> WriteBuffer;
> finally
> inherited Destroy;
> end;
> except
> FreeInstance;
> raise;
> end;
>end;
>
>That should preserve your exception while freeing the object.
>You should not subclass TWriter when using this approach.