News:

The new Release 25.03 is out! You can download binaries for Windows and many major Linux distros here .

Main Menu

BUG: Memory leak at ProjectManagerUI::BuildProjectTree

Started by lights_joy, January 22, 2014, 12:07:00 PM

Previous topic - Next topic

lights_joy

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);
   }
.......    
}



ollydbg

If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

MortenMacFly

#2
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.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: [url="https://www.codeblocks.org/docs/main_codeblocks_en.html"]https://www.codeblocks.org/docs/main_codeblocks_en.html[/url]
C::B FAQ: [url="https://wiki.codeblocks.org/index.php?title=FAQ"]https://wiki.codeblocks.org/index.php?title=FAQ[/url]

Jenna

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.

Jenna

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
It has a little more than 8 MB (more than 600 MB unzipped).

sodev

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.

oBFusCATed

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.


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. :(
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

oBFusCATed

After looking at the other topic about the same issue, I've been able to reproduce it, so I'm committing this change.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Jenna