12
votes

According to FastMM4, the Delphi program I'm working on at the moment is leaking a lot strings. AnsiStrings to be precise:

enter image description here

The application (http://sourceforge.net/projects/orwelldevcpp/) used to leak a lot more other data types, but FastMM4 could report where the instance was created, so I managed to fix that. The strange thing is that FastMM4 doesn't report locations of these leaks at all.

Edit: it seems it does after all, see answers for the fix. Anyways, the question still stands: how in the world am I leaking these things?

So, ehm, unfortunately, I've got no idea what to look for. I mean, if these things go out of scope, they should be automatically freed right (even though they're on the heap)?

I did manage to track a few leaks down by random commenting and seeing what would happen to the counts. Here's an example:

// simply passing it a constant creates a leak...
MainForm.UpdateSplash('Creating extra dialogs...');

procedure TMainForm.UpdateSplash(const text : AnsiString);
begin
  if not devData.NoSplashScreen then // even if this branch is NOT taken
    SplashForm.Statusbar.SimpleText := 'blablabla' + text;
end;

// And even if the function call itself is placed within a NOT taken branch!

Here's another example of a leak:

// Passing this constants produces leaks...
procedure TCodeInsList.AddItemByValues(const a, b, c: AnsiString;...);
var
  assembleditem : PCodeIns;
begin
   new(assembleditem);
   assembleditem^.Caption:=a;
   assembleditem^.Line:=b;
   assembleditem^.Desc:=c;
   ...
   fList.Add(assembleditem);
end;

// ... even when calling this on WM_DESTROY!
destructor TCodeInsList.Destroy;
var
  I: integer;
begin
  for I := 0 to fList.Count - 1 do
    Dispose(fList[I]);
  fList.Free;
  inherited Destroy;
end;

// produces leaks!?

There are quite a bunch of string leak questions here, but none really clarify what patterns one should look for. Google doesn't provide either.

Edit: so, I have to look for passed constants. But why?

So ehm, any ideas?

5
I can't get the sourceforge project to load at the moment. Is there any possibility that the main form is not being destroyed properly and hence leaving dangling strings? Would that do it?Richard A
delphi version? If you can, test with aqtime it will tell you exactly where the leak is.Warren P
@RichardA: As you can see in source\devcpp.dpr, the splashform is freed using 'Free'. Will try adding caFree to the OnClose event.<crlf> @ Warren: I highly doubt aqtime will be able to tell me more than FastMM4, gpProfiler and MemCheck. And I need to upgrade to XE too for aqtime (using D7 now). My university does have a D2009 license floating around somewhere (not for EE folks like me though), but aqtime doesn't even support that it seems.Orwell
aqtime runs fine with old delphi versions but yes, you'd have to buy it. AQTime runs with Delphi 7 through XE2.Warren P

5 Answers

14
votes

You don't need to be explicitly allocating strings. Apart from mangling with reference counts, string fields of objects or records may also leak. For instance,

type
  PRecord = ^TRecord;
  TRecord = record
    S: string;
  end;

procedure TForm1.Button4Click(Sender: TObject);
var
  r: PRecord;
begin
  GetMem(r, SizeOf(r^));
  Initialize(r^);
  r.S := ' ';
  FreeMem(r);

In the above example, since the memory of the record itself is freed, FastMM will report only the leaked string.


In any case, FastMM not showing a stack trace in the dialog does not mean that it lacks that information. Be sure to have FullDebugMode, LogMemoryLeakDetailToFile and LogErrorsToFile are defined in 'FastMM4Options.inc'. Then look for a '[ExecutableName]_MemoryManager_EventLog.txt' file in the directory of the executable.

For the above example, FastMM produces the following file:

--------------------------------2012/5/27 4:34:46--------------------------------
A memory block has been leaked. The size is: 12

Stack trace of when this block was allocated (return addresses):
40305E 
404B5D 
404AF0 
45C47B 
43D726 
42B0C3 
42B1C1 
43D21E 
76C4702C [GetWindowLongW]
77AE3CC3 [Unknown function at RtlImageNtHeader]

