Ticket #14563 (closed defect: fixed)
Crash when Disconnect()ing event handler without specifying function
|Reported by:||cwalther||Owned by:|
|Keywords:||wxEvtHandler Disconnect wxEventConnectionRef||Cc:|
After this sequence
source->Connect(wxEVT_SOMETHING, wxSomethingEventHandler(MySink::OnSomething), NULL, sink); source->Disconnect(wxID_ANY, wxEVT_SOMETHING)); delete source;
destruction of sink crashes because wxEventConnectionRef::OnObjectDestroy tries to call a method on the already destroyed source.
The cause is that calling Disconnect() without specifying the function (letting it figure it out on its own, taking the first match for wxEVT_SOMETHING) fails to call DecRef() on the wxEventConnectionRef connecting sink and source, so that it erroneously still exists at the time sink is destroyed.
I realize this is a somewhat exotic situation, as the problem can be easily worked around by passing the function to Disconnect() too, and Connect/Disconnect is supposed to be replaced by Bind/Unbind now, but as far as I understand the documentation, this should work. I just ran into this problem while porting an application to wxWidgets 2.9 that worked fine in 2.6.
To reproduce: apply the attached disconnectcrash-unittest.patch to the events/evthandler unit test and run test EvtHandlerTestCase. Expected result: tests should pass. Actual result: EvtHandlerTestCase::DisconnectWildcard() crashes on line evthandler.cpp:265 (at least when run on a C library that overwrites freed memory, such as in a debug build on Windows (MSVC10) - it didn’t crash for me on Linux, but even there stepping through with a debugger clearly shows that a method is being called on a destroyed object).
It seems to me this could be fixed by the attached disconnectcrash.patch, but I am completely unfamiliar with wxEventConnectionRef etc., so I’d appreciate a thorough review. In addition to fixing the above crash, it also makes sure that wxEventConnectionRef::DecRef() is not called when the loop over m_dynamicEvents doesn’t find anything to disconnect (Disconnect() will return false). Otherwise, such unmatching Disconnect() calls could cause OnSinkDestroyed not to be called. I added a test case for that too (EvtHandlerTestCase::AutoDisconnect).