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.
HopeSeekr
Earned a small fee
Earned a small fee
Posts: 16
Joined: Sun Aug 29, 2004 1:07 pm
Contact:

Post by HopeSeekr » Fri May 06, 2005 2:56 am

That stuff was accepted is actually shocking. I agree to recant all of my statements on faith that you are correct, even though I have not validated it myself. I apologize. Btw, how many accepted for my old nick "Un-Thesis"?

metalogic
Super wx Problem Solver
Super wx Problem Solver
Posts: 307
Joined: Fri Oct 08, 2004 8:21 am
Location: Area 51
Contact:

Post by metalogic » Fri May 06, 2005 7:59 am

Avi wrote: Current workaround: turn wxUSE_STL ON (define it to 1).
I thought one of the big changes in 2.6 was that STL was the default. Am I incorrect in thinking that or was there a change of plans?

- Santiago

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 » Fri May 06, 2005 8:08 am

metalogic wrote:
Avi wrote: Current workaround: turn wxUSE_STL ON (define it to 1).
I thought one of the big changes in 2.6 was that STL was the default. Am I incorrect in thinking that or was there a change of plans?

- Santiago
I know, I thought so too... But appearently it is not so (at least for wxMSW users who use Visual Studio 200x to compile wxWidgets). Now, lets go back on topic... Anybody (devs/users) cares to check some of the internal workings of the wxString (actually it's the wxStringBase class w/ it's wxString Data objects) class? I need some help here! My C++ knowledge is pretty limited, especially when it comes down to using C-Runtime functions (alloc(), free(), relloc()). :roll:

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 » Fri May 06, 2005 8:22 am

HopeSeekr wrote:That stuff was accepted is actually shocking. I agree to recant all of my statements on faith that you are correct, even though I have not validated it myself. I apologize. Btw, how many accepted for my old nick "Un-Thesis"?
"Un-Thesis" send 5 patches: 2 of them were accepted, 3rd was duplication of the 4th, 4th was rejected because was related to adding new functionality to deprecated class which was against sense of deprecating, 5th was rejected because it was a workaround without none clear purpose (according to what Vadim explnation) and there is none answer from you which could add argumentation to Vadim action so there is no way to know that there is really strong research behind preparation of 5th patch.

You know perfectly why you was called "let's just say 'mistaken'" and it was unrelated to your patches and comments written there. Please do not spread false facts about how the developers care about patches.

EDITED: some unreadable broken English fixes.

ABX
Last edited by ABX on Fri May 06, 2005 11:21 am, edited 1 time in total.
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

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 » Fri May 06, 2005 8:30 am

Avi wrote:Anybody (devs/users) cares to check some of the internal workings of the wxString (actually it's the wxStringBase class w/ it's wxString Data objects) class? I need some help here!
Sorry, I'm somehow lost in the issues raised in this thread.

