Help with dynamic record memory leak

[Gary 18]

Delphi 10.4.1

Windows 10

Why does this leak memory? FastMM4 reports

 

A memory block has been leaked. The size is: 36

This block was allocated by thread 0x948, and the stack trace (return addresses) at the time was:
4070A2 [System.pas][System][@GetMem$qqri][4843]
40A0F7 [System.pas][System][@NewUnicodeString$qqri][25659]
40A584 [System.pas][System][@UStrAsg$qqrr20System.UnicodeStringx20System.UnicodeString][26643]
615C6A [Unit1.pas][Unit1][TForm1.GetReminder][65]
616166 [Unit1][Generics.Collections.%TList__1$19Unit1.TReminderItem%.Create]
615A34 [Unit1.pas][Unit1][TForm1.Button1Click][46]
770168CF [GetWindowLongW]
720A55DC [Unknown function at SetWindowSubclass]
4086D3 [System.pas][System][@IsClass$qqrxp14System.TObjectp17System.TMetaClass][18453]
553531 [Vcl.Controls.pas][Vcl.Controls][Controls.TControl.Click][7596]
56C28B [Vcl.StdCtrls.pas][Vcl.StdCtrls][Stdctrls.TCustomButton.Click][5609]

 

type

  TReminderItem = record
  public
    RecordID: Integer;
    EmployeeID: Integer;
    EmployeeName: String;
    HireDate: TDate;
    LastDate: TDate;
    Notes: String;
    procedure Clear;
  end;

  TForm1 = class(TForm)
    Button1: TButton;
    procedure Button1Click(Sender: TObject);
  private
    procedure GetReminder(AList: TList<TReminderItem>);
  end;

var
  Form1: TForm1;

implementation

{$R *.dfm}

procedure TForm1.Button1Click(Sender: TObject);
var
  LList : TList<TReminderItem>;
  LReminderItem : TReminderItem;
  PReminderItem : ^TReminderItem;
begin
  LList := TList<TReminderItem>.Create;
  try
    GetReminder(LList);
    ShowMessage(LList[0].EmployeeName);
  finally
    for LReminderItem in LList do
      LReminderItem.Clear;
    LList.Free;
  end;
end;

procedure TForm1.GetReminder(AList :TList<TReminderItem>);
var
  LReminderItem: ^TReminderItem;
begin
  New(LReminderItem);
  LReminderItem.RecordID := 1;
  LReminderItem.EmployeeID := 1;
  LReminderItem.EmployeeName := 'Test';
  LReminderItem.HireDate := Now;
  LReminderItem.LastDate := Now;;
  LReminderItem.Notes := 'A Note';

  AList.Add(LReminderItem^);
end;

{ TReminderItem }

procedure TReminderItem.Clear;
begin
  Self := Default(TReminderItem);
end;
This is just my latest attempt, I have tried Finalize, FinalizeRecord, FreeMem. The Clear procedure is also an attempt after seeing it in a different post.

 

What is the proper way to free memory of a record I create in a list?

 

Thanks in advance,

Gary

Unit1.pas

Unit1.dfm

[Stefan Glienke 2019]

- you allocate memory but don't dispose it

- calling Clear on a value type that is being returned from the list in a for in loop won't do anything on the item still being stored in the list.

 

There is no point in dynamically allocating those records and then storing them as TReminderItem.

Either don't allocate and work with TReminderItem (but then you cannot change the fields on values stored in the list - value type semantics), or store as PReminderItem and think of disposing them.

Or simply make TReminderItem a class and use TObjectList<T> which will take care of freeing them.

Edited March 28, 2021 by Stefan Glienke

[David Heffernan 2353]

You would destroy those items using Dispose, since you made them with New. But that's the wrong solution. 

 

Use TList<TReminderItem> which will give you a type safe collection whose items are allocated and destroyed by the list itself. 

[Gary 18]

So how do I do it? I have tried Dispose,

finally
    for LReminderItem in LList do
      Dispose(LReminderItem);
    LList.Free;
  end;

I get a compiler error incompatible type

 

if I try (No doubt wrong) using the correct type

var

PReminderItem : ^TReminderItem;

 

and

finally
    for LReminderItem in LList do
    begin
      PReminderItem := @LReminderItem;
      Dispose(PReminderItem);
    end;
    LList.Free;
  end;

