dev.emsdn.com

Join About
Home SITEMAP Most Recent

Delphi

Home »» Delphi [Programming]
Thread Profile: Memory Leak


Memory Leak

Does the following code have a memory leak ?
type
rRecord = record
i : integer;
str : string;
end;
pRecord = ^rRecord;
procedure TForm1.testForLeak;
var
i : integer;
s : string;
pnt : pRecord;
list : Tlist;
begin
list := Tlist.create;
for i := 0 to 1000000 do
begin
new(pnt);
pnt.l1 := 1;
pnt.l2 := 'a';
list.Add(pnt);
end;
for i := 0 to list.Count -1 do
begin
dispose(list.Items[i]);
end;
list.Clear;
list.free;
end;
end;
Delphi-Talk mailing list -Delphi-Talk (AT) elists (DOT) org


total 6 Comments Similar Thread
  • 6Answer
  • Total

at [2008-5-3 22:21:16]


Hello Frans,

FWDoes the following code have a memory leak ?

No, because for the strings Delphi is taking care of, and you dispose
all items before destroying the list. ps: the List.Clear; is not needed.

Rgds, Wilfried
http://www.mestdagh.biz

Delphi-Talk mailing list -Delphi-Talk (AT) elists (DOT) org


  • 1No.

at [2008-5-3 22:22:20]


Frans Whyburd wrote:
Does the following code have a memory leak ?

type
rRecord = record
i : integer;
str : string;
end;

pRecord = ^rRecord;

procedure TForm1.testForLeak;
var
i : integer;
s : string;
pnt : pRecord;
list : Tlist;

begin
list := Tlist.create;
for i := 0 to 1000000 do
begin
new(pnt);
pnt.l1 := 1;
pnt.l2 := 'a';

Error: Undeclared identifier: 'l1'
Error: Undeclared identifier: 'l2'

*Always* copy and paste your *real* code.

list.Add(pnt);
end;

for i := 0 to list.Count -1 do
begin
dispose(list.Items[i]);
end;

Yes, this code has a memory leak. The Dispose procedure is a
compiler-magic routine. It knows how much memory to free, and how to
free it, based on the type of the pointer you pass to it. Likewise with
the New procedure.

