Print Page - BUG: Memory leak at ProjectManagerUI::BuildProjectTree

Developer forums (C::B DEVELOPMENT STRICTLY!) => Development => Topic started by: lights_joy on January 22, 2014, 12:07:00 pm

Title: BUG: Memory leak at ProjectManagerUI::BuildProjectTree
Post by: lights_joy on January 22, 2014, 12:07:00 pm

I'm using c::b 13.12, there is a memory leak at ProjectManagerUI::BuildProjectTree:

void ProjectManagerUI::BuildProjectTree(cbProject* project, cbTreeCtrl* tree, const wxTreeItemId& root, int ptvs, FilesGroupsAndMasks* fgam)
{
....

   // Now add any virtual folders
   const wxArrayString& virtualFolders = project->GetVirtualFolders();
   for (size_t i = 0; i < virtualFolders.GetCount(); ++i)
   {
   ftd = new FileTreeData(project, FileTreeData::ftdkVirtualFolder);
   ftd->SetFolder(virtualFolders);
   ProjectAddTreeNode(project, tree, virtualFolders, project->GetProjectNode(), true,
   FileTreeData::ftdkVirtualFolder, true, vfldIdx, ftd);
   }
.....
}

in the function ProjectAddTreeNode, it allocates a new ftd and copy the content.
so the ftd pointer here will cause a memory leak.
wxTreeItemId ProjectAddTreeNode(cbProject *project, wxTreeCtrl* tree, const wxString& text, const wxTreeItemId& parent,
   bool useFolders, FileTreeData::FileTreeDataKind folders_kind, bool compiles, int image,
   FileTreeData* data)
{
.....

   if (useFolders && pos >= 0)
   {
            .........

   if (!newparent)
   {
   // in order not to override wxTreeCtrl to sort alphabetically but the
   // folders be always on top, we just search here where to put the new folder...
   int fldIdx = cbProjectTreeImages::FolderIconIndex();
   int vfldIdx = cbProjectTreeImages::VirtualFolderIconIndex();

   newparent = ProjectFindNodeToInsertAfter(tree, folder, parent, true);

   FileTreeData* ftd = new FileTreeData(*data);
   ftd->SetKind(folders_kind);
   if (folders_kind != FileTreeData::ftdkVirtualFolder)
   ftd->SetFolder(project->GetCommonTopLevelPath() + GetRelativeFolderPath(tree, parent) + folder + wxFILE_SEP_PATH);
   else
   ftd->SetFolder(GetRelativeFolderPath(tree, parent) + folder + wxFILE_SEP_PATH);
   ftd->SetProjectFile(nullptr);
   int idx = folders_kind != FileTreeData::ftdkVirtualFolder ? fldIdx : vfldIdx;
   newparent = tree->InsertItem(parent, newparent, folder, idx, idx, ftd);
   }
   ret = ProjectAddTreeNode(project, tree, path, newparent, true, folders_kind, compiles, image, data);
   }
.......
}

Title: Re: BUG: Memory leak at ProjectManagerUI::BuildProjectTree
Post by: ollydbg on January 22, 2014, 12:55:50 pm

It looks like the bug you recently reported BUG: Memory Leak at cbProject::VirtualFolderAdded (http://forums.codeblocks.org/index.php/topic,18756.msg128514.html#msg128514) goes here now. :)

Title: Re: BUG: Memory leak at ProjectManagerUI::BuildProjectTree
Post by: MortenMacFly on January 22, 2014, 06:09:16 pm

And I am telling again: These objects are de-allocated by wxWidgets on destruction of the tree.

Its a user-data to wx tree items. If you delete them you may even cause crashes... please read the documentation accordingly. I've posted them in another thread dealing with the same issue...

Edit: Read here:
http://docs.wxwidgets.org/trunk/classwx_tree_item_data.html

Quote

The main advantage of having this class is that wxTreeItemData objects are destroyed automatically by the tree and, as this class has virtual destructor, it means that the memory and any other resources associated with a tree item will be automatically freed when it is deleted.

Title: Re: BUG: Memory leak at ProjectManagerUI::BuildProjectTree
Post by: Jenna on January 22, 2014, 06:35:36 pm

Quote from: MortenMacFly on January 22, 2014, 06:09:16 pm

And I am telling again: These objects are de-allocated by wxWidgets on destruction of the tree.

The problem here is, that the items are created with new and when they are added, they get copied with the default copy contructor.
The treeitems created with the copy ctor will be destroyed by wxWidgets, but the items created with new will persist and lead to a memory leak.

Title: Re: BUG: Memory leak at ProjectManagerUI::BuildProjectTree
Post by: Jenna on January 22, 2014, 08:23:59 pm

I just uplodaded a valgrind logfile, that shows many (possible) memory leaks, many of them in wxTreeCtrl related code.

It's just codeblocks without plugins, build against wx 2.8.12 on Fedor 20 64-bit with gcc 4.8.

http://apt.jenslody.de/downloads/codeblocks18065.vg.xz (http://apt.jenslody.de/downloads/codeblocks18065.vg.xz)
It has a little more than 8 MB (more than 600 MB unzipped).

Title: Re: BUG: Memory leak at ProjectManagerUI::BuildProjectTree
Post by: sodev on January 22, 2014, 08:40:38 pm

It *might* lead to a memory leak, the original FileTreeData object gets passed to a recursive call to ProjectAddTreeNode() that does not add it to a tree in every case, however i don't know if these cases can actually happen in the way this method gets called in that context. Without any deeper understanding of the code i would say it would be a good idea to delete the object inside ProjectAddTreeNode() when it returns without adding it to a tree.

Title: Re: BUG: Memory leak at ProjectManagerUI::BuildProjectTree
Post by: oBFusCATed on January 26, 2014, 02:57:25 pm

I agree with sodev. This is a patch that implements the suggestion. I don't see any other problem with the function.
Does someone know how to make C::B execute the changed paths?
I've set some breakpoints on the added delete calls, but I couldn't make C::B hit them with my projects.

Code

Index: src/src/projectmanagerui.cpp
===================================================================
--- src/src/projectmanagerui.cpp        (revision 9609)
+++ src/src/projectmanagerui.cpp        (working copy)
@@ -2535,7 +2535,10 @@ wxTreeItemId ProjectAddTreeNode(cbProject* project, wxTreeCtrl* tree,  const wxS
     wxTreeItemId ret;

     if (text.IsEmpty())
+    {
+        delete data;
         return ret;
+    }

     wxString path = text;

@@ -2548,7 +2551,10 @@ wxTreeItemId ProjectAddTreeNode(cbProject* project, wxTreeCtrl* tree,  const wxS
         path.Remove(0, 1);

     if (path.IsEmpty())
+    {
+        delete data;
         return ret;
+    }

     int pos = path.Find(_T('/'));
     if (pos == -1)

@jens: I can't download your valgrind log file. :(

Title: Re: BUG: Memory leak at ProjectManagerUI::BuildProjectTree
Post by: oBFusCATed on January 26, 2014, 03:43:15 pm

After looking at the other topic about the same issue, I've been able to reproduce it, so I'm committing this change.

Title: Re: BUG: Memory leak at ProjectManagerUI::BuildProjectTree
Post by: Jenna on January 26, 2014, 03:58:02 pm

Quote from: oBFusCATed on January 26, 2014, 02:57:25 pm

@jens: I can't download your valgrind log file. :(

Should work now, sorry.

Code::Blocks Forums

Developer forums (C::B DEVELOPMENT STRICTLY!) => Development => Topic started by: lights_joy on January 22, 2014, 12:07:00 pm