VirtualTreeView Memory Leak with Objects

Question

I'm using the code provided by Cosmin Prund in this post because it fits my needs however I often get memory leak and I could not figure out how to free the node which it's TNode object that contain TObjectList in turn this last can also contain TNode that also contain TObjectList and so on... it's some kind of recursion I though,

As I know to free the node in VirtualTreeView according to this link the node need to be validated and to be finalised within the OnFreeNode event this code return Invalid pointer operation and of course memory leak report:

procedure TfrmFichePermission.VSTFreeNode(Sender: TBaseVirtualTree;
  Node: PVirtualNode);
var
  AObject:TObject;
  ANode: TNode;
begin
  AObject := TObject(VST.GetNodeData(Node)^);
  ANode := TNode(AObject);
  ANode.Free;
end;

this is a full example to reproduce the memory leak

unit Unit1;

interface

uses
  Winapi.Windows, Winapi.Messages, System.SysUtils, System.Variants, System.Classes, Vcl.Graphics,
  Vcl.Controls, Vcl.Forms, Vcl.Dialogs, VirtualTrees, System.Generics.Collections,
  Vcl.StdCtrls;

type
  TNode = class;

  TForm1 = class(TForm)
    VST: TVirtualStringTree;
    Button1: TButton;
    procedure VSTGetNodeDataSize(Sender: TBaseVirtualTree;
      var NodeDataSize: Integer);
    procedure VSTGetText(Sender: TBaseVirtualTree; Node: PVirtualNode;
      Column: TColumnIndex; TextType: TVSTTextType; var CellText: string);
    procedure Button1Click(Sender: TObject);
  private
    procedure AddNodestoTree(ParentNode: PVirtualNode; Node: TNode);
  public
    { Public declarations }
  end;

  TNode = class
  public
    Name: string;
    VTNode: PVirtualNode;
    Sub: TObjectList<TNode>;
    constructor Create(aName: string);
    destructor Destroy; override;
  end;

var
  Form1: TForm1;

implementation

{$R *.dfm}

constructor TNode.Create(aName:string);
begin
  Name := aName;
  Sub := TObjectList<TNode>.Create;
end;

destructor TNode.Destroy;
begin
  Sub.Free;
end;

procedure TForm1.AddNodestoTree(ParentNode: PVirtualNode; Node: TNode);
var SubNode: TNode;
    ThisNode: PVirtualNode;
begin
  ThisNode := VST.AddChild(ParentNode, Node); // This call adds a new TVirtualNode to the VT, and saves "Node" as the payload

  Node.VTNode := ThisNode; // Save the PVirtualNode for future reference. This is only an example,
                           // the same TNode might be registered multiple times in the same VT,
                           // so it would be associated with multiple PVirtualNode's.

  for SubNode in Node.Sub do
    AddNodestoTree(ThisNode, SubNode);
end;

procedure TForm1.Button1Click(Sender: TObject);
var
  Root: TNode;
begin
  ReportMemoryLeaksOnShutdown := True;
  VST.Clear;
  //
  Root := TNode.Create('Test1');
  Root.Sub.Add(TNode.Create('Test2'));
  Root.Sub.Add(TNode.Create('Test3'));
  Root.Sub[1].Sub.Add(TNode.Create('Test4'));
  Root.Sub[1].Sub.Add(TNode.Create('Test5'));
  AddNodesToTree(nil, Root);
  //
  Root := TNode.Create('Test1');
  Root.Sub.Add(TNode.Create('Test2'));
  Root.Sub.Add(TNode.Create('Test3'));
  Root.Sub[1].Sub.Add(TNode.Create('Test4'));
  Root.Sub[1].Sub.Add(TNode.Create('Test5'));
  AddNodesToTree(nil, Root);
  //
end;
procedure TForm1.VSTGetNodeDataSize(Sender: TBaseVirtualTree;
  var NodeDataSize: Integer);
begin
  NodeDataSize := SizeOf(Pointer);
end;

procedure TForm1.VSTGetText(Sender: TBaseVirtualTree; Node: PVirtualNode;
  Column: TColumnIndex; TextType: TVSTTextType; var CellText: string);
var
    AObject:TObject;
    ANode: TNode;
begin
  AObject := TObject(VST.GetNodeData(Node)^);
  ANode := TNode(AObject);
  CellText := ANode.Name;
end;

end.

Memory Leak report :

Answer 1

Cosmin's code does not intend for the tree view nodes to own the TNode objects. I think in his post you were meant to hold on to the Root object and destroy it after the tree is destroyed.

In Cosmin's code the TNode objects are owned by the object list that contains them. Right the way up to the root node which is owned by whatever created it. You could do that too. You'd have to remember the root object, and stop destroying the TNode objects when tree view nodes were destroyed.

If you want to have the tree view own the TNode objects then you can do that. But you need to be clear about the ownership. You can't have the tree view and the object lists owning the objects, as you currently do. If the tree view is going to be the owner then you need to set OwnsObjects to False in the object list. Or even better switch to TList<TNode>.

So, to summarise, your present code gives each TNode object two owners. The tree view node and the owning object list. Objects need to have exactly one owner. You need to choose between the two options.