You pass a PRecord to New. It allocates S(TRecord) bytes, and then
it initializes the str field of the record to ensure it holds a valid
string value. (If you use GetMem to allocate the record, the str field
remain uninitialized, and using it will probably result in an access
violation. That's why we have the Initialize procedure.)

The pointer you pass to Dispose is just of type Pointer, not PRecord.
That's because Pointer is the declared type of the TList.Items property.
Therefore, Dispose doesn't know that there is a str field, so it doesn't
do anything about freeing the string's memory. So you're leaking at
least 10 bytes per record (the number of bytes in the string data), and
probably more, based on the allocation granularity of the memory manager.

To fix the problem, do this:

for i := 0 to Pred(list.Count) do begin
pnt := list[i];
Dispose(pnt);
end;

It isn't necessary to call Clear before you free the TList; the list's
destructor does that for you already.

Actually, in the particular code you showed, there *isn't* a memory
leak. That's because the 'a' string literal is stored as a constant in
your program's data segment -- it doesn't get allocated from the heap.
Each instance of the str field holds a reference to the same string.
Even if you called Dispose properly, the memory for those strings would
never get freed because the memory manager never allocated them.

In general, though, I assume those strings won't all come from
constants, and so you will eventually leak memory.


  • 2No.

at [2008-5-3 22:23:19]


Hello Rob,

RKThe pointer you pass to Dispose is just of type Pointer, not PRecord.
RKThat's because Pointer is the declared type of the TList.Items property.
RKTherefore, Dispose doesn't know that there is a str field, so it doesn't
RKdo anything about freeing the string's memory.

Thanx i did not knew that. Is this also the case (like in the example
from Frans) if the string (in this case the record with the string) go
out of scope ?

Rgds, Wilfried
http://www.mestdagh.biz

Delphi-Talk mailing list -Delphi-Talk (AT) elists (DOT) org


  • 3No.

at [2008-5-3 22:24:24]


Wilfried Mestdagh wrote:
Hello Rob,

RKThe pointer you pass to Dispose is just of type Pointer, not PRecord.
RKThat's because Pointer is the declared type of the TList.Items property.
RKTherefore, Dispose doesn't know that there is a str field, so it doesn't
RKdo anything about freeing the string's memory.

Thanx i did not knew that. Is this also the case (like in the example
from Frans) if the string (in this case the record with the string) go
out of scope ?

I don't understand your question. Could you rephrase it, or provide some
code?


  • 4No.

at [2008-5-3 22:25:23]


Interesting question and interesting answer from Rob.
The followng demonstrates a quick and dirty technique to that some
might find useful when testing this stuff. I use D7.

Strangely, Rob's answer doesn't appear to be quite correct. am I
(again) doing somethin wrong:-(
-malcolm

procedure TForm1.Button1Click(Sender: T);
var s:string;
begin
s:=Format('Start AllocMemCount: %9d',[AllocMemCount]);
DebugString(PChar(s));
s:=Format('Start AllocMemSize: %10d',[AllocMemSize]);
DebugString(PChar(s));

TestForLeak;

s:=Format('End AllocMemCount: %9d',[AllocMemCount]);
DebugString(PChar(s));
s:=Format('End AllocMemSize: %10d',[AllocMemSize]);
DebugString(PChar(s));
end;

procedure TForm1.TestForLeak;
type
rRecord = record
i : integer;
str : string;
end;
pRecord = ^rRecord;
var
i : integer;
pnt : pRecord;
list : Tlist;
begin
list := Tlist.create;
for i := 0 to 10 do
begin
new(pnt);
pnt.i:= 1;
pnt.str:=IntToStr(random(10));
//pnt.str:='a';
list.Add(pnt);
end;

for i := 0 to list.Count -1 do
begin
pnt:=list.Items[i];
dispose(pnt);
//dispose(list.Items[i]);
end;
list.free;
end;

(*
View/Debug Windows/Event Log shows following

With call to TestForLeak commented out:
DS: Start AllocMemCount: 286
DS: Start AllocMemSize: 8900
DS: End AllocMemCount: 287
DS: End AllocMemSize: 8900

TestForLeak uses
constant 'a' with dispose(list.Items[i]);
DS: Start AllocMemCount: 286
DS: Start AllocMemSize: 8900
DS: End AllocMemCount: 298
DS: End AllocMemSize: 9032

TestForLeak uses
random string with dispose(list.Items[i]);
DS: Start AllocMemCount: 286
DS: Start AllocMemSize: 8900
DS: End AllocMemCount: 298
DS: End AllocMemSize: 9032

TestForLeak uses
random string with dispose(pnt);
DS: Start AllocMemCount: 286
DS: Start AllocMemSize: 8900
DS: End AllocMemCount: 287
DS: End AllocMemSize: 8900

*)

Delphi-Talk mailing list -Delphi-Talk (AT) elists (DOT) org


  • 5No.

at [2008-5-3 22:26:28]


Malcolm Clark wrote:
Strangely, Rob's answer doesn't appear to be quite correct. am I
(again) doing somethin wrong:-(

No, I think everything's right. What I said about string literals is
only true _sometimes_, and I'm not sure just what the rules are.

With call to TestForLeak commented out:
DS: Start AllocMemCount: 286
DS: Start AllocMemSize: 8900
DS: End AllocMemCount: 287
DS: End AllocMemSize: 8900

K. The difference in AllocMemCount makes sense since the first time you
check that value, the string result from the Format function hasn't been
allocated yet. I have no idea why AllocMemSize doesn't change, though.

TestForLeak uses
constant 'a' with dispose(list.Items[i]);
DS: Start AllocMemCount: 286
DS: Start AllocMemSize: 8900
DS: End AllocMemCount: 298
DS: End AllocMemSize: 9032

Yep. Definitely a memory leak.

TestForLeak uses
random string with dispose(list.Items[i]);
DS: Start AllocMemCount: 286
DS: Start AllocMemSize: 8900
DS: End AllocMemCount: 298
DS: End AllocMemSize: 9032

Same memory leak. And that's weird since we should expect this leak to
be larger than the previous one. (Rather, the previous one should be
smaller than this one.) Sometimes, when assigning a string literal to a
string variable, Delphi will simply copy the underlying pointer and be
done with it. times, Delphi will make a new copy of the string. I
don't remember how it chooses. As I recall, Borland knows there are
cases (such as this one) when the whole string doesn't need to be
copied, but there are other cases when the copy is necessary. It might
have something to do with global variables or multiple threads.

TestForLeak uses
random string with dispose(pnt);
DS: Start AllocMemCount: 286
DS: Start AllocMemSize: 8900
DS: End AllocMemCount: 287
DS: End AllocMemSize: 8900

And the leak has been fixed.


  • 6No.