From 359f4e21beae4b45142536928a9daccfa116f198 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 3 Feb 2016 23:23:10 +0100 Subject: [PATCH] Don't crash when deleting an object from its event handler Since the changes of 99d9a81e28e93106b2a471198cbfaa474a0fa714 a crash would happen if an event handler was unbound from an object which was later deleted from its own event handler. As unlikely as such scenario sounds, this is what happened with wxTaskBarIcon when two wxNotificationMessages were created in close succession under MSW and it was difficult to debug because of the timing constraints involved, so avoid similar crashes in the future by avoiding to use the fields of the object after an event has been handled and postpone pruning of the unbound event table entries until later time. See #17229. --- src/common/event.cpp | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/common/event.cpp b/src/common/event.cpp index eae17196ce..3e7f76f614 100644 --- a/src/common/event.cpp +++ b/src/common/event.cpp @@ -1797,7 +1797,6 @@ bool wxEvtHandler::SearchDynamicEventTable( wxEvent& event ) DynamicEvents& dynamicEvents = *m_dynamicEvents; - bool processed = false; bool needToPruneDeleted = false; // We can't use Get{First,Next}DynamicEntry() here as they hide the deleted @@ -1824,8 +1823,20 @@ bool wxEvtHandler::SearchDynamicEventTable( wxEvent& event ) handler = this; if ( ProcessEventIfMatchesId(*entry, handler, event) ) { - processed = true; - break; + // It's important to skip pruning of the unbound event entries + // below because this object itself could have been deleted by + // the event handler making m_dynamicEvents a dangling pointer + // which can't be accessed any longer in the code below. + // + // In practice, it hopefully shouldn't be a problem to wait + // until we get an event that we don't handle before pruning + // because this should happen soon enough and even if it + // doesn't the worst possible outcome is slightly increased + // memory consumption while not skipping pruning can result in + // hard to reproduce (because they require the disconnection + // and deletion happen at the same time which is not always the + // case) crashes. + return true; } } } @@ -1843,7 +1854,7 @@ bool wxEvtHandler::SearchDynamicEventTable( wxEvent& event ) dynamicEvents.resize(nNew); } - return processed; + return false; } void wxEvtHandler::DoSetClientObject( wxClientData *data )