added wxQueueEvent() avoiding the bug of wxPostEvent() with the events having wxString fields

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@53405 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

docs/changes.txt

@@ -237,6 +237,8 @@ All:
 - Added wxWeakRef<T>, wxScopedPtr<T>, wxSharedPtr<T> class templates
 - Added wxVector<T> class templates
 - Added wxON_BLOCK_EXIT_SET() and wxON_BLOCK_EXIT_NULL() to wx/scopeguard.h.
+- Added wxEvtHandler::QueueEvent() replacing AddPendingEvent() and
+  wxQueueEvent() replacing wxPostEvent().
 
 All (Unix):
 

include/wx/event.h

@@ -439,7 +439,7 @@ public:
 
     wxCommandEvent(const wxCommandEvent& event)
         : wxEvent(event),
-          m_cmdString(event.m_cmdString.c_str()), // "thread-safe"
+          m_cmdString(event.m_cmdString),
           m_commandInt(event.m_commandInt),
           m_extraLong(event.m_extraLong),
           m_clientData(event.m_clientData),
@@ -2276,7 +2276,9 @@ public:
     void SetEvtHandlerEnabled(bool enabled) { m_enabled = enabled; }
     bool GetEvtHandlerEnabled() const { return m_enabled; }
 
-    // process an event right now
+    // Process an event right now: this can only be called from the main
+    // thread, use QueueEvent() for scheduling the events for
+    // processing from other threads.
     virtual bool ProcessEvent(wxEvent& event);
 
     // Process an event by calling ProcessEvent and handling any exceptions
@@ -2285,8 +2287,25 @@ public:
     // wouldn't correctly propagate to wxEventLoop.
     bool SafelyProcessEvent(wxEvent& event);
 
-    // add an event to be processed later
+    // Schedule the given event to be processed later. It takes ownership of
-    virtual void AddPendingEvent(const wxEvent& event);
+    // the event pointer, i.e. it will be deleted later. This is safe to call
+    // from multiple threads although you still need to ensure that wxString
+    // fields of the event object are deep copies and not use the same string
+    // buffer as other wxString objects in this thread.
+    virtual void QueueEvent(wxEvent *event);
+
+    // Add an event to be processed later: notice that this function is not
+    // safe to call from threads other than main, use QueueEvent()
+    virtual void AddPendingEvent(const wxEvent& event)
+    {
+        // notice that the thread-safety problem comes from the fact that
+        // Clone() doesn't make deep copies of wxString fields of wxEvent
+        // object and so the same wxString could be used from both threads when
+        // the event object is destroyed in this one -- QueueEvent() avoids
+        // this problem as the event pointer is not used any more in this
+        // thread at all after it is called.
+        QueueEvent(event.Clone());
+    }
 
     void ProcessPendingEvents();
 
@@ -2491,15 +2510,27 @@ private:
 };
 #endif // wxUSE_WEAKREF
 
-// Post a message to the given eventhandler which will be processed during the
+// Post a message to the given event handler which will be processed during the
-// next event loop iteration
+// next event loop iteration.
+//
+// Notice that this one is not thread-safe, use wxQueueEvent()
 inline void wxPostEvent(wxEvtHandler *dest, const wxEvent& event)
 {
-    wxCHECK_RET( dest, wxT("need an object to post event to in wxPostEvent") );
+    wxCHECK_RET( dest, "need an object to post event to" );
 
     dest->AddPendingEvent(event);
 }
 
+// Wrapper around wxEvtHandler::QueueEvent(): adds an event for later
+// processing, unlike wxPostEvent it is safe to use from different thread even
+// for events with wxString members
+inline void wxQueueEvent(wxEvtHandler *dest, wxEvent *event)
+{
+    wxCHECK_RET( dest, "need an object to queue event for" );
+
+    dest->QueueEvent(event);
+}
+
 typedef void (wxEvtHandler::*wxEventFunction)(wxEvent&);
 typedef void (wxEvtHandler::*wxIdleEventFunction)(wxIdleEvent&);
 

interface/event.h

