Disposing of Object added to TStringList

user_1735540 0
Viewed 585

Am I doing this right?
I am populating the objects in a TStringlist with records which I create during populating. I have set the TStringList.OwnsObjects to true. However when I close down the application I get an access violation. Below is an extract of my code.

type
  PUsageData = ^TUsageData;
  TUsageData = record
    DeclaredIn: String;
    LineNumber: Integer;
    TotalUsage: Integer;
  end;

...

var VarUsages: TStringList;    // object contains a TUsageData record


procedure BuildUsageList;
var idx, idy, n,m: Integer;
    sl: TStringList;
    s,t: String;
    rec: PUsageData;
begin
  VarUsages.Clear;
  sl:= TStringList.Create;
  s := '';
  // First make a list and create a record for each variable declared in program
  for idx := 0 to IncludeList.Count -1 do begin
    GetSource(IncludeList[idx], sl);
    for idy := 0 to sl.Count -1 do begin
      t := '';
      t := CodeAnalyser.GetItems(sl[idy], caVariables);
      if t > '' then begin
        system.New(Rec);
        rec.DeclaredIn := IncludeList[idx];
        rec.LineNumber := idy;
        rec.TotalUsage := 0;
        VarUsages.AddObject(t, TObject(rec));
      end;
    end;
  end;
end;

...

initialization
  VarUsages := TStringList.Create;
  VarUsages.OwnsObjects := true;

finalization
  VarUsages.Free;
end.

I am using Delphi 10.1.

I am also wondering what happens when I clear the StringList. Does it dispose the Objects?

delphi
1 Answers

You have at least three ways to handle this:

1. Disposing records manually

Because you are storing pointers to records, and not TObject descendants, you can't rely on OwnsObjects. Set it to False!

Before you Free the TStringList, you should manually Dispose of each record:

for I := 0 to VarUsages.Count - 1 do
  Dispose(PUsageData(VarUsages.Objects[i]));

Note that the cast to PUsageData is important for Dispose to work properly.

Because you set OwnsObjects to False, the string list will not try to free the "objects", so you can immediately call:

VarUsages.Free;

Note that Dispose will take care of properly finalizing the record, including the string inside.

Note: As David Heffernan commented: don't forget that, as well as manually disposing when the string list is destroyed, you must do so whenever you remove or modify an item. That makes the other two options even more attractive, IMO.

2. Using objects (class instances) instead of records

Alternatively, you can turn TUsageData into a simple class (no methods, no properties, only public fields):

type
  TUsageData = class
  public
    DeclaredIn: string;
    LineNumber: Integer;
    TotalUsage: Integer;
  end;

Then you can set OwnsObjects to True and let the string list take care of freeing them. That scenario would work for ARC managed targets too. Instead of New, you use Rec := TUsageData.Create;. You can otherwise use the same code, as far as I can tell.

3. Not using TStringList

Instead of a string list, use something like a TList<TUsageData>, where TUsageData can remain a record. No need to use pointers. Just put the t string in the record too. Since you are dealing with records, there is no need to free anything, except of course the TList<> itself. Freeing that will take care of finalizing the records inside it too.

  Rec: TUsageData; // record!
  ...
  t := CodeAnalyser.GetItems(sl[idy], caVariables);
  if t > '' then 
  begin
    Rec.t := t;
    Rec.DeclaredIn := IncludeList[idx];
    Rec.LineNumber := idy;
    Rec.TotalUsage := 0;
    VarUsages.Add(Rec);
user_959540
Related