11

Why does this program report memory leaks?

{$APPTYPE CONSOLE}

uses
  System.Generics.Collections;

type
  TDerivedGenericObjectList = class(TObjectList<TObject>)
  public
    constructor Create;
  end;

constructor TDerivedGenericObjectList.Create;
begin
  inherited;
end;

var
  List: TDerivedGenericObjectList;

begin
  ReportMemoryLeaksOnShutdown := True;
  List := TDerivedGenericObjectList.Create;
  List.Add(TObject.Create);
  List.Free;
end.
12
  • 1
    Refactoring can help.
    – smooty86
    Commented Sep 15, 2015 at 6:04
  • 1
    Your leaks are because your code is defective. We cannot see your code. In any case why subclass at all. You can use TObjectList<TPerson> directly. Unless you want to add methods to it. Commented Sep 15, 2015 at 6:07
  • @DavidHeffernan I've edited the question adding a constructor and usage. That code, with TObjectList, doesn't leak. But if I change the TMembers declaration to be TMembers = class(TObjectList<TPerson>) now the code above shows memory leaks. And yes, I want to add methods to it. What I am showing here is just a simple made-up example for a, hopefully, clear question.
    – alondono
    Commented Sep 15, 2015 at 6:33
  • 1
    I re-wrote your question to make it ask just what I believe you intended to ask, as per your comments. Also, with the question list this, it made it easier for me to answer!! ;-) Commented Sep 15, 2015 at 7:06
  • 1
    I my experience a simple complete program like this makes answering much easier, and it helps you too because the focus is clear and unobstructed. Commented Sep 15, 2015 at 7:19

2 Answers 2

14

You are calling the parameterless constructor of TObjectList<T>. That is in fact the constructor of TList<T>, the class from which TObjectList<T> is derived.

All the constructors declared in TObjectList<T> accept a parameter named AOwnsObjects which is used to initialise the OwnsObjects property. Because you are bypassing that constructor, OwnsObjects is defaulting to False, and the members of the list are not being destroyed.

You should make sure that you call constructor of TObjectList<T> that initialise OwnsObjects. For instance:

{$APPTYPE CONSOLE}

uses
  System.Generics.Collections;

type
  TDerivedGenericObjectList = class(TObjectList<TObject>)
  public
    constructor Create;
  end;

constructor TDerivedGenericObjectList.Create;
begin
  inherited Create(True);
end;

var
  List: TDerivedGenericObjectList;

begin
  ReportMemoryLeaksOnShutdown := True;
  List := TDerivedGenericObjectList.Create;
  List.Add(TObject.Create);
  List.Free;
end.

Perhaps a better variant would be to make your constructor also offer the AOwnsObjects parameter:

type
  TDerivedGenericObjectList = class(TObjectList<TObject>)
  public
    constructor Create(AOwnsObjects: Boolean = True);
  end;

constructor TDerivedGenericObjectList.Create(AOwnsObjects: Boolean);
begin
  inherited Create(AOwnsObjects);
end;

Or:

type
  TDerivedGenericObjectList = class(TObjectList<TObject>)
  public
    constructor Create(AOwnsObjects: Boolean = True);
  end;

constructor TDerivedGenericObjectList.Create(AOwnsObjects: Boolean);
begin
  inherited;
end;

So, you might wonder why the original version picked out a TList<T> constructor rather than one in TObjectList<T>. Well, let's look at that in more detail. Here's your code again:

type
  TDerivedGenericObjectList = class(TObjectList<TObject>)
  public
    constructor Create;
  end;

constructor TDerivedGenericObjectList.Create;
begin
  inherited;
end;

When inherited is used in this way, the compiler looks for a constructor with the exact same signature as this one. It cannot find one in TObjectList<T> because they all have parameter. It can find one in TList<T>, and so that's the one that it uses.

As you mention in comments the following variant does not leak:

constructor TDerivedGenericObjectList.Create;
begin
  inherited Create;
end;

This syntax, by contrast to the bare inherited, will find methods that match when default parameters are substituted. And so the single parameter constructor of TObjectList<T> is called.

