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.

Closes #24885.

src/common/webrequest_curl.cpp

@@ -869,10 +869,13 @@ constexpr const char* TRACE_CURL = "curl";
 
 // SocketPollerSourceHandler - a source handler used by the SocketPoller class.
 
+class SourceSocketPoller;
+
 class SocketPollerSourceHandler: public wxEventLoopSourceHandler
 {
 public:
-    SocketPollerSourceHandler(curl_socket_t, wxEvtHandler*);
+    SocketPollerSourceHandler(curl_socket_t sock, SourceSocketPoller* poller)
+        : m_socket(sock), m_poller(poller) {}
 
     void OnReadWaiting() override;
     void OnWriteWaiting() override;
@@ -881,16 +884,9 @@ public:
 private:
     void SendEvent(int);
     curl_socket_t m_socket;
-    wxEvtHandler* m_handler;
+    SourceSocketPoller* const m_poller;
 };
 
-SocketPollerSourceHandler::SocketPollerSourceHandler(curl_socket_t sock,
-                                                     wxEvtHandler* hndlr)
-{
-    m_socket = sock;
-    m_handler = hndlr;
-}
-
 void SocketPollerSourceHandler::OnReadWaiting()
 {
     SendEvent(SocketPoller::READY_FOR_READ);
@@ -906,14 +902,6 @@ void SocketPollerSourceHandler::OnExceptionWaiting()
     SendEvent(SocketPoller::HAS_ERROR);
 }
 
-void SocketPollerSourceHandler::SendEvent(int result)
-{
-    wxThreadEvent event(wxEVT_SOCKET_POLLER_RESULT);
-    event.SetPayload<curl_socket_t>(m_socket);
-    event.SetInt(result);
-    m_handler->ProcessEvent(event);
-}
-
 // SourceSocketPoller - a SocketPollerImpl based on event loop sources.
 
 class SourceSocketPoller: public SocketPollerImpl
@@ -925,6 +913,8 @@ public:
     void StopPolling(curl_socket_t) override;
     void ResumePolling(curl_socket_t) override;
 
+    void SendEvent(curl_socket_t sock, int result);
+
 private:
     using SocketDataMap = std::unordered_map<curl_socket_t, wxEventLoopSource*>;
 
@@ -932,8 +922,21 @@ private:
 
     SocketDataMap m_socketData;
     wxEvtHandler* m_handler;
+
+    // The socket for which we're currently processing a write IO notification.
+    curl_socket_t m_activeWriteSocket = 0;
+
+    // The sockets that we couldn't clean up yet but should do if/when we get
+    // an error notification for them.
+    std::vector<curl_socket_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;
@@ -991,11 +994,9 @@ bool SourceSocketPoller::StartPolling(curl_socket_t sock, int pollAction)
     }
     else
     {
-        // Otherwise create a new source handler.
         wxLogTrace(TRACE_CURL, "Creating new socket poller for %d", sock);
 
-        srcHandler =
-            new SocketPollerSourceHandler(sock, m_handler);
+        srcHandler = new SocketPollerSourceHandler(sock, this);
     }
 
     // Get a new source object for these polling checks.
@@ -1027,6 +1028,17 @@ bool SourceSocketPoller::StartPolling(curl_socket_t sock, int pollAction)
 
 void SourceSocketPoller::StopPolling(curl_socket_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.
+        wxLogTrace(TRACE_CURL, "Delaying cleanup of socket poller for %d", sock);
+
+        m_socketsToCleanUp.push_back(sock);
+        return;
+    }
+
     SocketDataMap::iterator it = m_socketData.find(sock);
 
     if ( it != m_socketData.end() )
@@ -1042,6 +1054,35 @@ void SourceSocketPoller::ResumePolling(curl_socket_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<curl_socket_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 ( auto sock : m_socketsToCleanUp )
+        {
+            StopPolling(sock);
+        }
+
+        m_socketsToCleanUp.clear();
+    }
+}
+
 void SourceSocketPoller::CleanUpSocketSource(wxEventLoopSource* source)
 {
     wxEventLoopSourceHandler* srcHandler = source->GetHandler();