added wxQueueEvent() avoiding the bug of wxPostEvent() with the events having wxStrin...

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

diff --git a/docs/changes.txt b/docs/changes.txt
index 5a21ed1904398a2918b3abb23d305c59f1e18880..e7ce5694bf8835f8e691a3f27a7d1d4588fa465b 100644 (file)
--- a/docs/changes.txt
+++ b/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 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):
 
 
 All (Unix):
 

diff --git a/include/wx/event.h b/include/wx/event.h
index 7de2c88a81549ad344b081a2c5bd8e4ee8326dc1..e9b0220996ee968f8f6e8d18d9c232b95c96b6cd 100644 (file)
--- a/include/wx/event.h
+++ b/include/wx/event.h
@@ -439,7 +439,7 @@ public:
 
     wxCommandEvent(const wxCommandEvent& event)
         : wxEvent(event),
 
     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),
           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; }
 
     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
     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);
 
     // wouldn't correctly propagate to wxEventLoop.
     bool SafelyProcessEvent(wxEvent& event);
 

-    // add an event to be processed later
-    virtual void AddPendingEvent(const wxEvent& event);
+    // Schedule the given event to be processed later. It takes ownership of
+    // 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();
 
 
     void ProcessPendingEvents();
 


@@ -2491,15 +2510,27 @@ private:
 };
 #endif // wxUSE_WEAKREF
 
 };
 #endif // wxUSE_WEAKREF
 

-// Post a message to the given eventhandler which will be processed during the
-// next event loop iteration
+// Post a message to the given event handler which will be processed during the
+// next event loop iteration.
+//
+// Notice that this one is not thread-safe, use wxQueueEvent()

 inline void wxPostEvent(wxEvtHandler *dest, const wxEvent& event)
 {
 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);
 }
 
 
     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&);
 
 typedef void (wxEvtHandler::*wxEventFunction)(wxEvent&);
 typedef void (wxEvtHandler::*wxIdleEventFunction)(wxIdleEvent&);
 

diff --git a/interface/event.h b/interface/event.h
index 81ff87735c0d112dacd9d8ca92ec3d40af551ba2..1b7468948811e76d40dd63513d9760c06e11fcdb 100644 (file)
--- a/interface/event.h
+++ b/interface/event.h
@@ -42,8 +42,9 @@ public:
     /**
         Returns a copy of the event.
 
     /**
         Returns a copy of the event.
 

-        Any event that is posted to the wxWidgets event system for later action (via
-        wxEvtHandler::AddPendingEvent or wxPostEvent()) must implement this method.
+        Any event that is posted to the wxWidgets event system for later action
+        (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
 
         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();
 
     /**
     virtual ~wxEvtHandler();
 
     /**

-        This function posts an event to be processed later.
+        Queue event for a later processing.
+
+        This method is similar to ProcessEvent() but while the latter is
+        synchronous, i.e. the event is processed immediately, before the
+        function returns, this one is asynchronous and returns immediately
+        while the event will be processed at some later time (usually during
+        the next event loop iteration).
+
+        Another important difference is that this method takes ownership of the
+        @a event parameter, i.e. it will delete it itself. This implies that
+        the event should be allocated on the heap and that the pointer can't be
+        used any more after the function returns (as it can be deleted at any
+        moment).
+
+        QueueEvent() can be used for inter-thread communication from the worker
+        threads to the main thread, it is safe in the sense that it uses
+        locking internally and avoids the problem mentioned in AddPendingEvent()
+        documentation by ensuring that the @a event object is not used by the
+        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;

 
 

-        The difference between sending an event (using the ProcessEvent
-        method) and posting it is that in the first case the event is
-        processed before the function returns, while in the second case,
-        the function returns immediately and the event will be processed
-        sometime later (usually during the next event loop iteration).
+                // NOT e->SetString(str) as this would be a shallow copy
+                e->SetString(str.c_str()); // make a deep copy

 
 

-        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.
+                wxTheApp->QueueEvent(new wxCommandEvent
+            }
+        @endcode

 
 

-        This is also the method to call for inter-thread communication - it will post
-        events safely between different threads which means that this method is
-        thread-safe by using critical sections where needed. In a multi-threaded program,
-        you often need to inform the main GUI thread about the status of other working
-        threads and such notification should be done using this method.
+        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.

 
 

-        This method automatically wakes up idle handling if the underlying window
-        system is currently idle and thus would not send any idle events.
-        (Waking up idle handling is done calling ::wxWakeUpIdle.)
+        @since 2.9.0

 
         @param event
 
         @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);
 
     */
     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
 
     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}
 */
 
     @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);

 
 //@}
 
 
 //@}
 

diff --git a/src/common/event.cpp b/src/common/event.cpp
index 817f40e4a896829f7105c8bd96c072d1d68a58f2..d2bd83d29b477bd84f3a689a656b163728f96e71 100644 (file)
--- a/src/common/event.cpp
+++ b/src/common/event.cpp
@@ -1130,23 +1130,17 @@ bool wxEvtHandler::ProcessThreadEvent(const wxEvent& event)
 
 #endif // wxUSE_THREADS
 
 
 #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
-
-    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") );
+    wxCHECK_RET( event, "NULL event can't be posted" );

 
 

+    // 1) Add this event to our list of pending events

     wxENTER_CRIT_SECT( m_pendingEventsLock );
 
     if ( !m_pendingEvents )
       m_pendingEvents = new wxList;
 
     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 );
 
 
     wxLEAVE_CRIT_SECT( m_pendingEventsLock );