Delphi error while returning TList

Question

I have made a very simple application but I have an issue that I really cannot understand. Look at this basic code:

unit Unit1;

interface

uses
  Winapi.Windows, Winapi.Messages, System.SysUtils, System.Variants, System.Classes, Vcl.Graphics,
  Vcl.Controls, Vcl.Forms, Vcl.Dialogs, generics.collections, Vcl.StdCtrls;

type
  TForm1 = class(TForm)
    Button1: TButton;
    procedure Button1Click(Sender: TObject);
    procedure FormCreate(Sender: TObject);
    procedure FormDestroy(Sender: TObject);
  private
    { Private declarations }
    test: TList<integer>;
    aList: TList<integer>;
  public
    { Public declarations }
    function testGenerics: TList<integer>;
  end;

var
  Form1: TForm1;

implementation

{$R *.dfm}

procedure TForm1.Button1Click(Sender: TObject);
begin
 test := testGenerics;
 test.Sort;
 showmessage(test[0].tostring);
end;

procedure TForm1.FormCreate(Sender: TObject);
begin
 test := TList<integer>.Create;
 aList := TList<integer>.Create;
end;

procedure TForm1.FormDestroy(Sender: TObject);
begin
 aList.Free;
 test.Free;
end;

function TForm1.testGenerics: TList<integer>;
begin

  aList.Add(4);
  result := aList;

end;

end.

Basically when the Form opens I am going to create test and aList and then when I press the button the function testGenerics is called. Why do I have the Invalid pointer operation error?

I really cannot understand since I am creating and destroying the objects (I guess) properly. This code instead works fine:

function TForm1.testGenerics: TList<integer>;
begin

  Result := TList<integer>.Create;
  Result.Add(4);

end;

In this case I am returning an instance of TList<integer> but also in the case above I am returning an instance of aList (which is a TList).

If I'm correct in the first case test := testGenerics is like test := aList (because I am returning aList in fact) so I am going to give test the same reference as aList. Am I correct?

Remy Lebeau · Accepted Answer · 2017-08-25 01:42:05Z

4

In the first example, whenever you call testGenerics(), you are re-assigning test to point at the aList object. You are losing track of the original test object created in the OnCreate event, so it is leaked. And then in the OnDestroy event, when you call test.Free, it crashes because you already freed the aList object beforehand, so you are trying to free the same object a second time, which is an invalid operation.

In the second example, you are still leaking the original test object (and every TList you allocate and assign to test, except for the last one), but you are not re-assigning test to point at the aList object anymore, so there is no crash in the OnDestroy event because both variables are pointing at separate objects.

What are you trying to accomplish in the first place? Returning objects in this manner is not good practice. Nor does it make sense to call Sort() on 1-element lists.

If you are trying to populate test with multiple values over time, you should pass test as an input parameter to testGenerics() (or just let testGenerics() access test directly via Self), don't use the return value at all.

And in any case, get rid of your aList private member, as you are not doing anything with it anyway.

Try this:

unit Unit1;

interface

uses
  Winapi.Windows, Winapi.Messages, System.SysUtils, System.Variants, System.Classes, Vcl.Graphics,
  Vcl.Controls, Vcl.Forms, Vcl.Dialogs, generics.collections, Vcl.StdCtrls;

type
  TForm1 = class(TForm)
    Button1: TButton;
    procedure Button1Click(Sender: TObject);
    procedure FormCreate(Sender: TObject);
    procedure FormDestroy(Sender: TObject);
  private
    { Private declarations }
    test: TList<integer>;
  public
    { Public declarations }
    procedure testGenerics(aList: TList<integer>);
  end;

var
  Form1: TForm1;

implementation

{$R *.dfm}

procedure TForm1.Button1Click(Sender: TObject);
begin
  testGenerics(test);
  test.Sort;
  ShowMessage(test[0].tostring);
end;

procedure TForm1.FormCreate(Sender: TObject);
begin
  test := TList<integer>.Create;
end;

procedure TForm1.FormDestroy(Sender: TObject);
begin
  test.Free;
end;

procedure TForm1.testGenerics(aList: TList<integer>);
begin
  // FYI, a better way to exercise Sort()
  // would be to use RandomRange() instead
  // of a hard-coded number...
  aList.Add(4);
end;

end.

edited Aug 25, 2017 at 1:42

answered Aug 25, 2017 at 1:10

Remy Lebeau

609k36 gold badges516 silver badges875 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Raffaele Rossi Over a year ago

Thank you Remy! to fix the leak I should use the 2nd example and remove the creation of test in FormCreate? Then the test.free inside ondestroy is fine from what I have undeestood

Remy Lebeau Over a year ago

That will only prevent a leak the first time you call testGenerics(), but will still leak on subsequent calls since you are not calling test.Free before calling testGenerics().

Collectives™ on Stack Overflow

Delphi error while returning TList

1 Answer 1

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related