wxString objects are not Thread Safe
wxString objects are not Thread Safe
I really like wxStrings. They provide a lot more functionality over std::string objects. We tried to use wsStrings in situations where multiple threads made read access to a single wxString. The result was catastrophe. Apparently the wxString keeps a local copy of a multibyte version of the string that would get overridden by the multiple threads.
Is there any thought to making wxString objects more thread safe?
Is there any thought to making wxString objects more thread safe?
The Max
-
- I live to help wx-kind
- Posts: 196
- Joined: Tue Dec 07, 2004 8:54 pm
- Location: Essen, Germany
wxWidgets has a number of synchronization primitives like wxMutex, wxCriticalSection, wxCondition. Use them to organize an access to the string. It can looks like
Code: Select all
static wxCriticalSection MyClass::m_csStringAccess;
...
void MyClass::WriteString(const wxString &str) {
wxCriticalSectionLocker lock(m_csStringAccess);
m_strValue = str.c_str(); // necessary for memory copy instead of reference counting
}
This is essentially what we had to do. The fact that we had to do it was a big blow to the desire of several of our engineers in using wxWidgets. In fact a lot of the development using wxWidgets was stopped because of this. It took weeks to undo lots of development.
It would be really nice if the read accessors of a wxString handled the syncronization on thier own. I don't mind having to do syncronization on writes, but the object could be smart on its own related to reads.
It would be really nice if the read accessors of a wxString handled the syncronization on thier own. I don't mind having to do syncronization on writes, but the object could be smart on its own related to reads.
The Max
-
- I live to help wx-kind
- Posts: 196
- Joined: Tue Dec 07, 2004 8:54 pm
- Location: Essen, Germany
How do you imagine a read synchronization for the following codeI think it should be something like thatBut this code has a lack. The GetAccessCS cannot be a const method and is not be able to be used everythere. So I think is easier to organize thread safe access out of a string class.
Code: Select all
...
cont wxChar *ptr = str.c_str();
....
printf(_T("%s\n"), ptr);
...
Code: Select all
...
{
wxCriticalSectionLocker lock(str.GetAccessCS());
cont wxChar *ptr = str.c_str();
....
printf(_T("%s\n"), ptr);
}
...
Actually I just want to have the following.
printf(_T("%s\n"), str.c_str());
The c_str() member is essentially a read accessor. It doesn't change the essential contents of the wxString object. (I.E. The value of of str should be the same before and after the call). Why should I have to syncronize to use it?
printf(_T("%s\n"), str.c_str());
The c_str() member is essentially a read accessor. It doesn't change the essential contents of the wxString object. (I.E. The value of of str should be the same before and after the call). Why should I have to syncronize to use it?
The Max
-
- I live to help wx-kind
- Posts: 196
- Joined: Tue Dec 07, 2004 8:54 pm
- Location: Essen, Germany
Ok. For instance, c_str() has access protection like thismaxderhak wrote:Actually I just want to have the following.
printf(_T("%s\n"), str.c_str());
The c_str() member is essentially a read accessor. It doesn't change the essential contents of the wxString object. (I.E. The value of of str should be the same before and after the call). Why should I have to syncronize to use it?
Code: Select all
class wxString {
...
mutable wxCriticalSection m_csAccess;
...
};
...
const wxChar *wxString::c_str() const {
wxCriticalSectionLocker lock(m_csAccess);
...
return m_pData;
}
There is a dummy code
Code: Select all
cont wxChar *ptr = str.c_str();
wxMilliSleep(100);
printf(_T("%s\n"), ptr);
-
- Part Of The Furniture
- Posts: 1511
- Joined: Sun Jun 19, 2005 11:37 am
- Location: Singapore
- Contact:
Read the STL documentation for c_str(). http://dinkumware.com/manuals/?manual=c ... ing::c_str
Joel
You should NEVER assume that the pointer returned by c_str() will remain constant even if no modifications are made (the VC STL is strict about this.). Though the Dinkum STL states that, actually it is common knowledge through experience that c_str() pointers are very dangerous and should be used atomically (e.g by immediately strcpy'ing it out)The member function returns a pointer to a non-modifiable C string constructed by adding a terminating null element (value_type()) to the controlled sequence. Calling any non-const member function for *this can invalidate the pointer.
Joel
No! This is not the case. Thread A initializes a string. Then threads B, C, D & E all potentially make calls to get the value of the string by calling c_str(). Once the string was initialized it is not modified again. It is just accessed. The problem is that the internal implementatin of wxString's c_str() causes a write to an internal private member to get the return value. I would like it if the syncronization to intialize and get the return value were put into the wxString object itself, so that outside users of the wxString wouldn't have to work around the internal implementation details of an operation that is essentially a read operation. Why should I syncronize when I know that the string is not being modified by another thread. The internal modification of prvate data that doesn't essentially change the state of the object is the problem.lowjoel wrote:Because as thread A reads from the string using c_str(), thread B may be writing to the string. Granted c_str is a const-member function, thread B changed the data as A is reading from it, which basically will result in undefined behaviour.
Joel
The Max
-
- Part Of The Furniture
- Posts: 1511
- Joined: Sun Jun 19, 2005 11:37 am
- Location: Singapore
- Contact:
Hm. Then it shows a flaw in wxString's implementation. But I doubt that's possible as c_str is a const member function, and I doubt there are mutable member variables.
Then again, you may not be using wx's wxString, as you may have defined wxUSE_STL to 1 and thus use std::string as the "main" string class and using wxString as a wrapper.
Joel
Then again, you may not be using wx's wxString, as you may have defined wxUSE_STL to 1 and thus use std::string as the "main" string class and using wxString as a wrapper.
Joel
We were using wx's wxString.lowjoel wrote:Hm. Then it shows a flaw in wxString's implementation. But I doubt that's possible as c_str is a const member function, and I doubt there are mutable member variables.
Then again, you may not be using wx's wxString, as you may have defined wxUSE_STL to 1 and thus use std::string as the "main" string class and using wxString as a wrapper.
Joel
The Max
-
- I live to help wx-kind
- Posts: 196
- Joined: Tue Dec 07, 2004 8:54 pm
- Location: Essen, Germany
c_str does not modify any internal variables
Code: Select all
wxChar *m_pchData;
...
const wxChar* c_str() const { return m_pchData; }
Your right. The problem is with mb_str() or casting the wxString to (const char*). The scenario is the same.megabyte wrote:c_str does not modify any internal variablesCode: Select all
wxChar *m_pchData; ... const wxChar* c_str() const { return m_pchData; }
The Max
-
- I live to help wx-kind
- Posts: 196
- Joined: Tue Dec 07, 2004 8:54 pm
- Location: Essen, Germany
mb_str does not modify wxString. It creates an instance of the wxCharBuffer class. Also wxString has no operator const char *(), but only const wxChar *() which is similar to c_str(). The wxCharBuffer class has the const char *() operator.maxderhak wrote:Your right. The problem is with mb_str() or casting the wxString to (const char*). The scenario is the same.megabyte wrote:c_str does not modify any internal variablesCode: Select all
wxChar *m_pchData; ... const wxChar* c_str() const { return m_pchData; }
Do you really need to convert wxString to const char *?
Edit: mb_str returns a local copy of wxCharBuffer, so I think the readers threads cannot create a lot of problems in this place.
Edit 2: Try to work not as
Code: Select all
const char *ptr = (const char*)str.mb_str();
Code: Select all
wxCharBuffer buf(1);
buf = str.mb_str();
const char *ptr = (const char *)buf;