View Issue Details

IDProjectCategoryView StatusLast Update
0034216FPCFCLpublic2018-09-03 17:18
ReporterwpAssigned ToMichael Van Canneyt 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product VersionProduct Build 
Target Version3.2.0Fixed in Version3.3.1 
Summary0034216: Memory leak in chmreader
DescriptionRecompiling lhelp with activated heaptrc reveals a memory leak in unit chmreader:

Procedure "TChmReader.ReadCommonData" contains two nested procedures "ReadFromWindows" and "ReadContextIDs". In both of them, two MemoryStreams (fWindows and fStrings in ReadFromWindows, and fIVB and fStrings in ReadContextIDs) are created by the function "TITSFReader.GetObject", but never destroyed again (at least in the regular case that the procedures are exited at their end.)

Adding the corresponding fWindows.Free, fStrings.Free, and fIVB.Free removes the memory leak.
Additional InformationPatch provided.
TagsNo tags attached.
Fixed in Revision39706
FPCOldBugId
FPCTarget
Attached Files
  • chmreader.pas.patch (582 bytes)
    Index: packages/chm/src/chmreader.pas
    ===================================================================
    --- packages/chm/src/chmreader.pas	(revision 39704)
    +++ packages/chm/src/chmreader.pas	(working copy)
    @@ -416,6 +416,8 @@
            end;
          end;
          ReadWindows(FWindows);
    +     fWindows.Free;
    +     fStrings.Free;
        end;
        procedure ReadContextIds;
        var
    @@ -441,6 +443,8 @@
            Str := '/'+ ReadString(fStrings, Offset, True);
            fContextList.AddContext(Value, Str);
          end;
    +     fIVB.Free;
    +     fStrings.Free;
        end;
     begin
        ReadFromSystem;
    
    chmreader.pas.patch (582 bytes)

Activities

wp

2018-09-03 00:54

reporter  

chmreader.pas.patch (582 bytes)
Index: packages/chm/src/chmreader.pas
===================================================================
--- packages/chm/src/chmreader.pas	(revision 39704)
+++ packages/chm/src/chmreader.pas	(working copy)
@@ -416,6 +416,8 @@
        end;
      end;
      ReadWindows(FWindows);
+     fWindows.Free;
+     fStrings.Free;
    end;
    procedure ReadContextIds;
    var
@@ -441,6 +443,8 @@
        Str := '/'+ ReadString(fStrings, Offset, True);
        fContextList.AddContext(Value, Str);
      end;
+     fIVB.Free;
+     fStrings.Free;
    end;
 begin
    ReadFromSystem;
chmreader.pas.patch (582 bytes)

Michael Van Canneyt

2018-09-03 09:51

administrator   ~0110475

Fixed using patch.

I had a quick look. This code is horrible regarding errors.
In case of an exception, no memory is freed at all, there are no try-finally blocks.
A thorough review is in order.

wp

2018-09-03 17:18

reporter   ~0110476

Thanks

Issue History

Date Modified Username Field Change
2018-09-03 00:53 wp New Issue
2018-09-03 00:54 wp File Added: chmreader.pas.patch
2018-09-03 09:51 Michael Van Canneyt Fixed in Revision => 39706
2018-09-03 09:51 Michael Van Canneyt Note Added: 0110475
2018-09-03 09:51 Michael Van Canneyt Status new => resolved
2018-09-03 09:51 Michael Van Canneyt Fixed in Version => 3.3.1
2018-09-03 09:51 Michael Van Canneyt Resolution open => fixed
2018-09-03 09:51 Michael Van Canneyt Assigned To => Michael Van Canneyt
2018-09-03 09:51 Michael Van Canneyt Target Version => 3.2.0
2018-09-03 17:18 wp Note Added: 0110476
2018-09-03 17:18 wp Status resolved => closed