process one event at once in wxEvtHandler::ProcessPendingEvents() to prevent crashes when a (pending) event handler destroys the event handler object itself; only add the event handler to wxPendingEvents list if it's not already there (and explicitly mention that an object can be present in this list only once in the comment) (replaces patch 1837719)

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

include/wx/event.h

@@ -3008,8 +3008,9 @@ typedef void (wxEvtHandler::*wxClipboardTextEventFunction)(wxClipboardTextEvent&
 // Global data
 // ----------------------------------------------------------------------------
 
-// for pending event processing - notice that there is intentionally no
+// list containing event handlers with pending events for them
-// WXDLLEXPORT here
+//
+// notice that each event handler should occur at most once in this list
 extern WXDLLIMPEXP_BASE wxList *wxPendingEvents;
 #if wxUSE_THREADS
     extern WXDLLIMPEXP_BASE wxCriticalSection *wxPendingEventsLocker;

src/common/event.cpp

@@ -136,12 +136,13 @@ IMPLEMENT_DYNAMIC_CLASS(wxEventTableEntryModule, wxModule)
 // global variables
 // ----------------------------------------------------------------------------
 
-// To put pending event handlers
+// List containing event handlers with pending events (each handler can occur
-wxList *wxPendingEvents = (wxList *)NULL;
+// at most once here)
+wxList *wxPendingEvents = NULL;
 
 #if wxUSE_THREADS
     // protects wxPendingEvents list
-    wxCriticalSection *wxPendingEventsLocker = (wxCriticalSection *)NULL;
+    wxCriticalSection *wxPendingEventsLocker = NULL;
 #endif
 
 // common event types are defined here, other event types are defined by the
@@ -1067,22 +1068,27 @@ wxEvtHandler::~wxEvtHandler()
         m_pendingEvents->DeleteContents(true);
     delete m_pendingEvents;
 
-#if wxUSE_THREADS
 #  if !defined(__VISAGECPP__)
     delete m_eventsLocker;
 #  endif
 
     // Remove us from wxPendingEvents if necessary.
-    if(wxPendingEventsLocker)
-        wxENTER_CRIT_SECT(*wxPendingEventsLocker);
     if ( wxPendingEvents )
     {
-        // Delete all occurences of this from the list of pending events
+        if(wxPendingEventsLocker)
-        while (wxPendingEvents->DeleteObject(this)) { } // Do nothing
+            wxENTER_CRIT_SECT(*wxPendingEventsLocker);
+
+        if ( wxPendingEvents->DeleteObject(this) )
+        {
+            // check that we were present only once in the list
+            wxASSERT_MSG( !wxPendingEvents->Find(this),
+                          "Handler occurs twice in wxPendingEvents list" );
         }
+        //else: we weren't in this list at all, it's ok
+
         if(wxPendingEventsLocker)
             wxLEAVE_CRIT_SECT(*wxPendingEventsLocker);
-#endif
+    }
 
     // we only delete object data, not untyped
     if ( m_clientDataType == wxClientData_Object )
@@ -1139,6 +1145,7 @@ void wxEvtHandler::AddPendingEvent(const wxEvent& event)
 
     if ( !wxPendingEvents )
         wxPendingEvents = new wxList;
+    if ( !wxPendingEvents->Find(this) )
         wxPendingEvents->Append(this);
 
     wxLEAVE_CRIT_SECT(*wxPendingEventsLocker);
@@ -1150,43 +1157,35 @@ void wxEvtHandler::AddPendingEvent(const wxEvent& event)
 
 void wxEvtHandler::ProcessPendingEvents()
 {
-    // this method is only called by wxApp if this handler does have
-    // pending events
-    wxCHECK_RET( m_pendingEvents,
-                 wxT("Please call wxApp::ProcessPendingEvents() instead") );
-
     wxENTER_CRIT_SECT( Lock() );
 
-    // we leave the loop once we have processed all events that were present at
+    // this method is only called by wxApp if this handler does have
-    // the start of ProcessPendingEvents because otherwise we could get into
+    // pending events
-    // infinite loop if the pending event handler execution resulted in another
+    wxCHECK_RET( m_pendingEvents && !m_pendingEvents->IsEmpty(),
-    // event being posted
+                 "should have pending events if called" );
-    size_t n = m_pendingEvents->size();
-    for ( wxList::compatibility_iterator node = m_pendingEvents->GetFirst();
-          node;
-          node = m_pendingEvents->GetFirst() )
-    {
-        wxEvent *event = (wxEvent *)node->GetData();
 
-        // It's importan we remove event from list before processing it.
+    wxList::compatibility_iterator node = m_pendingEvents->GetFirst();
-        // Else a nested event loop, for example from a modal dialog, might
+    wxEvent * const event = wx_static_cast(wxEvent *, node->GetData());
-        // process the same event again.
 
+    // it's important we remove event from list before processing it, else a
+    // nested event loop, for example from a modal dialog, might process the
+    // same event again.
     m_pendingEvents->Erase(node);
 
+    // if there are no more pending events left, we don't need to stay in this
+    // list
+    if ( m_pendingEvents->IsEmpty() )
+        wxPendingEvents->DeleteObject(this);
+
     wxLEAVE_CRIT_SECT( Lock() );
 
     ProcessEvent(*event);
 
+    // careful: this object could have been deleted by the event handler
+    // executed by the above ProcessEvent() call, so we can't access any fields
+    // of this object any more
+
     delete event;
-
-        wxENTER_CRIT_SECT( Lock() );
-
-        if ( --n == 0 )
-            break;
-    }
-
-    wxLEAVE_CRIT_SECT( Lock() );
 }
 
 /*