Jump to content

Recommended Posts

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

Share this post


Link to post
Posted (edited)

- 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 by Stefan Glienke

Share this post


Link to post

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. 

  • Like 1

Share this post


Link to post

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

Share this post


Link to post
Posted (edited)

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 by Stefan Glienke

Share this post


Link to post

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. 

Share this post


Link to post

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

Share this post


Link to post

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? 

Share this post


Link to post

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;

 

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×