Found a few memory leaks

[Der Meister]

As the topic says... I would fix them myself if there were not those damn wxArrays or wxHashArrays involved.  :lol:

Valgrind results:

==24949== 7836 (168 direct, 7668 indirect) bytes in 7 blocks are definitely lost in loss record 123 of 164
==24949==    at 0x1B904603: operator new(unsigned) (in /usr/lib/valgrind/vgpreload_memcheck.so)
==24949==    by 0x1BA6A1CB: MessageManager::DoAddLog(MessageLog*, wxString const&, wxBitmap const&) (messagemanager.cpp:400)
==24949==    by 0x1BA6AF7D: MessageManager::MessageManager() (string.h:325)
==24949==    by 0x1BA63CF5: Manager::GetMessageManager() const (manager.h:139)
==24949==    by 0x1BA63E91: Manager::Get(wxFrame*) (intl.h:556)
==24949==    by 0x80809D6: MainFrame::CreateIDE() (main.cpp:525)
==24949==    by 0x8081C23: MainFrame::MainFrame(wxWindow*) (main.cpp:458)
==24949==    by 0x8065638: CodeBlocksApp::InitFrame() (app.cpp:229)
==24949==    by 0x80667F1: CodeBlocksApp::OnInit() (app.cpp:390)
==24949==    by 0x80677C0: wxAppConsole::CallOnInit() (app.h:87)
==24949==    by 0x1C3D006B: wxEntry(int&, wchar_t**) (in /usr/lib/libwx_baseu-2.6.so.0.2.0)
==24949==    by 0x1C3D03ED: wxEntry(int&, char**) (in /usr/lib/libwx_baseu-2.6.so.0.2.0)
==24949==
==24949==
==24949== 5440 (280 direct, 5160 indirect) bytes in 14 blocks are definitely lost in loss record 127 of 164
==24949==    at 0x1B904603: operator new(unsigned) (in /usr/lib/valgrind/vgpreload_memcheck.so)
==24949==    by 0x1BA898CF: PluginManager::LoadPlugin(wxString const&) (pluginmanager.cpp:235)
==24949==    by 0x1BA89D09: PluginManager::ScanForPlugins(wxString const&) (pluginmanager.cpp:100)
==24949==    by 0x8077850: MainFrame::ScanForPlugins() (main.cpp:738)
==24949==    by 0x8081DE7: MainFrame::MainFrame(wxWindow*) (main.cpp:473)
==24949==    by 0x8065638: CodeBlocksApp::InitFrame() (app.cpp:229)
==24949==    by 0x80667F1: CodeBlocksApp::OnInit() (app.cpp:390)
==24949==    by 0x80677C0: wxAppConsole::CallOnInit() (app.h:87)
==24949==    by 0x1C3D006B: wxEntry(int&, wchar_t**) (in /usr/lib/libwx_baseu-2.6.so.0.2.0)
==24949==    by 0x1C3D03ED: wxEntry(int&, char**) (in /usr/lib/libwx_baseu-2.6.so.0.2.0)
==24949==    by 0x8062429: main (app.cpp:90)
==24949==
==24949==
==24949== 508 (40 direct, 468 indirect) bytes in 2 blocks are definitely lost in loss record 129 of 164
==24949==    at 0x1B904603: operator new(unsigned) (in /usr/lib/valgrind/vgpreload_memcheck.so)
==24949==    by 0x1BA8964F: PluginManager::LoadPlugin(wxString const&) (pluginmanager.cpp:203)
==24949==    by 0x1BA89D09: PluginManager::ScanForPlugins(wxString const&) (pluginmanager.cpp:100)
==24949==    by 0x8077850: MainFrame::ScanForPlugins() (main.cpp:738)
==24949==    by 0x8081DE7: MainFrame::MainFrame(wxWindow*) (main.cpp:473)
==24949==    by 0x8065638: CodeBlocksApp::InitFrame() (app.cpp:229)
==24949==    by 0x80667F1: CodeBlocksApp::OnInit() (app.cpp:390)
==24949==    by 0x80677C0: wxAppConsole::CallOnInit() (app.h:87)
==24949==    by 0x1C3D006B: wxEntry(int&, wchar_t**) (in /usr/lib/libwx_baseu-2.6.so.0.2.0)
==24949==    by 0x1C3D03ED: wxEntry(int&, char**) (in /usr/lib/libwx_baseu-2.6.so.0.2.0)
==24949==    by 0x8062429: main (app.cpp:90)
==24949==
==24949==
==24949== 60552 (22080 direct, 38472 indirect) bytes in 345 blocks are definitely lost in loss record 150 of 164
==24949==    at 0x1B904603: operator new(unsigned) (in /usr/lib/valgrind/vgpreload_memcheck.so)
==24949==    by 0x1BA1FC22: EditorColorSet::AddOption(wxString, OptionColor*, bool) (editorcolorset.h:33)
==24949==    by 0x1BA206BE: EditorColorSet::AddOption(wxString, wxString const&, int, wxColour, wxColour, bool, bool, bool, bool) (string.h:224)
==24949==    by 0x1BA34263: EditorLexerLoader::DoStyles(wxString, TiXmlElement*) (string.h:224)
==24949==    by 0x1BA35275: EditorLexerLoader::DoLexer(TiXmlElement*) (string.h:224)
==24949==    by 0x1BA35B14: EditorLexerLoader::Load(wxString const&) (editorlexerloader.cpp:69)
==24949==    by 0x1BA1EC92: EditorColorSet::LoadAvailableSets() (editorcolorset.cpp:108)
==24949==    by 0x1BA2394A: EditorColorSet::EditorColorSet(wxString const&) (editorcolorset.cpp:49)
==24949==    by 0x1BA42E6D: EditorManager::EditorManager() (string.h:325)
==24949==    by 0x1BA63C75: Manager::GetEditorManager() const (manager.h:139)
==24949==    by 0x807FF72: MainFrame::CreateMenubar() (editormanager.h:73)
==24949==    by 0x8080F08: MainFrame::CreateIDE() (main.cpp:537)

