What is the source of this memory leak (when using TCollection)

Question

I have some code which i did not write, but there is a memory leak. The real strangeness is the memory only leaks if i zero a structure before returning it.

Reproducible minimum code

The leak is reproducible in Delphi 5 and Delphi 7.

First we have a structure:

type
   TLocalFile = packed record
      FileName: AnsiString;
   end;

This structure is the private member of a CollectionItem object:

TEntry = class(TCollectionItem)
private
   FLocalFile: TLocalFile;
end;

Then we have the owning collection, which has a function that can return a populated structure:

TEntries = class(TCollection)
protected
    function GetLocalFile: TLocalFile;
public
    procedure DoStuff;
end;

With the weirdness located in the GetLocalFile function:

function TEntries.GetLocalFile: TLocalFile;
var
    s: AnsiString;
begin
    //Only leaks if i initialize the returned structure
//  FillChar(Result, SizeOf(Result), 0);
    ZeroMemory(@Result, SizeOf(Result));

    s := 'Testing Leak';
    Result.Filename := s; //'Testing leak';  only leaks if i set the string through a variable
end;

In reality this function is passed a stream, and returns a populated structure, but that's not important now.

Next we have a method of the collection that will populate all it's entries's LocalFile structures:

procedure TEntries.DoStuff;
var
    x: Integer;
begin
    for X := 0 to Count-1 do
    begin
       (Items[X] as TEntry).FLocalFile := GetLocalFile;
    end;
end;

Finally, we construction a collection, add 10 items to it, have them DoStuff, then free the list:

procedure TForm1.Button1Click(Sender: TObject);
var
    list: TEntries;
    i: Integer;
    entry: TCollectionItem;
begin
    list := TEntries.Create(TEntry);
    try
        for i := 1 to 10 do
            entry := list.Add;

        list.DoStuff;
    finally
        list.Free;
    end;
end;

We created 10 items, we leak 9 AnsiStrings.

The horrifying confusing things

There are ways in which this code doesn't leak. It only leaks when using an intermediate string stack variable

Change:

function TEntries.GetLocalFile: TLocalFile;
var
   s: AnsiString;
begin
   s := 'Testing Leak';
   Result.Filename := s; //'Testing leak';  only leaks if i set the string through a variable
end;

to

function TEntries.GetLocalFile: TLocalFile;
begin
   Result.Filename := 'Testing leak'; //doesn't leak
end;

and it doesn't leak.

The other method is not to initialize the structure before returning it:

Remove the call to FillChar or ZeroMemory, and it doesn't leak:

function TEntries.GetLocalFile: TLocalFile;
var
    s: AnsiString;
begin
    //Only leaks if i initialize the returned structure
//  FillChar(Result, SizeOf(Result), 0);
//  ZeroMemory(@Result, SizeOf(Result));

    s := 'Testing Leak';
    Result.Filename := s; //'Testing leak';  only leaks if i set the string through a variable
end;

These are strange resolutions. Whether i use an intermediate stack variable, or not, whether i Zero the structure or not, should not have any effect on memory cleanup.

I doubt this is a bug in the compiler. Which mean that i (meaning the person who wrote this) is doing something fundamentally wrong. i assume it has something to do with TCollectionItemClass. But i cannot for the life of me figure out what.

Full code

unit FMain;

interface

uses
  Windows, Messages, SysUtils, Variants, Classes, Graphics, Controls, Forms,
  Dialogs, StdCtrls;

type
    TLocalFile = packed record
        FileName: AnsiString;
    end;

    TEntry = class(TCollectionItem)
   private
        FLocalFile: TLocalFile;
    end;

    TEntries = class(TCollection)
    protected
        function GetLocalFile: TLocalFile;
    public
        procedure DoStuff;
    end;

  TForm1 = class(TForm)
    Button1: TButton;
    procedure Button1Click(Sender: TObject);
  private
  public
  end;


var
  Form1: TForm1;

implementation

{$R *.dfm}

uses
    contnrs;

procedure TForm1.Button1Click(Sender: TObject);
var
    list: TEntries;
    i: Integer;
    entry: TCollectionItem;
begin
    list := TEntries.Create(TEntry);
    try
        for i := 1 to 10 do
        begin
            entry := list.Add;
        end;

        list.DoStuff;
    finally
        list.Free;
    end;
end;

{ TEntries }

procedure TEntries.DoStuff;
var
    x: Integer;
    entry: TEntry;
begin
    for X := 0 to Count-1 do
    begin
        entry := Items[X] as TEntry;
        entry.FLocalFile := GetLocalFile;
   end;
end;

function TEntries.GetLocalFile: TLocalFile;
var
    s: AnsiString;
begin
    //Only leaks if i initialize the returned structure
//  FillChar(Result, SizeOf(Result), 0);
    ZeroMemory(@Result, SizeOf(Result));

    s := 'Testing Leak';
    Result.Filename := s; //'Testing leak';  only leaks if i set the string through a variable
end;

end.

Oh, and don't forget to add FastMM4 to your project (if you don't already have it built-in), so you can detect the leaks.

Answer 1

AnsiString (and its Unicode counterpart) is reference-counted by the compiler. You can't simply zero memory containing a reference to it; you need to assign '' to it so that the compiler will generate code to decrement the refcount and release the memory correctly.

You'll have similar problems trying to block-clear data structures containing references to dynamic arrays, Interfaces, or (some) variants.

If you're not using a Delphi version recent enough to support the Default compiler magic expression, (I believe it was introduced in D2009,) the best way to clear out a record safely would be to call Finalize first, and then zero the memory as you're doing.