@@ -42,8 +42,9 @@ public:
     /**
         Returns a copy of the event.
 
-        Any event that is posted to the wxWidgets event system for later action (via
+        Any event that is posted to the wxWidgets event system for later action
-        wxEvtHandler::AddPendingEvent or wxPostEvent()) must implement this method.
+        (via wxEvtHandler::AddPendingEvent or wxPostEvent()) must implement
+        this method.
 
         All wxWidgets events fully implement this method, but any derived events
         implemented by the user should also implement this method just in case they
@@ -265,31 +266,72 @@ public:
     virtual ~wxEvtHandler();
 
     /**
-        This function posts an event to be processed later.
+        Queue event for a later processing.
 
-        The difference between sending an event (using the ProcessEvent
+        This method is similar to ProcessEvent() but while the latter is
-        method) and posting it is that in the first case the event is
+        synchronous, i.e. the event is processed immediately, before the
-        processed before the function returns, while in the second case,
+        function returns, this one is asynchronous and returns immediately
-        the function returns immediately and the event will be processed
+        while the event will be processed at some later time (usually during
-        sometime later (usually during the next event loop iteration).
+        the next event loop iteration).
 
-        A copy of event is made by the function, so the original can be deleted as
+        Another important difference is that this method takes ownership of the
-        soon as function returns (it is common that the original is created on the
+        @a event parameter, i.e. it will delete it itself. This implies that
-        stack). This requires that the wxEvent::Clone method be implemented by event
+        the event should be allocated on the heap and that the pointer can't be
-        so that it can be duplicated and stored until it gets processed.
+        used any more after the function returns (as it can be deleted at any
+        moment).
 
-        This is also the method to call for inter-thread communication - it will post
+        QueueEvent() can be used for inter-thread communication from the worker
-        events safely between different threads which means that this method is
+        threads to the main thread, it is safe in the sense that it uses
-        thread-safe by using critical sections where needed. In a multi-threaded program,
+        locking internally and avoids the problem mentioned in AddPendingEvent()
-        you often need to inform the main GUI thread about the status of other working
+        documentation by ensuring that the @a event object is not used by the
-        threads and such notification should be done using this method.
+        calling thread any more. Care should still be taken to avoid that some
+        fields of this object are used by it, notably any wxString members of
+        the event object must not be shallow copies of another wxString object
+        as this would result in them still using the same string buffer behind
+        the scenes. For example
+        @code
+            void FunctionInAWorkerThread(const wxString& str)
+            {
+                wxCommandEvent * const e = new wxCommandEvent;
 
-        This method automatically wakes up idle handling if the underlying window
+                // NOT e->SetString(str) as this would be a shallow copy
-        system is currently idle and thus would not send any idle events.
+                e->SetString(str.c_str()); // make a deep copy
-        (Waking up idle handling is done calling ::wxWakeUpIdle.)
+
+                wxTheApp->QueueEvent(new wxCommandEvent
+            }
+        @endcode
+
+        Finally notice that this method automatically wakes up the event loop
+        if it is currently idle by calling ::wxWakeUpIdle() so there is no need
+        to do it manually when using it.
+
+        @since 2.9.0
 
         @param event
-            Event to add to process queue.
+            A heap-allocated event to be queued, QueueEvent() takes ownership
+            of it. This parameter shouldn't be @c NULL.
+     */
+    virtual void QueueEvent(wxEvent *event);
+
+    /**
+        Post an event to be processed later.
+
+        This function is similar to QueueEvent() but can't be used to post
+        events from worker threads for the event objects with wxString fields
+        (i.e. in practice most of them) because of an unsafe use of the same
+        wxString object which happens because the wxString field in the
+        original @a event object and its copy made internally by this function
+        share the same string buffer internally. Use QueueEvent() to avoid
+        this. 
+
+        A copy of event is made by the function, so the original can be deleted
+        as soon as function returns (it is common that the original is created
+        on the stack). This requires that the wxEvent::Clone() method be
+        implemented by event so that it can be duplicated and stored until it
+        gets processed.
+
+        @param event
+            Event to add to the pending events queue.
     */
     virtual void AddPendingEvent(const wxEvent& event);
 
@@ -3264,11 +3306,29 @@ public:
 
     Otherwise, it dispatches @a event immediately using
     wxEvtHandler::ProcessEvent(). See the respective documentation for details
-    (and caveats).
+    (and caveats). Because of limitation of wxEvtHandler::AddPendingEvent()
+    this function is not thread-safe for event objects having wxString fields,
+    use wxQueueEvent() instead.
 
     @header{wx/event.h}
 */
-void wxPostEvent(wxEvtHandler* dest, wxEvent& event);
+void wxPostEvent(wxEvtHandler* dest, const wxEvent& event);
+
+/**
+    Queue an event for processing on the given object.
+
+    This is a wrapper around wxEvtHandler::QueueEvent(), see its documentation
+    for more details.
+
+    @header{wx/event.h}
+
+    @param dest
+        The object to queue the event on, can't be @c NULL.
+    @param event
+        The heap-allocated and non-@c NULL event to queue, the function takes
+        ownership of it.
+ */
+void wxQueueEvent(wxEvtHandler* dest, wxEvent *event);
 
 //@}
 

src/common/event.cpp

@@ -1130,23 +1130,17 @@ bool wxEvtHandler::ProcessThreadEvent(const wxEvent& event)
 
 #endif // wxUSE_THREADS
 
-void wxEvtHandler::AddPendingEvent(const wxEvent& event)
+void wxEvtHandler::QueueEvent(wxEvent *event)
 {
-    // 1) Add event to list of pending events of this event handler
+    wxCHECK_RET( event, "NULL event can't be posted" );
-
-    wxEvent *eventCopy = event.Clone();
-
-    // we must be able to copy the events here so the event class must
-    // implement Clone() properly instead of just providing a NULL stab for it
-    wxCHECK_RET( eventCopy,
-                 _T("events of this type aren't supposed to be posted") );
 
+    // 1) Add this event to our list of pending events
     wxENTER_CRIT_SECT( m_pendingEventsLock );
 
     if ( !m_pendingEvents )
       m_pendingEvents = new wxList;
 
-    m_pendingEvents->Append(eventCopy);
+    m_pendingEvents->Append(event);
 
     wxLEAVE_CRIT_SECT( m_pendingEventsLock );