1. Can I reproduce it using makefile.vc route of building?
2. Can you provide complete minimal.cpp sample or diff to it?
3. Can you provide lib/vc_lib/*/build.cfg which describes your settings?

Thanks in advance, 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 » Fri May 06, 2005 2:54 pm

Well, I just reinstalled wxWidgets 2.6.0 to follow the exact steps I'm doing:
A) Double click "wxMSW-2.6.0-Setup.exe" and run the installation to "C:\wxWidgets-2.6.0" (default location).
B) Open the "C:\wxWidgets-2.6.0\build\msw" dir, and double click "wx.dsw". Visual Studio 2003 now opens and asks me if I would like to convert project files... I clicked "Yes To All".
C) I clicked "File"->"Save All" to save the newly converted project files and the newly (automatically) generated Solution.
D) I open the file "C:\wxWidgets-2.6.0\include\wx\msw\setup.h" and change the following option: #define WXWIN_COMPATIBILITY_2_4 0 (turns off wxWidgets 2.4.x compatibility). Note: wxUSE_STL is defined to 0 by default. Note 2: there is no need to change wxUSE_UNICODE to 1 since the project configurations "Unicode Release/Debug" set it by themselves.
E) I clicked "File"->"Save setup.h" to save the change I've made.
F) I now change the Solution configuration to "Unicode Debug" and press the "Start" button (looks like a play button actually :)).
G) Visual Studio asks if I would like to build the "out of date" configuations ... I clicked "Yes"
H) Repeat steps F+G to the "Unicode Release" configuration...

I hope that's all you need to determine the type of compilation I ran. ;)

Now, what do you mean when you say minimal.cpp diff? I didn't change a single line in the minimal sample. I compiled it (same way I compiled the library-> converted project files and all) and then ran it inside GlowCode which showed me the leaks.

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 » Fri May 06, 2005 3:30 pm

@ABX, I just ran minimal.exe (debug on) inside GlowCode and saved its memory leaks report to a text file. I uploaded the text file to the net and you can download it here. :wink:

Hopefully this report will help you determine the cause of GlowCode's finidings. An important thing to look at is the SIZES of the leaks, which are very constant! (52, 84, 116, and 148 bytes; I expanded the report tree to show more information for each of these)

Hope this helps

EDIT: maybe it will also help you that sizeof(wxStringData) is 12 here?

jms
Experienced Solver
Experienced Solver
Posts: 54
Joined: Wed Sep 29, 2004 6:37 am
Location: Finland

Post by jms » Fri May 06, 2005 5:40 pm

Avi,

I wonder why nobody has commented your code findings yet. Well, I'll go ahead... don't worry, I'll try not to bash you :)
Avi wrote:

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)?
Maybe because it would double the size of wxString? I really like the fact that wxString itself is only 4 bytes in size - in comparison, VC6 std::string is something like 16 bytes.
Avi wrote: *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 quite sure that the unit of nAllocLength is characters, not bytes.
Avi wrote: 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?
Actual contents of the string (that is, what data() returns) is already part of the wxStringData allocation. Thus, freeing the wxStringData will automatically free that memory as well.
Avi wrote: Memory usage went from 4372KB (before going to the first loop) to 4464KB (after leaving the second loop).
I don't think you can rely on this information to conclude that there is a memory leak, since, in Windows, app-runtime or OS itself does not necessarily relinquish freed memory immediately (AFAIK, this is done to optimize the memory management).

---

Now, I was a little curious about these leaks myself as well, so I made a little test:

To find out, for certain, that the number of string mallocs was balanced by the same amount of frees, I replaced string mallocs and frees with functions that modified a global counter as well and wrote results back to a file (I'm quite sure I didn't miss anything in string.h/cpp). Since you reported that the minimal sample leaked, it would seem probable that any wx app would leak, so I tested just some wx app I was working with (much larger than minimal), also adding a simplified (less iterations) version of your string check code to the mix. The End Result: Number of mallocs was same as number of frees (I did not repeat the test for release build, but I understand you detected the leaks in both builds equally).

With this information at hand, I'm inclined not to believe that there are actual memory leaks. One possibility that my test did not take into account is that the pointers fed to the free() calls are somehow bogus. However, in my experience, that should result to either a crash or a very loud RTL error message (especially since there are tons of strings to free).

Regards,
Jaakko Salli

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 » Fri May 06, 2005 5:54 pm

jms wrote:I'm inclined not to believe that there are actual memory leaks.
Yes, most probably. When I asked about how to reproduce with makefile.vc I got a description how to open project files so I was able only to google because I have only command line VC. Googling lead me to section "My program runs fine on Windows but core dumps on Linux" at http://www.cs.tau.ac.il/~sorkine/resources/cprog.htm and into http://msdn.microsoft.com/library/defau ... g_heap.asp . The listing send by Avi from his debugging tool shows that fields of wxStringData structure reflects the one I could "by hand" calculate doing analyze for the string "status_line" mentioned in available log. The problem is that text within log indicates that it presents data at the time of allocation so my quess is that it is destroyed later. Definietly I do not have your environment to duplicate when :-(

(ps. sorry for unreadable english)

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 » Fri May 06, 2005 6:21 pm

@jms: Thank you. You pretty much cleared up any question I wrote here. The test you did with the malloc()/free() counters is the best thing to do to check this... I wonder why I didn't think of that. :D I also thank you for taking the time for reading and replying to all the information I posted here.

@ABX: Sorry for my ignorance! I saw that .vc and my mind completely blocked the "makefile" right-part, and so I thought you were asking about the way I built wxWidgets on my computer (which is a valid question). I apologive. Well, maybe now someone can copy the steps to the wiki and it will help others as well. :) I thank you for looking at the logs I uploaded.

I thank you both. :!:

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 » Fri May 06, 2005 6:57 pm

Oh, and almost forgot...
From /include/wx/string.h, wxStringData struct:
size_t nDataLength, // actual string length
nAllocLength; // allocated memory size
maybe the above should be changed to:
size_t nDataLength, // actual string length
nAllocLength; // allocated memory size (in characters)

RyanAlt
In need of some credit
In need of some credit
Posts: 6
Joined: Tue May 03, 2005 8:49 am

Post by RyanAlt » Sun May 08, 2005 9:16 pm

jms wrote:I wonder why nobody has commented your code findings yet. Well, I'll go ahead... don't worry, I'll try not to bash you :)
Heh heh. The reason for this is that few people have an intamite knowledge of wxString I suppose. Vadim and I were the unofficial maintainers, with me booted now Vadim is.

Anyway, you've cleared up most of this, so I'll just comment on the rest :). Thanks!
Avi wrote:

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)?
Maybe because it would double the size of wxString? I really like the fact that wxString itself is only 4 bytes in size - in comparison, VC6 std::string is something like 16 bytes.
IIRC VC6 is 12 bytes - one for the string pointer with the ref count and two size_ts for the length and size members. Later ones use the "short string optimization" in which strings less than or equal to 16 bytes are not allocated - those are 24 bytes (without ref counting IIRC).

In actuality though wxString is the same size as VC6's string class (16 bytes). The difference is that wxString's buffer is preceded by its ref count, length and size whereas VC6's string is only preceded by its ref count and the length and size are kept as class members.

Avi - the reason for this is that if you lay out the memory like so
m_pchData
[wxStringData][actual string buffer]
12 bytes # of characters * sizeof(wxChar)

You only have to call malloc once, otherwise you'd have to call it once for the ref count and the string buffer.

Supposively you do it this way so that you can pass it to printf without calling c_str also (since the only member of wxString is its character buffer), but this doesn't work on a lot of compilers anyway.
I got voted off the wxIsland!

Post Reply