From f2918a9ac823074901ce27de939baa57788beb3d Mon Sep 17 00:00:00 2001
From: Vadim Zeitlin <vadim@wxwidgets.org>
Date: Sun, 27 Oct 2024 00:56:21 +0200
Subject: [PATCH] Fix crash when connection is refused in wxWebRequestCURL

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)
---
 docs/changes.txt               |  1 +
 src/common/webrequest_curl.cpp | 80 +++++++++++++++++++++++++---------
 2 files changed, 61 insertions(+), 20 deletions(-)

diff --git a/src/common/webrequest_curl.cpp b/src/common/webrequest_curl.cpp
index f50acf4f8d2a..64650ab6b45a 100644
--- a/src/common/webrequest_curl.cpp
+++ b/src/common/webrequest_curl.cpp
@@ -704,10 +704,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;
@@ -716,16 +719,9 @@ class SocketPollerSourceHandler: public wxEventLoopSourceHandler
 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);
@@ -741,14 +737,6 @@ void SocketPollerSourceHandler::OnExceptionWaiting()
     SendEvent(SocketPoller::HAS_ERROR);
 }
 
-void SocketPollerSourceHandler::SendEvent(int result)
-{
-    wxThreadEvent event(wxEVT_SOCKET_POLLER_RESULT);
-    event.SetPayload<wxSOCKET_T>(m_socket);
-    event.SetInt(result);
-    m_handler->ProcessEvent(event);
-}
-
 // SourceSocketPoller - a SocketPollerImpl based on event loop sources.
 
 class SourceSocketPoller: public SocketPollerImpl
@@ -760,6 +748,8 @@ class SourceSocketPoller: public SocketPollerImpl
     void StopPolling(wxSOCKET_T) wxOVERRIDE;
     void ResumePolling(wxSOCKET_T) wxOVERRIDE;
 
+    void SendEvent(wxSOCKET_T sock, int result);
+
 private:
     WX_DECLARE_HASH_MAP(wxSOCKET_T, wxEventLoopSource*, wxIntegerHash,\
                         wxIntegerEqual, SocketDataMap);
@@ -768,11 +758,25 @@ class SourceSocketPoller: public SocketPollerImpl
 
     SocketDataMap m_socketData;
     wxEvtHandler* m_handler;
+
+    // The socket for which we're currently processing a write IO notification.
+    wxSOCKET_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<wxSOCKET_T> 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()
@@ -822,9 +826,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.
@@ -858,6 +860,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() )
@@ -871,6 +882,35 @@ void SourceSocketPoller::ResumePolling(wxSOCKET_T WXUNUSED(sock))
 {
 }
 
+void SourceSocketPoller::SendEvent(wxSOCKET_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<wxSOCKET_T>(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();
