From dd23d2cf25019b933b7a862d24f92cced433efc2 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 5 Jan 2021 00:26:49 +0100 Subject: [PATCH] Fix and simplify wxWebRequestImpl objects ref counting When cancelling a request it was possible for it to be deleted while it was still used from another thread. Fix this and make the code much more obviously correct by simply "locking" the request until the event generated by it is processed: now IncRef() and DecRef() calls are always balanced as they're called from ctor and dtor of StateEventProcessor helper only. --- src/common/webrequest.cpp | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/common/webrequest.cpp b/src/common/webrequest.cpp index 7d87a533a0..58fecf82ec 100644 --- a/src/common/webrequest.cpp +++ b/src/common/webrequest.cpp @@ -160,6 +160,15 @@ struct StateEventProcessor const wxString& failMsg) : m_request(request), m_state(state), m_failMsg(failMsg) { + // Ensure that the request object stays alive until this event is + // processed. + m_request.IncRef(); + } + + StateEventProcessor(const StateEventProcessor& other) + : m_request(other.m_request), m_state(other.m_state), m_failMsg(other.m_failMsg) + { + m_request.IncRef(); } void operator()() @@ -167,6 +176,11 @@ struct StateEventProcessor m_request.ProcessStateEvent(m_state, m_failMsg); } + ~StateEventProcessor() + { + m_request.DecRef(); + } + wxWebRequestImpl& m_request; const wxWebRequest::State m_state; const wxString m_failMsg; @@ -176,10 +190,6 @@ struct StateEventProcessor void wxWebRequestImpl::SetState(wxWebRequest::State state, const wxString & failMsg) { - // Add a reference while the request is active - if ( IsActiveState(state) && !IsActiveState(m_state) ) - IncRef(); - m_state = state; // Trigger the event in the main thread @@ -292,10 +302,6 @@ void wxWebRequestImpl::ProcessStateEvent(wxWebRequest::State state, const wxStri if ( state == wxWebRequest::State_Completed && m_storage == wxWebRequest::Storage_File && wxFileExists(responseFileName) ) wxRemoveFile(responseFileName); - - // Remove reference after the request is no longer active - if ( !IsActiveState(state) ) - DecRef(); } //