The documentation has this information:

The reserved word inherited plays a special role in implementing polymorphic behavior. It can occur in method definitions, with or without an identifier after it.

If inherited is followed by the name of a member, it represents a normal method call or reference to a property or field, except that the search for the referenced member begins with the immediate ancestor of the enclosing method's class. For example, when:

inherited Create(...);

occurs in the definition of a method, it calls the inherited Create.

When inherited has no identifier after it, it refers to the inherited method with the same name as the enclosing method or, if the enclosing method is a message handler, to the inherited message handler for the same message. In this case, inherited takes no explicit parameters, but passes to the inherited method the same parameters with which the enclosing method was called. For example:

inherited;

occurs frequently in the implementation of constructors. It calls the inherited constructor with the same parameters that were passed to the descendant.

2
  • I have just tried the top solution and actually removing the True parameter also works. So, you only need inherited Create;.
    – alondono
    Commented Sep 15, 2015 at 7:11
  • 2
    But it's better to be explicit. My version of TObjectList<T> forces the caller to pass OwnsObjects. Commented Sep 15, 2015 at 7:15
1

You can use generics. It's work fine without type casting and memory leak ( TObjectList<T> or TObjectDictionary<T> lists destroy inner objects automatic on free command).

Some tips:

  • TObjectList<TPerson> -- destroy persons list automatic on free like membersList.Free;

  • TList<TPerson> -- do not destroy persons list. You must create destructor and free manually every person in list;

Here is example of your code (using new constructor, no memory leak and with backward compatibility with old code -- see GetPerson):

    type
      TPerson = class
      public
        Name: string;
        Age: Integer;

        function Copy: TPerson;
      end;

      TMembers = class(TObjectList<TPerson>)
      private
        function GetPerson(i: Integer): TPerson;
      public
        property Person[i: Integer]: TPerson read GetPerson;

        constructor Create(SourceList: TMembers); overload;
      end;


    { TPerson }

    function TPerson.Copy: TPerson;
    var
      person: TPerson;
    begin
      person := TPerson.Create;
      person.Name := Self.Name;
      person.Age := Self.Age;
      Result := person;
    end;

    { TMembers }

    constructor TMembers.Create(SourceList: TMembers);
    var
      person: TPerson;
    begin
      inherited Create;

      for person in SourceList do
      begin
        Self.Add(person.Copy);
      end;
    end;

    function TMembers.GetPerson(i: Integer): TPerson;
    begin
      Result := Self[i];
    end;

    procedure TForm21.Button1Click(Sender: TObject);
    var
      person: TPerson;
      memsList1: TMembers;
      memsList2: TMembers;
    begin
      // test code

      memsList1 := TMembers.Create;

      person := TPerson.Create;
      person.Name := 'name 1';
      person.Age := 25;
      memsList1.Add(person);

      person := TPerson.Create;
      person.Name := 'name 2';
      person.Age := 27;
      memsList1.Add(person);

      memsList2 := TMembers.Create(memsList1);

      ShowMessageFmt('mems 1 count = %d; mems 2 count = %d', [memsList1.Count, memsList2.Count]);

      FreeAndNil(memsList1);
      FreeAndNil(memsList2);
    end;
8
  • You should describe why there are no memleaks. Just the code will not help to understand.
    – Sir Rufo
    Commented Sep 15, 2015 at 6:38
  • To enable memory leak report add ReportMemoryLeaksOnShutdown := True; in project source file before Application.Initialize. If there are memory leak then report will be shown after application close.
    – JayDi
    Commented Sep 15, 2015 at 6:47
  • That is how you proove there are no memleaks, but not why?
    – Sir Rufo
    Commented Sep 15, 2015 at 6:50
  • Because generic class TObjectList<T> destroy all inner objects in that list automatically after free command.
    – JayDi
    Commented Sep 15, 2015 at 6:57
  • 1
    Well the OP inherit his class from TObjectList<T> and he has memleaks. That is the question that needs an answer.
    – Sir Rufo
    Commented Sep 15, 2015 at 6:59

Not the answer you're looking for? Browse other questions tagged or ask your own question.