The correct line for the last one is editorcolorset.cpp:270 - valgrind doesn't print the correct line in every case. (But the others are correct).

[Der Meister]

I fixed these memory-leaks and replaced the wxArrays and wxHashMaps that were involved with std::vector and std::map. The patch can be found at berlios.de.

[Game_Ender]

I fixed these memory-leaks and replaced the wxArrays and wxHashMaps that were involved with std::vector and std::map. The patch can be found at berlios.de.

Maybe we should get together an effort like the Unicode switch to change over everything to STL containers.  The only issue is that a some wxwidgets classes take wx based containers as arguments so it does make it convenient to store things in the wx containers.

[rickg22]

IMHO, The unicode patching was a walk in the park compared with the replacing of wxArrays. Why? Because lots of wxWidgets functions use them and it'd be harder to distinguish which ones are safe to delete. Plus, not only we have to change the arrays, but the functions that use them, and some wxWidgets array manipulation functions don't have an STL equivalent.

[Der Meister]

Well, I found another memory-leak, but I could not fix it yet:

==7838== 56 bytes in 14 blocks are definitely lost in loss record 42 of 153
==7838==    at 0x1B904603: operator new(unsigned) (in /usr/lib/valgrind/vgpreload_memcheck.so)
==7838==    by 0x1BA8887D: PluginManager::LoadPlugin(wxString const&) (pluginmanager.cpp:120)
==7838==    by 0x1BA89829: PluginManager::ScanForPlugins(wxString const&) (pluginmanager.cpp:100)
==7838==    by 0x8077840: MainFrame::ScanForPlugins() (main.cpp:738)
==7838==    by 0x808D1E7: MainFrame::MainFrame(wxWindow*) (main.cpp:473)
==7838==    by 0x8065628: CodeBlocksApp::InitFrame() (app.cpp:229)
==7838==    by 0x80667E1: CodeBlocksApp::OnInit() (app.cpp:390)
==7838==    by 0x80677B0: wxAppConsole::CallOnInit() (app.h:87)
==7838==    by 0x1C3D006B: wxEntry(int&, wchar_t**) (in /usr/lib/libwx_baseu-2.6.so.0.2.0)
==7838==    by 0x1C3D03ED: wxEntry(int&, char**) (in /usr/lib/libwx_baseu-2.6.so.0.2.0)
==7838==    by 0x8062419: main (app.cpp:90)

The line that requests this memory is the following one:

Code

    wxDynamicLibrary* lib = new wxDynamicLibrary();

If the loading failes this memory will be released properly. But if it is sucessfull, every plugin loaded from this library will hold a pointer to this object. That means we can't just delete it if a plugin gets destroyed - another plugin might have this pointer, too, and will crash if it accesses this object or tries to delete it. Thus we need something like boost::shared_pointer here. This thing would perfectly solve this problem but as far as I know Code::Blocks will not use Boost.
Anyway, I implemented a simple version of boost::shared_pointer and added it. It seemed to work pretty good and the memory-leak should be gone (although I did not test it with valgrind). But I got a segfault. Not at the point were my shared-pointer was involved, this code worked without problem. Here is a part of the backtrace:

#0  0xb77fd38f in wxWindowBase::DestroyChildren () from /usr/lib/libwx_gtk2u_core-2.6.so.0
#1  0xb77024ea in wxWindow::~wxWindow () from /usr/lib/libwx_gtk2u_core-2.6.so.0
#2  0xb7823c6a in wxPanel::~wxPanel () from /usr/lib/libwx_gtk2u_core-2.6.so.0
#3  0xb7ef1c4f in ~wxFlatNotebook (this=0x840c860) at new_allocator.h:69
#4  0xb77fd287 in wxWindowBase::Destroy () from /usr/lib/libwx_gtk2u_core-2.6.so.0
#5  0xb770154e in wxWindow::Destroy () from /usr/lib/libwx_gtk2u_core-2.6.so.0
#6  0xb7e034e4 in ~MessageManager (this=0x8438d50) at messagemanager.cpp:187
#7  0xb7dffbe8 in Manager::Shutdown () at manager.h:147
#8  0x08082d41 in MainFrame::OnApplicationClose (this=0x825f770, event=@0x0) at main.cpp:1947
#9  0xb74a2cab in wxAppConsole::HandleEvent () from /usr/lib/libwx_baseu-2.6.so.0
#10 0xb754817b in wxEvtHandler::ProcessEventIfMatches () from /usr/lib/libwx_baseu-2.6.so.0
#11 0xb7548431 in wxEventHashTable::HandleEvent () from /usr/lib/libwx_baseu-2.6.so.0
#12 0xb754916e in wxEvtHandler::ProcessEvent () from /usr/lib/libwx_baseu-2.6.so.0
#13 0xb754910e in wxEvtHandler::ProcessEvent () from /usr/lib/libwx_baseu-2.6.so.0

