Developer forums (C::B DEVELOPMENT STRICTLY!) > Development

wx2.8.12, sending event cross threads are not safe in current CB source

(1/5) > >>

ollydbg:
There are two functions which can send events from a worker thread to main GUI.
wxPostEvent() and AddPendingEvent()

According to the discussions below:

* wx forum post: thread safety issue about wxCommandEvent in wx2.8.12
* our CB forum post: unnecessory locker in CClogger

I think there are many issues in our C::B source:

wxCommandEvent is not thread safe for its m_cmdString member, e.g.:

--- Code: ---           
            wxString message;
            wxCommandEvent evt(...);
            evt.SetString(message.c_str());
            wxPostEvent(m_Parent, evt);

--- End code ---
The code snippet is not thread safe, because "evt" and the cloned wxCommandEvent shared the same wxString member buffer.

CodeBlocksEvent is not thread safe, e.g. in cbthreadpool.cpp

--- Code: ---      CodeBlocksEvent evt = CodeBlocksEvent(cbEVT_THREADTASK_ALLDONE, m_ID);
      wxPostEvent(m_pOwner, evt);

--- End code ---
CodeBlocksEvent is a derived class from wxCommandEvent, and I don't see it Clone its m_strString.

3, I'm not sure PipedProcess class has such issue, because its related to send event cross processes.

Solution:
If the event need to carry wxString from worker thread to main GUI thread, we should implement a Clone method which deep copy its m_cmdString. Just like the new event type wxThreadEvent in wx 2.9.x.

Any comments.

MortenMacFly:

--- Quote from: ollydbg on June 21, 2013, 02:32:41 am ---Solution:
If the event need to carry wxString from worker thread to main GUI thread, we should implement a Clone method which deep copy its m_cmdString. Just like the new event type wxThreadEvent in wx 2.9.x.

--- End quote ---
Yes, we should always do. As a matter of fact, for wx events its a good thing to always implement the clone method once you are using member vars.

ollydbg:
Ok, this is the first draft patch. I add a new class named cbThreadEvent, which replace all the cross thread sending message from wxCommandEvent to cbThreadEvent. The patch also included an improved hook measure patch from: Performance measurement of all C::B event handlers

MortenMacFly:

--- Quote from: ollydbg on June 21, 2013, 09:34:31 am ---Ok, this is the first draft patch.

--- End quote ---
Could you do a proper SVN based patch? This patch does not apply.

golgepapaz:
This ,

--- Quote from: MortenMacFly on June 21, 2013, 01:13:34 pm ---Could you do a proper SVN based patch? This patch does not apply.

--- End quote ---

Won't it be proper that defining the event in sdk_events.h and sdk_events.cpp instead of
cclogger.cpp and it's own header?

Also about that profiling stuff, I don't want to pay for something that I don't need. This should be
optional.

Navigation

[0] Message Index

[#] Next page