On Fri, Nov 20, 2009 at 01:36:03AM -0800, Ferran wrote:
>
> Hello,
>
> I have developed an application that makes intensive use of wxString
> and it has a lot of threads running. After several minutes or hours
> running, the application crashes in "wxStringBase::AllocBuffer". I can
> debug the problem and it crashes in the following line of string.cpp:
>
> wxStringData* pData = (wxStringData*)
> malloc(sizeof(wxStringData) + (nLen + EXTRA_ALLOC + 1)*sizeof
> (wxChar));
>
> The nLen variable has value '4'.
>
> I am debugging in MSVC6 and it reports the following: "HEAP: Free Heap
> block f05be8 modified at f05c10 after it was freed".
Such things don't need much debugging, high-level analyzing is
much more important ;)
> wxStringInputStream stream(msg);
>
> if (m_pXmlDoc->Load(stream) == false)
> {
> LOG_ERROR(wxT("Error loading input stream string in document. Input
> string:") + msg);
> return false;
> }
>
> where msg is a valid wxString and m_pXmlDoc is a valid pointer to
> wxXmlDocument.
>
> And it also crashes (with the same problem) in:
>
> strRet += wxT("</") + pNode->GetName() + wxT(">\r\n");
>
> where strRet is an empty wxString and pNode is a valid pointer to
> wxXmlNode.
There is not much to deduce from these specific locations IMHO.
Other than maybe some thread-concurrent loading of wxXmlResource
resources, which certainly needs protection.
> I am using version 2.8.10 of wxWidgets in Windows XP.
>
> Do you have any idea of which can be the problem?
In case all other (explicit use, or also implicit use e.g. XML document loading
or so) string use in your app is properly mutex-locked,
the remaining issue might be the wxPostEvent non-thread-safe string-member event
that got dealt with in recent times (via wxQueueEvent()).
> Have you experienced a similar problem using wxString in multithread
> environments?
With improperly protected areas of code, certainly.
I'm hearing that from time to time in some code that is known to
have problems, which are difficult to analyze and awkward to correct due
to structural weakness.
A good way to debug this would be using helgrind / valgrind on e.g. wxGTK.
HTH,
Andreas Mohr
It's *definitively* better to use the latter (passing a const ref) both
performance and memory wise.
The first version makes a copy of the string while the second version in effect
only passes a pointer.
Cheers,
/uli
--
Uli Hertlein
Research and Development mailto:u...@xdt.com.au
XDT Pty Ltd http://www.xdt.com.au
AFAIK wxString uses copy on write, so it is not safe to pass wxString
objects between threads.
Regards,
Łukasz
F> Do you have any idea of which can be the problem?
Probably heap corruption in some other, completely unrelated place. It's
difficult to find such bugs which is why there are helper tools to do it.
If the bug also happens under Linux, use valgrind there. If not, you can
try Purify/BoundsChecker/... under Windows. There is also (free but less
"friendly") Microsoft app verifier tool. And I do assume that you already
run it using debug version of MSVC CRT which is capable of giving useful
messages/asserts when heap corruption is detected (you can manually insert
checks for heap integrity using _CrtXXX functions in your code).
F> Have you experienced a similar problem using wxString in multithread
F> environments?
wxString is not MT-safe so if you use the same string object (and be
careful here if you pass wxStrings between threads as they're not really
copied due to the use of COW in 2.8) this could definitely explain the
problem. If you don't do this then it shouldn't matter at all whether the
environment is multithreaded or not.
Regards,
VZ
--
TT-Solutions: wxWidgets consultancy and technical support
http://www.tt-solutions.com/
As far as I know, SetString and wxCommandEvent is a thread safe way to
pass wxString from the secondary to the main thread, since it makes a
copy of the string.
Marcus
newString = srcString.c_str() will definitely create a new independent copy.
Les
> I am debugging in MSVC6 and it reports the following: "HEAP: Free Heap
> block f05be8 modified at f05c10 after it was freed".
This is the critical point, and is likely to make it easy to find, given
the right tool. The right tool in this case is gflags, a Microsoft program
for changing the way the heap works to catch use-after-free bugs.
<http://support.microsoft.com/kb/286470>
gflags is part of Debugging Tools for Windows:
<http://www.microsoft.com/whdc/DevTools/Debugging/default.mspx>
This I guess is exactly the reason for wxQueueEvent() (wxCommandEvent
etc.) at http://article.gmane.org/gmane.comp.lib.wxwindows.general/61801
, thus the answer given all the way down to the bottom probably is wrong.
> newString = srcString.c_str() will definitely create a new independent copy.
Yup, but better add a comment, otherwise someone else going over it will be
confused in the mild case (or simply change it back to object assignment
in the sorry case).
Or, maybe better, implement a header inline helper (plus comment!) like
const char *string_assign_non_refcount(const wxString &in)
for such cases (for the sole purpose of documenting it centrally, of course).
(perhaps string_copy_for_thread() is a better name)
One should note that all this is talking about a correct implementation case
where string objects _are_ properly contained within their own threads
and only the _clean passing_ of these strings to other threads is problematic
(due to mechanisms such as reference counting).
If, however, one even uses string objects (e.g. as members
of shared structs/classes) between different threads in an unsafe way,
then one has another set of big problems to worry about first.
> Les
>
>
> Marcus Frenkel wrote:
> > As far as I know, SetString and wxCommandEvent is a thread safe way to
> > pass wxString from the secondary to the main thread, since it makes a
> > copy of the string.
> >
> > Marcus
Andreas Mohr
> One should note that all this is talking about a correct implementation
> case where string objects _are_ properly contained within their own
> threads and only the _clean passing_ of these strings to other threads is
> problematic (due to mechanisms such as reference counting).
Some arguments against COW:
<http://www.gotw.ca/publications/optimizations.htm>
<http://www.gotw.ca/gotw/045.htm>
It might be better to use two versions of the wxString library, with COW
only used for the single-threaded case.
KP> Some arguments against COW:
FWIW COW is not used (by default) any more in 2.9.0 as it uses (again, by
default, you can still enable our own version) std::string which doesn't
use COW in any recent implementations I know of.
> FWIW COW is not used (by default) any more in 2.9.0 as it uses (again, by
> default, you can still enable our own version) std::string which doesn't
> use COW in any recent implementations I know of.
Thanks, that's good to know.
I normally work with std::string converted to wxString in the
secondary threads, so I guess this is thread safe:
void FunctionInAWorkerThread(const std::string& str){
wxString newStr = wxString(str.c_str(), wxConvUTF8) ;
wxCommandEvent event(wxEVT_COMMAND_BUTTON_CLICKED,ID_FunctionInAMainThread);
event.SetString( newStr );
wxQueueEvent(frame, event);
}
---
Marcus
I wonder how much of a performance benefit there is to ref counting
strings anyway...
Les
On Mon, Nov 23, 2009 at 12:01:43PM +0000, Leslie Newell wrote:
>
> No. The problem is that strings are ref counted so m_str = str only
> copies a reference to the string, not the string itself. Use m_str =
> str.c_str() to create a copy of the string data. To be safe you should
> probably return(m_str.c_str()) in GetString() as well.
>
> I wonder how much of a performance benefit there is to ref counting
> strings anyway...
That's what the GotW URLs given in this thread thoroughly dealt with
(thanks to the one who brought them up, very useful!),
and the answer is: devastating performance "benefits" of COW mechanisms.
Andreas Mohr
F> Is there a way to disable COW in wxString of v2.8.10?
Using wxUSE_STL==1 build of wx could help as your std::string
implementation probably doesn't use COW. Otherwise no, not easily.