Am I creating a memory leak here?
Asked Answered
S

4

7

A very simple question:

type

TMyRecord = Record
  Int: Integer;
  Str: String;
end;

PMyRecord = ^TMyRecord;

var
  Data: PMyRecord;
begin
  New(Data);
  Data.Int := 42;
  Data.Str := 'Test';
  Dispose(Data);
end;

My question is, am I creating a memory leak here (with the String)? Should I call Data.Str := ''; before calling Dispose?

Thanks!

Shiri answered 26/11, 2010 at 14:1 Comment(0)
C
12

No, Dispose properly frees strings and dynamic arrays in records, including nested ones. GetMem/FreeMem(Data) would create a memory leak, indeed.

Celestyna answered 26/11, 2010 at 14:3 Comment(3)
Add variants, reference counted interfaces and maybe even anonymous functions to the list, I think everything that is reference counted is released on Dispose().Hadrian
GetMem/FreeMem would require to call Finalize(). Check Finalize in the help for an explanation on what it does when Dispose() can't be used.Kanpur
@ldsanson: New = GetMem+Initialize and Dispose = Finalize+FreeMem - check System.pas. Those are just wrappers to both functions.Topmost
F
10

It's a memory leak if an exception is raised in between your allocate/deallocate pairs. It is normal to protect them as such:

New(Data);
Try
  Data.Int := 42;
  Data.Str := 'Test';
Finally
  Dispose(Data);
End;
Flophouse answered 26/11, 2010 at 15:20 Comment(4)
You should better allocate your records either in the stack, either in a dynamic array. Both will initialize the content of every record item. The try..finally is mandatory for on the heap allocation, like a TObject.Create needs a corresponding try..finally Free. Such try...finally will be created automatically by the compiler if your record is allocated via a dynamic array or on the stack.Topmost
@ArnaudBouchez Allocate in an array if you want an array. Not appropriate if you want a single record. Allocate on the heap with New rather than the stack if the lifetime of the record must outlive the scope that creates the record.Flophouse
@DavidHeffernan I put +1 to your answer, which is the correct one. I just wanted to point out some alternative. Using New/Dispose to allocate records on the heap is weird, since you have class at hand just for this purpose. Pointers on records are less clean than a class instance - which features the whole OOP paradigm, whereas record do not.Topmost
@Arnaud Maybe. If the structure is always heap allocated then a class is sensible. But what about a structure that you want to be able to allocate either on the stack or the heap?Flophouse
G
0

No you are not, String cleans its own memory itself when it is deleted.

Godbeare answered 26/11, 2010 at 14:2 Comment(1)
Actually: strings free themselves when the encompassing structure is finalized.Phototelegraphy
B
0

If you want a memory leak, afaik you have to use TP objects :-) They are afaik the only structured types in Delphi that are not initialized/finalized

Becket answered 26/11, 2010 at 14:10 Comment(3)
TP objects are initialised. They are treated the same way as records. We actually used them extensively until Delphi added support for records with methods.Flophouse
record or object don't create memory leaks if you instance them using dynamic arrays or on the stack, which is very common. Only with heap allocation is needed New() and Dispose(). It's not much difficult to use New(aRecordPointer) than anObject.Create...Topmost
groups.google.com/group/comp.lang.pascal.delphi.misc/msg/… Admitted, it goes for TP objects that have a VMT. So usage as record is ok, usage as object notBecket

© 2022 - 2024 — McMap. All rights reserved.