From 391555d341b1d91dadb79defafec2c8f0c55e69e Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 15 Sep 2022 00:05:07 +0200 Subject: [PATCH] Fix data race when using wxTheApp::m_handlersWithPendingEvents Appending to the vector from another thread could reallocate it, making access to its first element from the main thread invalid and resulting in probably very hard to diagnose crashes. Fix this by storing the handler in a local variable before releasing the critical section protecting the vector. This still relies on the handler not being deleted, but this was the case before too and shouldn't happen and, most importantly, there doesn't seem to be anything we can do about it if it does. Thanks TSAN for the warning. --- src/common/appbase.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/common/appbase.cpp b/src/common/appbase.cpp index 16328cee79..9bc3cce719 100644 --- a/src/common/appbase.cpp +++ b/src/common/appbase.cpp @@ -556,15 +556,18 @@ void wxAppConsoleBase::ProcessPendingEvents() // from it when they don't have any more pending events while (!m_handlersWithPendingEvents.IsEmpty()) { - // In ProcessPendingEvents(), new handlers might be added - // and we can safely leave the critical section here. - wxLEAVE_CRIT_SECT(m_handlersWithPendingEventsLocker); - // NOTE: we always call ProcessPendingEvents() on the first event handler // with pending events because handlers auto-remove themselves // from this list (see RemovePendingEventHandler) if they have no // more pending events. - m_handlersWithPendingEvents[0]->ProcessPendingEvents(); + wxEvtHandler* const handler = m_handlersWithPendingEvents[0]; + + // In ProcessPendingEvents(), new handlers might be added + // and we can safely leave the critical section here as we're not + // accessing m_handlersWithPendingEvents while we don't hold it. + wxLEAVE_CRIT_SECT(m_handlersWithPendingEventsLocker); + + handler->ProcessPendingEvents(); wxENTER_CRIT_SECT(m_handlersWithPendingEventsLocker); }