Fix crashes after using "wildcard" wxEvtHandler::Disconnect().

When not specifying the function to disconnect, the associated event sink was
destroyed too early resulting in crashes later. Fix this and add unit tests
verifying that things work as expected and at least don't crash.

Closes #14563.

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

diff --git a/src/common/event.cpp b/src/common/event.cpp
index 5db57446394e0ac17f4a3d7c3be6be23535a6efb..4bc798968278b10caf853d2b617959b682951804 100644 (file)
--- a/src/common/event.cpp
+++ b/src/common/event.cpp
@@ -1675,15 +1675,6 @@ wxEvtHandler::DoUnbind(int id,
     if (!m_dynamicEvents)
         return false;
 
-    // Remove connection from tracker node (wxEventConnectionRef)
-    wxEvtHandler *eventSink = func.GetEvtHandler();
-    if ( eventSink && eventSink != this )
-    {
-        wxEventConnectionRef *evtConnRef = FindRefInTrackerList(eventSink);
-        if ( evtConnRef )
-            evtConnRef->DecRef();
-    }
-
     wxList::compatibility_iterator node = m_dynamicEvents->GetFirst();
     while (node)
     {
@@ -1695,6 +1686,15 @@ wxEvtHandler::DoUnbind(int id,
             entry->m_fn->IsMatching(func) &&
             ((entry->m_callbackUserData == userData) || !userData))
         {
+            // Remove connection from tracker node (wxEventConnectionRef)
+            wxEvtHandler *eventSink = entry->m_fn->GetEvtHandler();
+            if ( eventSink && eventSink != this )
+            {
+                wxEventConnectionRef *evtConnRef = FindRefInTrackerList(eventSink);
+                if ( evtConnRef )
+                    evtConnRef->DecRef();
+            }
+
             delete entry->m_callbackUserData;
             m_dynamicEvents->Erase( node );
             delete entry;

diff --git a/tests/events/evthandler.cpp b/tests/events/evthandler.cpp
index 2e5d478c7d5735ae5c6edeb8e8f68c9b9ec1e5aa..d8e24a3438bb916891c93db07686c7916e979f83 100644 (file)
--- a/tests/events/evthandler.cpp
+++ b/tests/events/evthandler.cpp
@@ -160,6 +160,8 @@ private:
     CPPUNIT_TEST_SUITE( EvtHandlerTestCase );
         CPPUNIT_TEST( BuiltinConnect );
         CPPUNIT_TEST( LegacyConnect );
+        CPPUNIT_TEST( DisconnectWildcard );
+        CPPUNIT_TEST( AutoDisconnect );
 #ifdef wxHAS_EVENT_BIND
         CPPUNIT_TEST( BindFunction );
         CPPUNIT_TEST( BindStaticMethod );
@@ -174,6 +176,8 @@ private:
 
     void BuiltinConnect();
     void LegacyConnect();
+    void DisconnectWildcard();
+    void AutoDisconnect();
 #ifdef wxHAS_EVENT_BIND
     void BindFunction();
     void BindStaticMethod();
@@ -249,6 +253,31 @@ void EvtHandlerTestCase::LegacyConnect()
     handler.Disconnect( 0, 0, LegacyEventType, (wxObjectEventFunction)&MyHandler::OnEvent, NULL, &handler );
 }
 
+void EvtHandlerTestCase::DisconnectWildcard()
+{
+    // should be able to disconnect a different handler using "wildcard search"
+    MyHandler sink;
+    wxEvtHandler source;
+    source.Connect(wxEVT_IDLE, wxIdleEventHandler(MyHandler::OnIdle), NULL, &sink);
+    CPPUNIT_ASSERT(source.Disconnect(wxID_ANY, wxEVT_IDLE));
+    // destruction of source and sink here should properly clean up the
+    // wxEventConnectionRef without crashing
+}
+
+void EvtHandlerTestCase::AutoDisconnect()
+{
+    wxEvtHandler source;
+    {
+        MyHandler sink;
+        source.Connect(wxEVT_IDLE, wxIdleEventHandler(MyHandler::OnIdle), NULL, &sink);
+        // mismatched event type, so nothing should be disconnected
+        CPPUNIT_ASSERT(!source.Disconnect(wxEVT_THREAD, wxIdleEventHandler(MyHandler::OnIdle), NULL, &sink));
+    }
+    // destruction of sink should have automatically disconnected it, so
+    // there should be nothing to disconnect anymore
+    CPPUNIT_ASSERT(!source.Disconnect(wxID_ANY, wxEVT_IDLE));
+}
+
 #ifdef wxHAS_EVENT_BIND
 
 void EvtHandlerTestCase::BindFunction()