The block is currently used for an object of class: Unknown

The allocation number is: 484

Current memory dump of 256 bytes starting at pointer address 7EF8DEF8:
01 00 00 ...
...

Now you can run the application, pause it and then search for the addresses. For the above log and the test application, the addresses resolves to:

Stack trace of when this block was allocated (return addresses):
40305E    -> _GetMem
404B5D    -> _NewAnsiString
404AF0    -> _LStrAsg
45C47B    -> TForm1.Button4Click (on FreeMem line)
43D726    -> TControl.Click
... 


edit: Instead of manually looking up addresses, generate a detailed map file through linker options and FastMM will do it (thanks to Mason's comment).


Your edit on the question reflects a quite similar leak like the one in the above example. If the 'fList' is a regular TList, it just holds pointers and have no knowledge of what those pointers points to. Hence when you dispose the pointer, just the memory allocated for the pointer itself is freed, not the fields of the record. So the leaks have nothing to do constants passed to functions but is like the pattern below:

var
  assembleditem: PCodeIns;
  p: Pointer;
begin
  new(assembleditem);
  assembleditem^.Caption:='a';
  ..    
  p := assembleditem;
  Dispose(p);

For the record to be disposed, the code should typecast the pointer to its type:

Dispose(PCodeIns(p));

So your 'TCodeInsList.Destroy' should be:

destructor TCodeInsList.Destroy;
var
  I: integer;
begin
  for I := 0 to fList.Count - 1 do
    Dispose(PCodeIns(fList[I]));
  fList.Free;
  inherited Destroy;
end;


In the end, the pattern you're looking for seems to be looking for places where the code intents to free records (less likely objects) having string fields. Looking for Dispose, a little less likely FreeMem, even less likely FreeInstance to free memory of objects/records that FastMM shows as the allocated memory is leaked could help.

4
votes

You're right that strings should be cleaned up automatically. I've seen a few ways to screw that up, though.

The first is if you're doing things with the string data structure directly that can break the reference count. This is the most likely, with the number of strings you're leaking.

The other one is calling Halt and leaving string references on the stack. But you're not going to leave 40,000 string references on the stack, so I'd look for code that gets passed a string and then fiddles with its reference count.

1
votes

With short words, Delphi build-in string types are reference counted. Memory allocation and dispose methods do not take care of updating reference counting, so that the compiler does not know, that strings in your records can be freed actually.

It's discouraged to define a record with reference counted string types. I had same confusion before. If you take a look into the source of Delphi library. You will find many records have PChar not string.

Some discuss about records

1
votes

The most common way to leak strings is to have a record that contains a string and a pointer to that record. If you simply do a Dispose() of that pointer, the compiler will just free the pointer, not all the stuff in the subjacent record. Always make sure that your dispose code tells the compiler what you are disposing of.

For example, suppose that in a TTreeView I put PMyRecord = ^MyRecord in the Node.Data. If, at the end you loop though all Nodes and simply do Dispose(Node.Data) then any strings in MyRecord will not be handled properly.

But if you dispose of your pointer by telling the compiler explicitely what the pointer's underlying type is, by calling Dispose(PMyRecord(Node.Data)), then there will be no memory leak.

0
votes

I discovered that a string (as a field in record) can be leaked even without memory allocation/pointer operations.

It sounds crazy, but it's true, at least in XE3. Here is the example:

TMyRecord = record
x,
y: integer;
s: ansistring;
end;

function GetMyRec: TMyRecord;
begin
....
end;

....
procedure DoSomething;
var
  rec: TMyRecord;
begin
  ...
  rec := GetMyRec; //First call - everything is OK
  ...
  rec := GetMyRec; //Repeated call > Memory Leak of 
                   //Ansistring !!!!
  //To avoid the leak do the following BEFORE a 
  //repeated call: rec.s := unassigned;
end;