wxString leaks memory? :O (wxWidgets 2.6.0)

This forum can be used to talk about general design strategies, new ideas and questions in general related to wxWidgets. If you feel your questions doesn't fit anywhere, put it here.
Avi
Super wx Problem Solver
Super wx Problem Solver
Posts: 398
Joined: Mon Aug 30, 2004 9:27 pm
Location: Tel-Aviv, Israel

wxString leaks memory? :O (wxWidgets 2.6.0)

Post by Avi » Thu May 05, 2005 2:14 am

wxString leaks memory? :shock:
I used a new tool, GlowCode, to check for memory leaks in my program. It appears that it found tons of leaks ALL coming from wxStringBase::AllocBuffer (lines 248-249):

Code: Select all

  wxStringData* pData = (wxStringData*)
    malloc(sizeof(wxStringData) + (nLen + EXTRA_ALLOC + 1)*sizeof(wxChar));
and wxStringBase::Alloc (lines 368-369):

Code: Select all

      wxStringData* pData = (wxStringData*)
          malloc(sizeof(wxStringData) + (nLen + 1)*sizeof(wxChar));
The good news is, that after turning wxUSE_STL ON (wxString now derives from std::string), ALL leaks are gone...

Now, I heard this tool (e.g. gives you true stuff) is really good so I'm kinda worried for wxWidgets users who use the "old" wxString class.

If you guys want me to recompile wxWidgets with STL usage off and give you more information regarding the leak just let me know.

Hopefully, we will all get to the bottom of this! :roll:

Avi
Super wx Problem Solver
Super wx Problem Solver
Posts: 398
Joined: Mon Aug 30, 2004 9:27 pm
Location: Tel-Aviv, Israel

Post by Avi » Thu May 05, 2005 2:16 am

Oh, and I can give you function call stack and exact number of bytes leaked... (or simply provide you with a simple screenshot)

I almost forgot to say I'm using Microsoft Visual C++ 2003 on Windows XP+SP2 (English).

EDIT: oh, and I have Unicode turned ON... It might help you guys track this down, since I'm seeing the sizeof(wxChar) part in the malloc()... :oops:

Avi
Super wx Problem Solver
Super wx Problem Solver
Posts: 398
Joined: Mon Aug 30, 2004 9:27 pm
Location: Tel-Aviv, Israel

Post by Avi » Thu May 05, 2005 2:34 am

Wow, trying to understand some of the wxString class is nutz! :)
Some stuff that makes me go "ouch" are:

Code: Select all

  wxStringData* GetStringData() const { return (wxStringData*)m_pchData - 1; }
