TCustomShellTreeView.GetFilesInDir memory leak? lazarus / 31-LCL ---------------------------------------------------------------------- circular · 2016-07-03 15:23 Hello, I have noticed a memory leak in LazPaint. After investigating, it seems that it comes from a call to GetFilesInDir of the class TCustomShellTreeView. It populates a TStringList that is freed by my program however there are many little memory blocks that seem to remain allocated. ---------------------------------------------------------------------- lainz · 2016-07-03 15:47 Hi, this is the solution: you need to free the objects in the TStringList. Code: Pascal  [Select][+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i var  s: TStringList;  i: integer;begin  s := TStringList.Create();  ShellTreeView1.GetFilesInDir('C:\Temp\', '*.*', [otFolders, otNonFolders], s);   for i:=0 to s.Count-1 do    s.Objects[i].Free;  s.Free; In line 623 of ShellCtrls: "AResult will contain TFileItem objects upon return, make sure to free them in the calling routine" ---------------------------------------------------------------------- Bart · 2016-07-03 15:51 GetFilesInDirs adds objects (TFileItems) to the AResult: TStrings parameter. It is the responsability of the caller of GetFilesInDir to free these items: Code: [Select]    //don't free the TFileItems here, they will freed by the calling routine     Files.Free; Maybe setting the TSringList's OwnsObjects property to TRUE will fix the memory leak. [Edit] Lainz beat me to it. Bart ---------------------------------------------------------------------- wp · 2016-07-03 16:14 Quote from: Bart on July 03, 2016, 05:51:15 pmGetFilesInDirs adds objects (TFileItems) to the AResult: TStrings parameter. It is the responsability of the caller of GetFilesInDir to free these items: I think this is a hackish, quick-and-dirty approach. A clean dedicated list type for the TFileItems which automatically cleans up would be safer in my eyes. ---------------------------------------------------------------------- Bart · 2016-07-03 16:23 The problem is the the TFileItems are attached (not copied) to the tree-items, so they should not be automatically freed. A separate list woul need to be guaranteed to be in sync with the AResult parameter, otherwise the TFileItem objects of the tree items don't match up. An optional parameter AResultOwnsFileItems: Boolean = False may be of help? Bart ---------------------------------------------------------------------- wp · 2016-07-03 17:44 Quote from: Bart on July 03, 2016, 06:23:57 pmThe problem is the the TFileItems are attached (not copied) to the tree-itemsMaybe I don't understand the code correctly. But PopulateTreeNodeWithFiles and PopulateWithRoot are the two methods of TCustomShellTreeview and TCustomShellListView which call  GetFilesInDir. They use a local stringlist for GetFilesInDir; the stringlist has OwnsObjects=true and is destroyed at the end of these methods. Therefore, if the FileItems were only attached to the treenodes/listitems they would be gone here. This can't be. This is part of what really happens in TCustomShellTreeView.PopulateTreeNodeWithFiles: Code: Pascal  [Select][+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i       NewNode := Items.AddChildObject(ANode, Files.Strings[i], nil);      TShellTreeNode(NewNode).FFileInfo := TFileItem(Files.Objects[i]).FileInfo;This does not attach the TFileItem to the treenode, it copies the TFileItem.FileInfo to the TShellTreeNode.FFileInfo (both of them are TSearchRec). Therefore it is safe to destroy the TFileInfo objects when the stringlist is destroyed. To be on the safe side an exception should be raised if the stringlist passed to GetFilesInDir has OwnsObjects = false. Of course, this would break existing code where the programmer frees the objects manually in a for loop. ---------------------------------------------------------------------- Bart · 2016-07-03 18:35 Quote from: wp on July 03, 2016, 07:44:12 pmMaybe I don't understand the code correctly. But PopulateTreeNodeWithFiles and PopulateWithRoot are the two methods of TCustomShellTreeview and TCustomShellListView which call  GetFilesInDir. They use a local stringlist for GetFilesInDir; the stringlist has OwnsObjects=true and is destroyed at the end of these methods. Therefore, if the FileItems were only attached to the treenodes/listitems they would be gone here. This can't be. Yes, you are right. The TShellTreeNode class just has some extra fields, none of them a class or pointer as I mistakenly believed. My mistake. It's a long time since I wrote that. Quote from: wp on July 03, 2016, 07:44:12 pmTo be on the safe side an exception should be raised if the stringlist passed to GetFilesInDir has OwnsObjects = false. At the time I considered setting OwnsObjects to True inside TCustomShellTreeView.GetFilesInDir, but that cannot be done, since the parameter is defined as being a TStrings, which does not have this property! So, neither can an exception be raised (nor can my previous suggestion be implemented). The behaviour is well documented in the code. Hoovering the mouse over a call to TCustomShellTreeView.GetFilesInDir however does not show this warning in the hint, whereas it propery display all the other text that is in the comments right above it's definition. Maybe the proper xml file (in docs/xml) should be updated? Bart ---------------------------------------------------------------------- wp · 2016-07-03 18:56 Just some brainstorming: Deprecate the GetFilesInDir and overload it with a new variant which gets a "TFileItemList" (a TObjectList of TFileItem) as a parameter? The TFileItemList could have a readonly property FileName[AIndex] and/or a method ExtractFileNames which writes the filenames to a TStrings for those who need the bare stringlist from GetFilesInDir. ---------------------------------------------------------------------- Bart · 2016-07-03 19:45 IIRC then this class function uses TStrings for it's parameter to make it as generic as possible? Better ask on devel list. I don't mind implementing it. Bart ---------------------------------------------------------------------- circular · 2016-10-22 16:17 Somehow I had missed this answer. Thank you very much for the explanation. Learning something new every day.