How is this a memory leak?

Question

FastMM reports this line as the source of a memory leak :

StrClassName := MidStr (curLine, length(START_OF_CLASSNAME)+1, length(curline)+1)

What's up with Copy and MidStr? Is this only a Delphi 2007 compiler bug, or do the later versions also have this problem? Here is the link to the copy of the FastMM report, and an image of how my applications displays those kinds of reports. See, in order to display the nodes in VirtualTreeView I need a new data type. I call it TMemoryLeak. When I parse the report, I give my TMemoryLeak a class name, the callstack, it's size etc. But when the app shuts down, and FastMM kicks in, the copy line from above seems to leak memory. I deallocate the callstack the size, the whole object, but the ClassName field which is a string always leeks memory.

Update (from comment)

Here are the declarations and constructors and deconstructors. As for the lifetime- the objects' deconstructors are called as soon as the objects are used to display the node tree, After that they are obsolete, and are deallocated (I hope).

TMemoryLeak = class(TObject)
    private
      fID              :integer;
      fSize            :integer;
      fTotalSize       :integer;
      fCallStack       :TStringList;
      fClassName       :string;
      fRepeatedInstance:integer;


    public
      property ID               :integer      read fID                write fID;
      property TotalSize        :Integer      read fTotalSize         write fTotalSize;
      property Size             :integer      read fSize              write fSize;
      property CallStack        :TStringList  read fCallStack         write fCallStack;
      property ClassName        :string       read fClassName         write fClassName;
      property RepeatedInstance :integer      read fRepeatedInstance  write fRepeatedInstance;
      class function Equal(xA: TMemoryLeak; xB: TMemoryLeak): Boolean;
      procedure clear;
      constructor create;
      destructor destroy; override;
  end;

  TMemoryLeakList=class(TObjectList)
    private
      fSortType         :TMlSortType;
      fSortDirection    :TMLSortDirection;
      fTotalLeakSize    :integer;
      fClassName        :string;
      fRepeatedInstance :Integer;
      fID               :Integer;
      function  GetItem(Index: Integer): TMemoryLeak;
      procedure SetItem(Index: Integer; const Value: TMemoryLeak);

    public
      property Items[Index: Integer]:TMemoryLeak      read GetItem             write SetItem; default;
      property TotalLeakSize        :integer          read fTotalLeakSize      write fTotalLeakSize;
      property SortType             :TMLSortType      read fSortType           write fSortType;
      property SortDirection        :TMLSortDirection read fSortDirection      write fSortDirection;
      property ClassName            :string           read fClassName          write fClassName;
      property RepeatedInstance     :integer          read fRepeatedInstance   write fRepeatedInstance;
      property ID                   :Integer          read fID                 write fID;

      function Add(AObject: TMemoryLeak): Integer;
      procedure Clear();
      procedure Sort;

      constructor create;
      destructor destroy; override;
  end;


constructor TMemoryLeak.create;
  begin
    inherited;
    fCallStack := TStringList.create;
    fRepeatedInstance:=0;
  end;
  destructor TMemoryLeak.destroy;
  begin
    clear;
  end;
  procedure TMemoryLeak.clear;
  begin
    fCallStack.Clear;
  end;
  class function TMemoryLeak.Equal(xA, xB: TMemoryLeak): Boolean;
  var i:Integer;
  begin
    Result:=False;

    if xA.ClassName = xb.ClassName then
    begin
      if xA.size = xb.size then
      begin
        if xA.CallStack.Count = xB.CallStack.Count then
        begin
          for i := 0 to xa.CallStack.Count - 1 do
          begin
            if CompareStr(xA.CallStack[i], xB.CallStack[i]) <> 0 then
            begin
             break;
            end;
          end;
          if i = xa.CallStack.Count then
            Result:=True;
        end
      end
    end

  end;

  { TMemoryLeakList }

  constructor TMemoryLeakList.create;
  begin
    inherited;
    fSortType         :=stID;
    fSortDirection    :=sdAsc;
    fClassName        :='';
    fRepeatedInstance :=0;
  end;
  destructor TMemoryLeakList.destroy;
  begin
    Clear;
  end;
  procedure TMemoryLeakList.Clear;
  var i : Integer;
  begin
      for i := 0 to Count - 1 do
        Items[i].clear;
  end;

Answer 1

The explanation that makes sense is that you have a memory leak.

I think you have a mis-understanding of how FastMM leak reporting works. You seem to infer from the leak report that Copy, MidStr etc. are responsible for the leak. That's not the case. A leak is reported when memory is allocated but not subsequently deallocated. In the case of functions like Copy and MidStr, their job is to create new strings which naturally involves allocation of memory. The leak is reported because the memory that was created to hold the string buffer was not deallocated. It is not the fault of Copy or MidStr, the allocating function, when the rest of the code fails to deallocate that memory.

Delphi 2007 is a mature product and the memory management for strings is known to be correct. Perhaps you do some manual memory copying that bypasses the reference counting for strings. Are you setting some variables/fields to nil with a call to FillChar? Are you disposing of a record with FreeMem instead of Dispose? The former will not decrement the strings reference count. Something like this is the likely cause of the leak.

---

Looking at extracts of the code you posted, this is a problem:

destructor TMemoryLeakList.destroy;
begin
  Clear;
end;

You have failed to call the inherited destructor. Which means that the members of the list won't be destroyed. Which explains why your string is not destroyed.

In fact you don't need to provide a destructor for the list class. Simply remove it and let the inherited TObjectList destructor do the work. Since OwnsObjectsdefaults to True, any members of the list are destroyed as soon as they are removed from the list, and when the list itself is destroyed.

If your Clear method actually cleared the list, then that would happen. But your Clear isn't a real Clear. A real Clear in a container is expected to remove all members. You should remove your Clear and rely on the inherited version.

In TMemoryLeak, you also fail to call the inherited destructor. And also fail to destroy the string list instance that is owned by that class.

To summarise, I'd write these constructors and destructors like this:

constructor TMemoryLeak.Create;
begin
  inherited;
  fCallStack := TStringList.Create;
end;

destructor TMemoryLeak.Destroy;
begin
  fCallStack.Free;
  inherited;
end;

constructor TMemoryLeakList.Create;
begin
  inherited;//by default OwnsObjects is set to True, list manages member lifetime
  fSortType :=stID;
  fSortDirection :=sdAsc;
end;

And then remove the destructor, and remove the Clear method. The versions inherited from TObjectList suffice.

In the comments you state:

The objects' destructors are called as soon as the objects are used to display the node tree. After that they are obsolete, and are deallocated (I hope).

I'd say there's a good chance that this is not helping. Since you created the object list in OwnsObjects mode, you should not be destroying the members of the list at all. You've asked the list itself to do that. You can't both do it. And the "I hope" comment doesn't fill me with much confidence that this code is correct.

Since we can't see all of your code, I'm far from sure that this is the entirety of the problems with it.

The bottom line is that your code has leaks. Trust in FastMM!