View Issue Details

IDProjectCategoryView StatusLast Update
0033650FPCpublic2018-08-02 21:17
ReporterPaulo SérgioAssigned To 
PrioritynormalSeverityminorReproducibilityalways
Status newResolutionopen 
PlatformDesktopOSWindowsOS Version10
Product Version3.0.4Product Build 
Target VersionFixed in Version 
Summary0033650: Memory leak when raising exception in "try finally" block.
DescriptionMemory leak when raising exception in "try finally" block.
Steps To ReproduceSteps:
- Create new project.
- Add a Button in form.
- Program this:
  procedure TForm1.Button1Click(Sender: TObject);
  begin
    try
      StrToInt('Not an number');
    finally
      StrToInt('Not an number');
    end;
  end;
- Go to Project Options > Compiler Options > Debugging > Other debugging info.
- Check the box "Use Heaptrc unit (check for mem-leaks) (-gh)".
- Run the program.
- Click in Button.
- Close the form.

- Will be shown the box dialog with unfree memory blocks.
Additional InformationTested in Lazarus versions: 1.8.0, 1.8.2 and 1.9.0
TagsNo tags attached.
Fixed in Revision
FPCOldBugId
FPCTarget
Attached Files

Activities

Paulo Sérgio

2018-04-25 23:23

reporter  

Bug Memory Leak.zip (2,205 bytes)

Martin Friebe

2018-06-08 14:05

manager   ~0108753

I would say this is expected.

StrToInt raises an exception, so an exception object is created.
There is no "except" block to handle this. So the exception object is never freed.

The following works without leak
  try
    StrToInt('Not an number');
  except
  end;

In any case, this is (if at all) an issue in fpc.
So I let the fpc team have the final word.

If you have any questions on how to use the above, please use the forum.

Thaddy de Koning

2018-06-08 17:43

reporter   ~0108759

Last edited: 2018-06-08 17:45

View 3 revisions

That's not a good idea, Martin. He should use StrToIntDef or TryStrToInt.
It is indeed expected as Martin explained: you protect something and then you repeat it in the finally with exactly the same - wrong - code, but without protection? Belongs on the forum.

Joost van der Sluis

2018-06-18 23:11

manager   ~0108958

We've discussed this in length on core. And finally decided that we should follow Delphi in this.
This means that an uncatched exception leaks. The exception in the finally-section is not catched so the leak is expected.
It is very difficult to change this behavior and can have all kinds of unexpected side-effects. (Freeing the Exception object in case of an unhandled exception may lead to indefinite loops if the memory is corrupted, dixit Karoly Balogh)

However, we (Yuriy Sydorov) did found a related issue: An exception in a finally section is not catched when this statement itself is in another try..except block. I keep this bug report open.

Here is a testcase (code from Yuriy):

=== code begin ===
{$APPTYPE CONSOLE}

{$ifdef FPC} {$mode objfpc} {$endif}

uses
  SysUtils;

var
  ECounter: integer;

type
  ETest = class(Exception)
    destructor Destroy; override;
  end;

destructor ETest.Destroy;
begin
  Inc(ECounter);
  Writeln('ETest.Destroy: ' + Message);
  inherited;
end;

begin
  try
    try
      raise ETest.Create('aaa');
    finally
      raise ETest.Create('bbb');
    end;
  except
    writeln('Exception handled.');
  end;
  writeln('After except');
  if ECounter <> 2 then begin
    writeln('FAILED: Exception has leaked.');
    Halt(1);
  end;
  writeln('OK.');
end.

=== code end ===

Paulo Sérgio

2018-06-25 22:17

reporter   ~0109050

Thank you all.
My code is just a very simple example.
I did it just to demonstrate how to generate the leak.

Joost van der Sluis, then the leak is expected, what would be the solution to the code example below?

try
  // Code that can raise an exception.
  Code...
finally
  // Code that needs to be executed.
  Code...
end;

The code inside the "finally ... end" must be executed. So I'm using the "try ... finally ... end" block.
But this code may raise an error.

Joost van der Sluis

2018-08-02 15:25

manager   ~0109841

There are several ways to do this. One way is the example of Yuri, but that one currently also leaks...

Best is to do this (but might not be that easy):

try
  // Code that can raise an exception.
  Code...
Except
  // Handle exception
end;
try
  // Code that needs to be executed.
  Code...
Except
  // Handle exception
end;



Or:
try
  // Code that can raise an exception.
  Code...
finally
  // Code that needs to be executed.
  try
    Code...
  except
    // Handle exception
  end
end;

Thaddy de Koning

2018-08-02 21:17

reporter   ~0109844

try
  // Code that can raise an exception.
  Code...
finally
  // Code that needs to be executed.
  try
    Code...
  except
    // Handle exception
  end
end;

That should be the documented solution.

Issue History

Date Modified Username Field Change
2018-04-25 23:23 Paulo Sérgio New Issue
2018-04-25 23:23 Paulo Sérgio Status new => assigned
2018-04-25 23:23 Paulo Sérgio Assigned To => Martin Friebe
2018-04-25 23:23 Paulo Sérgio File Added: Bug Memory Leak.zip
2018-06-08 14:05 Martin Friebe Note Added: 0108753
2018-06-08 14:05 Martin Friebe LazTarget => -
2018-06-08 14:05 Martin Friebe Assigned To Martin Friebe =>
2018-06-08 14:05 Martin Friebe Status assigned => new
2018-06-08 14:05 Martin Friebe Category Debugger =>
2018-06-08 14:05 Martin Friebe Product Version 1.0.9 (SVN) =>
2018-06-08 14:06 Martin Friebe Project Lazarus => FPC
2018-06-08 14:06 Martin Friebe Product Version => 3.0.4
2018-06-08 17:43 Thaddy de Koning Note Added: 0108759
2018-06-08 17:44 Thaddy de Koning Note Edited: 0108759 View Revisions
2018-06-08 17:45 Thaddy de Koning Note Edited: 0108759 View Revisions
2018-06-18 23:11 Joost van der Sluis Note Added: 0108958
2018-06-25 22:17 Paulo Sérgio Note Added: 0109050
2018-08-02 15:25 Joost van der Sluis Note Added: 0109841
2018-08-02 21:17 Thaddy de Koning Note Added: 0109844