This crash does not happen if I just remove the delete that deletes the wxDynamicLibrary-object from my shared-pointer. And this delete is never called with invalid data - I already checked this... But why crashes wxWidgets at this place if the libraries are unloaded correctly?

[Game_Ender]

Couldn't we just bring over the header?  I am pretty sure the shared_pointer is contained in a single *.hpp file.  A name space alias and a typedef to cbSharedPointer can even hide the fact that its a boost class.

[Der Meister]

No, this was my first idea, too. But this one header has a lot of dependencies. Using just these headers would be much to difficult and error-prone.
Anyway, I don't think that this would solve this one specific problem, because my small implementation seemed to work pretty well. I don't think that it would work with boost::shared_pointer.

[mandrav]

Guys, no boost::smart_ptr nor any other smart pointer here please. Plus we 'd then need boost::weak_ptr and so on. wxWidgets don't play well with these, especially in debug versions.
Besides, every object in C::B has a well-known lifetime. There's really no reason for refcounting...

DerMeister, thanks for the effort and the patch. Although it doesn't apply cleanly (it has failed hunks). What I don't get is why you changed all wx containers with their STL counterparts. This is quite a change that is not needed right now. It might even cause more problems in debug versions.
Don't get me wrong: I use STL every day. But I 'm a little reluctant to use it inside wx-based projects...

[takeshimiya]

Just something for home made projects, there is a modified boost::shared_ptr that plays nice with wxWindow's here:
http://wxforum.shadonet.com/viewtopic.php?p=26277#26277

Code: cpp

#include <boost/shared_ptr.hpp>
typedef boost::shared_ptr wx_ptr;
wx_ptr<wxMessageDialog> ptr(new wxMessageDialog(...));

About STL containers, there isn't a need for doing the conversion now, but sooner or later there will be a need...
As you may know, wx3 will make use of STL everywhere and even high likely some boost classes.

For anyone interested in the development of wxTNG see here: http://www.wxwidgets.org/wiki/index.php/WxWidgets3

[Der Meister]

Guys, no boost::smart_ptr nor any other smart pointer here please. Plus we 'd then need boost::weak_ptr and so on. wxWidgets don't play well with these, especially in debug versions.
Besides, every object in C::B has a well-known lifetime. There's really no reason for refcounting...

Well, I guess you're right. These memory-leaks are no real problem as they only appear once during the execution of Code::Blocks. But the only way to solve them would be something like a smart-pointer. But as this is no choice these memory-leaks will probably stay forever. Well, not a problem in this case... 

DerMeister, thanks for the effort and the patch. Although it doesn't apply cleanly (it has failed hunks). What I don't get is why you changed all wx containers with their STL counterparts. This is quite a change that is not needed right now. It might even cause more problems in debug versions.

Well, I didn't try to apply it to the current revision, maybe there is a problem now. But anyway, as most of the patch is the change from wx containers to stl it should be no problem. Just forget about the patch. The only parts about memory-leaks should be the lines inside the destructors or functions that are called from destructors. Maybe you have to change them a bit to work with wx containers.
The reason why I changed these containers is that I'm not familiar with wx containers but with the stl-containers. But as you and rickg22 already said these change is neither needed nor a really good idea...

[thomas]

But as you and rickg22 already said these change is neither needed nor a really good idea...

Actually, I think that it is not generally a bad idea. STL containers are a lot more transparent and less error-prone (for example, you cannot forget to include arrimpl.cpp because no such tampering is necessary). The class browser is able to follow a typedef, but it utterly fails on WX_DECLARE_XXX. It is also not immediately obvious what exactly some macros do (have to look at some of them for 20 seconds to understand). A typedef on a STL container is self-explanatory.

The same is true when you actually make a mistake (whatsoever) and have to read through unintellegible compiler errors that reference some code which you have never heard about coming from inside a container macro. Granted, STL error messages are not easy for a newbie either, but they are intellegible, because they are not just random garbage.

The problem with converting to STL containers is just that
a) we started using wx containers in the first place
b) it requires an unjustifiable amount of work to switch now (we simply don't have the time)
c) it is not feasonable in all places
d) it does not work with wxWidgets memory debugging (have to #undef new)

[Der Meister]

DerMeister, thanks for the effort and the patch. Although it doesn't apply cleanly (it has failed hunks).

I just updated the patch. It should now apply to revision 2274 and only removes the memory-leaks and not the wx containers.