* Why not keep a (wxStringBase class-member) pointer to the wxStringData, instead of going back to that piece of memory every time (knowing that it's there of course)?

*Also, shouldn't line 258 of string.cpp be:

Code: Select all

  pData->nAllocLength = (nLen + EXTRA_ALLOC)*sizeof(wxChar);
instead of:

Code: Select all

  pData->nAllocLength = nLen + EXTRA_ALLOC;
?
I mean, wouldn't it leak half the amount of memory allocated under Unicode builds? (where sizeof(wxChar)==2) :?

I'm just a C++ beginner, so don't bash me if I'm completely wrong... :oops:

Avi
Super wx Problem Solver
Super wx Problem Solver
Posts: 398
Joined: Mon Aug 30, 2004 9:27 pm
Location: Tel-Aviv, Israel

Post by Avi » Thu May 05, 2005 2:44 am

Looking more into this crazy-ass class, I found the following:

Code: Select all

void wxStringData::Free()
{
    free(this);
}
Now, that's great and everything (and correct), but where the rest of the malloc()'ed memory is free()'d? (the part where the actual characters lie in) Or am I completely wrong and the free(this) will free ALL the block of memory allocated by malloc()? (I'm learning C++, but malloc() and free() are C-style, so I am not sure.

To explain myself better... I'll start here:

Code: Select all

  ~wxStringBase()
  {
#if defined(__VISUALC__) && (__VISUALC__ >= 1200)
      //RN - according to the above VC++ does indeed inline this,
      //even though it spits out two warnings
      #pragma warning (disable:4714)
#endif

      GetStringData()->Unlock();
  }
The destructor of wxStringBase is calling Unlock().
Unlock() either calls Free() (a regular non-inline function; under non-DLL builds I think) OR is doing the free()'ing by itself (as an inline, under DLL builds I think).

Given that the wxStringData isn't empty, and no other wxStringBase object is using that wxStringData), it calls free(this) (one way or another, as described above)... but shouldn't it also free(data()) first?
What I mean is, changing line 236 of string.h (and line 183 in string.cpp on a DLL wx build) to:

Code: Select all

  void  Unlock() { if ( !IsEmpty() && --nRefs == 0) { free(data(); free(this); } }
instead of:

Code: Select all

  void  Unlock() { if ( !IsEmpty() && --nRefs == 0) free(this);  }

Avi
Super wx Problem Solver
Super wx Problem Solver
Posts: 398
Joined: Mon Aug 30, 2004 9:27 pm
Location: Tel-Aviv, Israel

Post by Avi » Thu May 05, 2005 3:28 am

Ok, I now recompiled wxWidgets 2.6.0 in a Unicoded Non-STL build again. Some of the memleaks reports I get look like:

Code: Select all

-a block @0x15FD2D8, 52 bytes, malloc called from  wxStringBase::AllocBuffer();  file c:\wxwidgets-2.6.0\src\common\string.cpp; line 249 + 30 bytes; module xMule-Debug.exe; 0x582DA7 : lReq:23463
  -find pointers to this block
     Cannot perform mult-level expand!
  -call stack during allocation
     caller(1): wxStringBase::AllocBuffer();  file c:\wxwidgets-2.6.0\src\common\string.cpp; line 249 + 35 bytes; module xMule-Debug.exe; 0x582DAC
     caller(2): wxStringBase::InitWith();  file c:\wxwidgets-2.6.0\src\common\string.cpp; line 207 + 12 bytes; module xMule-Debug.exe; 0x582C22
     caller(3): wxStringBase::wxStringBase();  file c:\wxwidgets-2.6.0\include\wx\string.h; line 323 + 55 bytes; module xMule-Debug.exe; 0x5747F7
     caller(4): wxString::wxString();  file c:\wxwidgets-2.6.0\include\wx\string.h; line 648 + 77 bytes; module xMule-Debug.exe; 0x57478D
     caller(5): TextHnd();  file c:\wxwidgets-2.6.0\src\xml\xml.cpp; line 443 + 32 bytes; module xMule-Debug.exe; 0x753023
     caller(6): doContent();  file c:\wxwidgets-2.6.0\src\expat\lib\xmlparse.c; line 2304 + 35 bytes; module xMule-Debug.exe; 0x7585B0
     caller(7): contentProcessor();  file c:\wxwidgets-2.6.0\src\expat\lib\xmlparse.c; line 1778 + 33 bytes; module xMule-Debug.exe; 0x75973C
     caller(8): XML_ParseBuffer();  file c:\wxwidgets-2.6.0\src\expat\lib\xmlparse.c; line 1445 + 66 bytes; module xMule-Debug.exe; 0x75697A
  -dump bytes
     015FD2D8  01  00  00  00  04  00  00  00  13  00  00  00  74  00  65  00    . . . . . . . . . t. e. 
     015FD2E8  78  00  74  00  00  00  CD  CD  CD  CD  CD  CD  CD  CD  CD  CD    x. t. . . ֽֽֽֽֽֽֽֽֽֽ
     015FD2F8  CD  CD  CD  CD  CD  CD  CD  CD  CD  CD  CD  CD  CD  CD  CD  CD    ֽֽֽֽֽֽֽֽֽֽֽֽֽֽֽֽ
     015FD308  CD  CD  CD  CD  ??  ??  ??  ??  ??  ??  ??  ??  ??  ??  ??  ??    ֽֽֽֽ. . . . . . . . . . . . 
  -dump dwords
     015FD2D8    00000001  00000004  00000013  00650074  
     015FD318    00740078  CDCD0000  CDCDCDCD  CDCDCDCD  
     015FD358    CDCDCDCD  CDCDCDCD  CDCDCDCD  CDCDCDCD  
     015FD398    CDCDCDCD  ????????  ????????  ????????  
Maybe it is all simply because GlowCode doesn't see a direct pointer to the wxStringData object? :roll: I'm not sure though...
I might check it tomorrow (it's already 6:30am, crap! debugging wxString takes time!) by simply saving an "unused" pointer to the wxStringData object in the wxStringBase class... :)

Jorg
Moderator
Moderator
Posts: 3971
Joined: Fri Aug 27, 2004 9:38 pm
Location: Delft, Netherlands
Contact:

Post by Jorg » Thu May 05, 2005 7:25 am

Hi Avi

To break through this monologue have you tried valgrind or the likes to see what they produce with the same code?

It must be weird that the most used class in wxWidgets is so leaky.

Regards,
- Jorgen
Forensic Software Engineer
Netherlands Forensic Insitute
http://english.forensischinstituut.nl/
-------------------------------------
Jorg's WasteBucket
http://www.xs4all.nl/~jorgb/wb

Avi
Super wx Problem Solver
Super wx Problem Solver
Posts: 398
Joined: Mon Aug 30, 2004 9:27 pm
Location: Tel-Aviv, Israel

Post by Avi » Thu May 05, 2005 7:47 am

Well, first, I would like to apologize for this long monologue... I was "on a roll" last night while writing it (I couldn't believe my C++ skills allowed me to userstand such a complicated class)... About trying Valgrind, I do not have Linux installed atm, so I cannot do it. I would like for someone on the wx-dev-team to check this monologue and tell me if I (and this GlowCode program) got it all wrong... :oops:

A test case can be built to check if this class is indeed leaky (probably a while loop creating thousands and thousands of wxStrings, then we can check the memory usage)...

Avi
Super wx Problem Solver
Super wx Problem Solver
Posts: 398
Joined: Mon Aug 30, 2004 9:27 pm
Location: Tel-Aviv, Israel

Post by Avi » Thu May 05, 2005 8:47 am

Well, I now ran the following simple piece of testcase:

Code: Select all

#include <wx/app.h>                    // wxApp
#include <wx/msgdlg.h>                 // wxMessageBox
#include <wx/string.h>                 // wxString

class wxStringCheck : public wxApp
{
public:
    virtual bool OnInit();
private:
    void CreateStringsStack();
    void CreateStringsHeap();
};

IMPLEMENT_APP(wxStringCheck)

bool wxStringCheck::OnInit()
{
    wxMessageBox(wxT("Check memory usage and click OK to continue..."));
    for (int i=5000; i>=0;--i)
        CreateStringsStack();
    wxMessageBox(wxT("Finished allocating on stack.\nCheck memory usage again and click OK to continue..."));
    for (int i=5000; i>=0;--i)
        CreateStringsHeap();
    wxMessageBox(wxT("Finished allocating on heap.\nCheck memory usage again and click OK to finish..."));
    return false;
}

void wxStringCheck::CreateStringsStack()
{
    for (int i=1000; i>=0;--i)
    {
        wxString FirstString=wxT("Just a Stack wxString");
		wxString SecondString=FirstString+FirstString;
    }
}

void wxStringCheck::CreateStringsHeap()
{
    for (int i=1000; i>=0;--i)
    {
        wxString* FirstString=new wxString(wxT("Just a Heap wxString"));
		delete FirstString;
    }
}
Memory usage went from 4372KB (before going to the first loop) to 4464KB (after leaving the second loop).

Now, test it on your computer, guys. Use wxWidgets 2.6.0, Unicode ON, and STL-usage OFF(#define wxUSE_STL 0). And monitor the memory usage in before/after each message box. I used Spy++ to check exact process memory usage, you can use any other program of course. :roll:

UPDATE: Running the test case while monitorring it in GlowCode showed no memory leak coming out of CreateStringsStack() or CreateStringsHeap(). :roll: On the other hand, GlowCode did show a leak in 128 Blocks, 8576bytes. Maybe I have to use some special strings or string combinations which will call Alloc() and AllocBuffer()? :?

Will keep you guys updated...

HopeSeekr
Earned a small fee
Earned a small fee
Posts: 16
Joined: Sun Aug 29, 2004 1:07 pm
Contact:

Post by HopeSeekr » Thu May 05, 2005 1:36 pm

Carlo Wood and I first discovered these memory leaks back in March 2004 and I have announced this fact several times on the wx-dev lists, both on threads I have started and in corroboration with others experiencing the same (such as this one).

They don't show up on *default* valgrind because you have to use a lower tolerence for errors, but they still exist where as similiar leaks in Qt and GTK-- do not.

After Mr. Wood analyzed the situation, he discovered that approximately 10KB of memory leaks for every wxString that is appended to (I believe) and the total leaks for a large application such as xMule accounted for ~10MB of residual memory usage.

[begin flame]
The fix? There officially was no need for a fix, as we were told that our pure-STL version of wxString (which has none of these problems, is a lot smaller, and much faster) was already implemented at that time (2.5.0) with the flag wxUSE_STL. When I pointed out that several wx functions did not work with their string in this mode, there was no real alternative except to point out that the memory "leaks" aren't really leaks at all; and beside, why didn't I submit a patch to wxUSE_STL instead of providing a complete 100% STLized version of wxString as I had done already.
[end flame]

This was a *long* time ago; they've probably fixed their wxUSE_STL a long time ago (I would assume) and thus why isn't wXUSE_STL used by default? I assume that their wxUSE_STL version of wxString does not have these problems, but that is just an assumption.

The pure STL version of wxString can be found in xMule's old CVS as UniString.h.

User avatar
ABX
Can't get richer than this
Can't get richer than this
Posts: 810
Joined: Mon Sep 06, 2004 1:43 pm
Location: Poznan, Poland
Contact:

Post by ABX » Thu May 05, 2005 2:16 pm

HopeSeekr wrote:Carlo Wood and I first discovered these memory leaks back in March 2004 and I have announced this fact several times on the wx-dev lists
Which of http://lists.wxwidgets.org/cgi-bin/ezml ... n:0:200402 ?

ABX
CVS Head, 2.8.X
wxMSW, wxWinCE, wxPalmOS, wxOS2, wxMGL, bakefile
gcc 3.2.3, bcc 5.51, dmc 8.48, ow 1.6, vc 7.1, evc 3/4, pods 1.2

Avi
Super wx Problem Solver
Super wx Problem Solver
Posts: 398
Joined: Mon Aug 30, 2004 9:27 pm
Location: Tel-Aviv, Israel

Post by Avi » Thu May 05, 2005 2:53 pm

HopeSeekr wrote:I assume that their wxUSE_STL version of wxString does not have these problems, but that is just an assumption.
Like I said before in this post, when wxUSE_STL is on, no mem-leaks seem to be there... What I'm trying to fix here, is the wxString for non-STL'ed compilers (so it won't leak)...

Anyway, I need some feedback here guys. This is a hard class to understand. Also, I haven't completely proven it's leaking memory *yet*! I need some explanation about the code parts I already posted here.

The most important line in my posts is:
Maybe it is all simply because GlowCode doesn't see a direct pointer to the wxStringData object?
@ABX: are you the dev maintaining the wxString class? Are you familiar with it's internal structure/works? Could you point this post out on the mailing list (so that the other devs will check this out as well)?

@HopeSeekr: IF there is indeed a leak, it is NOT of 10KB per wxString FOR SURE. GlowCode itself shows leaks of sizes: 52 bytes, 84 bytes, 116bytes, (each per Alloc() or AllocBuffer() operation) but no more! On the other hand, on a VERY simple program (that contains very limited amount of wxString manipulation)- it found 6187 blocks leaking 346028 bytes (or 337.91KB). That being said, I cannot even imagine the ammount of leaking on a large wxWidgets-based application.


Current workaround: turn wxUSE_STL ON (define it to 1).

HopeSeekr
Earned a small fee
Earned a small fee
Posts: 16
Joined: Sun Aug 29, 2004 1:07 pm
Contact:

Post by HopeSeekr » Thu May 05, 2005 3:01 pm

http://lists.wxwidgets.org/archive/wx-u ... 62372.html

Sorry tihs one is from 15 July 2004

The test case was example/minimal and as you can *plainly* see back then at least it was leaking

~ definately lost: 228 bytes in 9 blocks.
~ possibly lost: 4868 bytes in 111 blocks.
~ still reachable: 777630 bytes in 10111 blocks.
~ suppressed: 3591 bytes in 63 blocks.

which is pretty horrible for minimal.

This was *not* the first time I reported htis, merely the first one i found, searching for [wxString "memory leak" xmule]

Cheers,
-hope-

User avatar
ABX
Can't get richer than this
Can't get richer than this
Posts: 810
Joined: Mon Sep 06, 2004 1:43 pm
Location: Poznan, Poland
Contact:

Post by ABX » Thu May 05, 2005 3:55 pm

Avi wrote:@ABX: are you the dev maintaining the wxString class? Are you familiar with it's internal structure/works? Could you point this post out on the mailing list (so that the other devs will check this out as well)?
I'm not a maintainer of wxString and I think there is nobody in such position. This is so vital part of wxWidgets that all should care... I do not see what's going on yet. If you would find it, please refer asap :-)

ABX
CVS Head, 2.8.X
wxMSW, wxWinCE, wxPalmOS, wxOS2, wxMGL, bakefile
gcc 3.2.3, bcc 5.51, dmc 8.48, ow 1.6, vc 7.1, evc 3/4, pods 1.2

HopeSeekr
Earned a small fee
Earned a small fee
Posts: 16
Joined: Sun Aug 29, 2004 1:07 pm
Contact:

Post by HopeSeekr » Thu May 05, 2005 10:22 pm

I don't think I understand...after over a year of being called -- let's just say "mistaken" -- is there finally some dialog along the lines that it *might* be possible? if so, i'm duly impressed [seriously].

I already have had a solution that is ready for production for a year now if you want I can submit it to SourceForge...but every one of my patches there gets rejected, so I just keep them 'forked' in xMule until some smuck submits THOSE to sf and then they get accepted ... albeit w/o proper [e.g. zero] citation :-(

leio
Can't get richer than this
Can't get richer than this
Posts: 802
Joined: Mon Dec 27, 2004 10:46 am
Location: Estonia, Tallinn
Contact:

Post by leio » Thu May 05, 2005 11:19 pm

HopeSeekr wrote:I don't think I understand...after over a year of being called -- let's just say "mistaken" -- is there finally some dialog along the lines that it *might* be possible? if so, i'm duly impressed [seriously].

I already have had a solution that is ready for production for a year now if you want I can submit it to SourceForge...but every one of my patches there gets rejected, so I just keep them 'forked' in xMule until some smuck submits THOSE to sf and then they get accepted ... albeit w/o proper [e.g. zero] citation :-(
I was able to find two of your patches on sf.net, of which 100% were commited. Also I found two bug reports, of which all were fixed aswell. In the case you used a different sf.net username earlier than June 2004, then the four accepted tracker items in the second part of 2004 should have given enough positive feedback for you imho to not say what you say now.

Could we pretty please continue discussing the issue at hand and leave some kind of personal beefs and sarcasm aside?
Compilers: gcc-3.3.6, gcc-3.4.5, gcc-4.0.2, gcc-4.1.0 and MSVC6
OS's: Gentoo Linux, WinXP; WX: CVS HEAD

Project Manager of wxMUD - http://wxmud.sf.net/
Developer of wxGTK;
gtk+ port maintainer of OMGUI - http://www.omgui.org/

Post Reply