It compiles but I get an exception

 

Also David notice the list is indeed a TList<TReminderItem>.

 

if I just leave out the attempt to free the memory and just free the List I get the Memory leak

[Stefan Glienke 2019]

Dynamically allocating records and then not storing them as reference won't work because you just create memory to then do a value copy once you add the item to the list making the dynamically allocated memory useless.

 

I suggest you read up on the difference between value and reference types - I am sure there is a chapter about that in Object Pascal Handbook by Marco Cantu which should be available for free to you as owner of Delphi 10.4

Edited March 28, 2021 by Stefan Glienke

[David Heffernan 2353]

Uff, I didn't read your code properly. I assumed you were using TList. So, yeah you just declare a local variable of the record type, populate it, and add it. As Stefan says, clarity on value vs reference semantics is key. 

[Gary 18]

Thanks Stefan,

I have always been confused by records and pointers, but with the new custom managed records wanted to give it another try since it looks like records for just data storage are simpler to work with than classes. I will probably just use a class with simple properties but here is the revised code with record pointer. I do need to change Data as I will use the record to read data from a database and fill the list with records.

 

TReminderItem = record
    FEmployeeID: Integer;
    FEmployeeName: String;
    FHireDate: TDate;
    FLastDate: TDate;
    FNotes: String;
  end;

  PReminderItem = ^TreminderItem;

 

procedure TForm1.Button1Click(Sender: TObject);
var
  LList : TList<PReminderItem>;
  LReminderItem : PReminderItem;
begin
  LList := TList<PReminderItem>.Create;
  try
    GetReminder(LList);
    ShowMessage(Format('Name: %s', [LList[0]^.FEmployeeName]));
  finally
    for LReminderItem in LList do
      Dispose(LReminderItem);
    LList.Free;
  end;
end;

procedure TForm1.GetReminder(AList: TList<PReminderItem>);
var
  LReminderItem : PReminderItem;
begin
//  LReminderItem := TReminderItem.Create;
  New(LReminderItem);

  LReminderItem^.FEmployeeID := 1;
  LReminderItem^.FEmployeeName := 'Test Name';
  LReminderItem^.FHireDate := Now;
  LReminderItem^.FLastDate := Now;
  LReminderItem^.FNotes := 'A Note';

  AList.Add(LReminderItem);
end;
 

Thanks again

Gary

[David Heffernan 2353]

You really should not be trying to manage allocation manually. Your should try to avoid that. 

[Gary 18]

Ya, so is it bad practice to have a simple class like

 

  TReminderItem = class
  public
    EmployeeID: Integer;
    EmployeeName: String;
    HireDate: TDate;
    LastDate: TDate;
    Notes: String;
  end;

without the requisite properties, getters and setters just to move data around? 

[PatV 1]

Hi

 

You can still use record and forget pointers

 

  TReminderItem = record
    FEmployeeID: Integer;
    FEmployeeName: String;
    FHireDate: TDate;
    FLastDate: TDate;
    FNotes: String;
  end;

  PReminderItem = ^TreminderItem;

  TForm1 = class(TForm)
    Button1: TButton;
    procedure Button1Click(Sender: TObject);
  private
    procedure GetReminder(AList: TList<TReminderItem>);
    { Private declarations }
  public
    { Public declarations }
  end;

var
  Form1: TForm1;

implementation

{$R *.dfm}


procedure TForm1.Button1Click(Sender: TObject);
var
  LList : TList<TReminderItem>;
  LReminderItem : PReminderItem;
begin
  LList := TList<TReminderItem>.Create;
  try
    GetReminder(LList);
    ShowMessage(Format('Name: %s', [LList[0].FEmployeeName]));
  finally
//    for LReminderItem in LList do
//      Dispose(LReminderItem);
    LList.Free;
  end;
end;

procedure TForm1.GetReminder(AList: TList<TReminderItem>);
var
  LReminderItem : TReminderItem;
begin
//  LReminderItem := TReminderItem.Create;
  LReminderItem:=default(TReminderITem);
  with LReminderItem do
  begin
     FEmployeeID := 1;
     FEmployeeName := 'Test Name';
     FHireDate := Now;
     FLastDate := Now;
     FNotes := 'A Note';
  end;
  AList.Add(LReminderItem);
end;