From f2918a9ac823074901ce27de939baa57788beb3d Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 27 Oct 2024 00:56:21 +0200 Subject: [PATCH] Fix crash when connection is refused in wxWebRequestCURL Refer: https://github.com/wxWidgets/wxWidgets/commit/f2918a9ac823074901ce27de939baa57788beb3d Avoid deleting wxEventLoopSourceHandler which may be still in use, as is the case when we get write IO notification just before an error one: if we delete the handler while handling the former, we crash when getting the latter one. Use a hack to avoid deleting the handlers for which write notification is being processed and delete them later, when we get the error one. See #24885. (cherry picked from commit 4e0fca8ab9756989598d07b41e672af86eac7092) --- src/common/webrequest_curl.cpp | 80 +++++++++++++++++++++++++--------- 1 file changed, 60 insertions(+), 20 deletions(-) diff --git a/src/common/webrequest_curl.cpp b/src/common/webrequest_curl.cpp index 91a8aec..e7a0ce5 100644 --- a/src/common/webrequest_curl.cpp +++ b/src/common/webrequest_curl.cpp @@ -685,10 +685,13 @@ SocketPollerImpl* SocketPollerImpl::Create(wxEvtHandler* hndlr) // SocketPollerSourceHandler - a source handler used by the SocketPoller class. +class SourceSocketPoller; + class SocketPollerSourceHandler: public wxEventLoopSourceHandler { public: - SocketPollerSourceHandler(wxSOCKET_T, wxEvtHandler*); + SocketPollerSourceHandler(wxSOCKET_T sock, SourceSocketPoller* poller) + : m_socket(sock), m_poller(poller) {} void OnReadWaiting() wxOVERRIDE; void OnWriteWaiting() wxOVERRIDE; @@ -697,16 +700,9 @@ public: private: void SendEvent(int); wxSOCKET_T m_socket; - wxEvtHandler* m_handler; + SourceSocketPoller* const m_poller; }; -SocketPollerSourceHandler::SocketPollerSourceHandler(wxSOCKET_T sock, - wxEvtHandler* hndlr) -{ - m_socket = sock; - m_handler = hndlr; -} - void SocketPollerSourceHandler::OnReadWaiting() { SendEvent(SocketPoller::READY_FOR_READ); @@ -722,14 +718,6 @@ void SocketPollerSourceHandler::OnExceptionWaiting() SendEvent(SocketPoller::HAS_ERROR); } -void SocketPollerSourceHandler::SendEvent(int result) -{ - wxThreadEvent event(wxEVT_SOCKET_POLLER_RESULT); - event.SetPayload(m_socket); - event.SetInt(result); - m_handler->ProcessEvent(event); -} - // SourceSocketPoller - a SocketPollerImpl based on event loop sources. class SourceSocketPoller: public SocketPollerImpl @@ -741,6 +729,8 @@ public: void StopPolling(wxSOCKET_T) wxOVERRIDE; void ResumePolling(wxSOCKET_T) wxOVERRIDE; + void SendEvent(curl_socket_t sock, int result); + private: WX_DECLARE_HASH_MAP(wxSOCKET_T, wxEventLoopSource*, wxIntegerHash,\ wxIntegerEqual, SocketDataMap); @@ -749,11 +739,25 @@ private: SocketDataMap m_socketData; wxEvtHandler* m_handler; + + // The socket for which we're currently processing a write IO notification. + curl_socket_t m_activeWriteSocket; + + // The sockets that we couldn't clean up yet but should do if/when we get + // an error notification for them. + wxVector m_socketsToCleanUp; }; +// This function must be implemented after full SourceSocketPoller declaration. +void SocketPollerSourceHandler::SendEvent(int result) +{ + m_poller->SendEvent(m_socket, result); +} + SourceSocketPoller::SourceSocketPoller(wxEvtHandler* hndlr) { m_handler = hndlr; + m_activeWriteSocket = 0; } SourceSocketPoller::~SourceSocketPoller() @@ -803,9 +807,7 @@ bool SourceSocketPoller::StartPolling(wxSOCKET_T sock, int pollAction) } else { - // Otherwise create a new source handler. - srcHandler = - new SocketPollerSourceHandler(sock, m_handler); + srcHandler = new SocketPollerSourceHandler(sock, this); } // Get a new source object for these polling checks. @@ -839,6 +841,15 @@ bool SourceSocketPoller::StartPolling(wxSOCKET_T sock, int pollAction) void SourceSocketPoller::StopPolling(wxSOCKET_T sock) { + if ( sock == m_activeWriteSocket ) + { + // We can't clean up the socket while we're inside OnWriteWaiting() for + // it because it could be followed by OnExceptionWaiting() and we'd + // crash if we deleted it already. + m_socketsToCleanUp.push_back(sock); + return; + } + SocketDataMap::iterator it = m_socketData.find(sock); if ( it != m_socketData.end() ) @@ -852,6 +863,35 @@ void SourceSocketPoller::ResumePolling(wxSOCKET_T WXUNUSED(sock)) { } +void SourceSocketPoller::SendEvent(curl_socket_t sock, int result) +{ + if ( result == SocketPoller::READY_FOR_WRITE ) + { + // Prevent the handler from this socket from being deleted in case we + // get a HAS_ERROR event for it immediately after this one. + m_activeWriteSocket = sock; + } + + wxThreadEvent event(wxEVT_SOCKET_POLLER_RESULT); + event.SetPayload(sock); + event.SetInt(result); + m_handler->ProcessEvent(event); + + m_activeWriteSocket = 0; + + if ( result == SocketPoller::HAS_ERROR ) + { + // Check if we have any sockets to clean up and do it now, it should be + // safe. + for ( size_t n = 0; n < m_socketsToCleanUp.size(); ++n ) + { + StopPolling(m_socketsToCleanUp[n]); + } + + m_socketsToCleanUp.clear(); + } +} + void SourceSocketPoller::CleanUpSocketSource(wxEventLoopSource* source) { wxEventLoopSourceHandler* srcHandler = source->GetHandler(); -- 2.48.1