Hi, when reading the code:
void CCLogger::AddToken(const wxString& msg)
{
wxMutexLocker l(m_CCLoggerMutex);
if (!m_Parent || m_AddTokenId<1) return;
wxCommandEvent evt(wxEVT_COMMAND_MENU_SELECTED, m_AddTokenId);
evt.SetString(msg);
#if CC_PROCESS_LOG_EVENT_TO_PARENT
m_Parent->ProcessEvent(evt);
#else
wxPostEvent(m_Parent, evt);
#endif
}
void CCLogger::Log(const wxString& msg)
{
wxMutexLocker l(m_CCLoggerMutex);
if (!m_Parent || m_LogId<1) return;
wxCommandEvent evt(wxEVT_COMMAND_MENU_SELECTED, m_LogId);
evt.SetString(msg);
#if CC_PROCESS_LOG_EVENT_TO_PARENT
m_Parent->ProcessEvent(evt);
#else
wxPostEvent(m_Parent, evt);
#endif
}
void CCLogger::DebugLog(const wxString& msg)
{
wxMutexLocker l(m_CCLoggerMutex);
if (!m_Parent || m_DebugLogId<1) return;
wxCommandEvent evt(wxEVT_COMMAND_MENU_SELECTED, m_DebugLogId);
evt.SetString(msg);
#if CC_PROCESS_LOG_EVENT_TO_PARENT
m_Parent->ProcessEvent(evt);
#else
wxPostEvent(m_Parent, evt);
#endif
}
Here, in the case of CC_PROCESS_LOG_EVENT_TO_PARENT is 0.
Refer to the link below, I think no wxMutexLocker should be used, because wxPostEvent is safe to send an event from a working thread to main GUI thread.
http://wiki.wxwidgets.org/Custom_Events#wxEvtHandler::ProcessEvent_vs._wxPostEvent_and_wxEvtHandler::AddPendingEvent
Any ideas/comments ?
Hi, dmoore, you are definitely right. I just debug a simple wx multiply thread app which link to the debug wx 2.8.12 library , so I can step into all the wx sources. I see that "wxPostEvent(m_Parent, evt);" will clone a new event:
virtual wxEvent *Clone() const { return new wxCommandEvent(*this); }
wxCommandEvent(const wxCommandEvent& event)
: wxEvent(event),
#if WXWIN_COMPATIBILITY_2_4
m_commandString(this),
#endif
m_cmdString(event.m_cmdString),
m_commandInt(event.m_commandInt),
m_extraLong(event.m_extraLong),
m_clientData(event.m_clientData),
m_clientObject(event.m_clientObject)
{ }
But the bad thing is here:
m_cmdString(event.m_cmdString)
A wxString is construct from another wxString, so reference counting is used. Too bad, wxString's reference counting is not atomic variables (see my post several months ago wxString and the locker issue in our CC code (http://forums.codeblocks.org/index.php/topic,17543.msg120220.html#msg120220))
Here, in the code:
void CCLogger::AddToken(const wxString& msg)
{
wxMutexLocker l(m_CCLoggerMutex);
if (!m_Parent || m_AddTokenId<1) return;
wxCommandEvent evt(wxEVT_COMMAND_MENU_SELECTED, m_AddTokenId);
evt.SetString(msg);
#if CC_PROCESS_LOG_EVENT_TO_PARENT
m_Parent->ProcessEvent(evt);
#else
wxPostEvent(m_Parent, evt);
#endif
}
I think the wxMutexLocker can NOT solve the thread-safe issue of wxString, as you said, here we must do a deep copy of wxString by ourselves, the only way use: evt.SetString(msg.c_str()); (See this post as in wx forum as a reference (http://forums.wxwidgets.org/viewtopic.php?p=125548#p125548)
EDIT:
wx since 2.9.0 has a nice event type: wxThreadEvent, which did a deep copy of every thing (in-fact, it did a deep copy of it's wxString)
see event.h in wx 2.9.0, there is also a "SetString(GetString().c_str());"
// Thread event
class WXDLLIMPEXP_BASE wxThreadEvent : public wxEvent,
public wxEventAnyPayloadMixin
{
public:
wxThreadEvent(wxEventType eventType = wxEVT_THREAD, int id = wxID_ANY)
: wxEvent(id, eventType)
{ }
wxThreadEvent(const wxThreadEvent& event)
: wxEvent(event),
wxEventAnyPayloadMixin(event)
{
// make sure our string member (which uses COW, aka refcounting) is not
// shared by other wxString instances:
SetString(GetString().c_str());
}
virtual wxEvent *Clone() const
{
return new wxThreadEvent(*this);
}
// this is important to avoid that calling wxEventLoopBase::YieldFor thread events
// gets processed when this is unwanted:
virtual wxEventCategory GetEventCategory() const
{ return wxEVT_CATEGORY_THREAD; }
private:
DECLARE_DYNAMIC_CLASS_NO_ASSIGN(wxThreadEvent)
};
EDIT2:
In wx2.8.12, when build with GCC, the wxString is COW(copy on write, and reference counted), but the reference counter is not thread-safe(it does not use an atomic counter).
In wx2.9.x, when wxString build with GCC use GCC's native std::string, which is also COW, and it uses the atomic reference counter, so it is safe to use "evt.SetString(msg);".
But COW with atomic reference counter has performance issue(reported by Herb Sutter in his article Optimizations That Aren't (In a Multithreaded World) (http://www.gotw.ca/publications/optimizations.htm)), so I see somewhere that GCC 5.0 or GCC 4.9 in the future may totally remove the COW feature of std::string. I once hear that Visual C++'s std::string is already NOT COW for years.