From e50ab1f6c5e3bd1bd702fec31359e4390c55e0e2 Mon Sep 17 00:00:00 2001
From: PA Dev Bot <344001113+pa-devbot@users.noreply.github.com>
Date: Tue, 3 Nov 2026 17:03:33 +0000
Subject: [PATCH] Sockets (Qt): fix send/close race that hangs send() forever

ClientSocket_Qt::send() copied the payload (up to 256 MiB in the socket
tests) before registering the send in m_inflight_sends. If close() - or a
reconnecting connect() - ran during that window, it cancelled an empty
list and queued the socket's destruction; the send's queued
internal_send call was then posted after the destruction event, Qt
silently discarded it, and "done" was never set. The infinite-timeout
wait in send() then blocked forever, the dispatched send task never
finished, and the whole test suite hung - the ParallelUnitTestRunner
waits for every test to complete. (ServerSocket_Qt::send documents this
exact Qt limitation and bounds its wait for this reason; the client
relies on close() cancelling the send, which this race defeats.)

Register the send under m_lock while re-checking that m_socket is still
attached: a send either registers before close() moves the list (and is
cancelled, waking the wait) or sees the detached socket and returns 0
without queueing anything. Either way, the unbounded infinite-timeout
wait is actually always ended, as its comment claims.

Reproduced deterministically before the fix: 256 MiB send with close()
during the payload copy hung the send task forever (close() returned in
1 ms, both event threads idle, caller parked in cv.wait). Passes after.
---
 Common/Cpp/Sockets/ClientSocket_Qt.cpp | 37 ++++++++++++++++++++------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/Common/Cpp/Sockets/ClientSocket_Qt.cpp b/Common/Cpp/Sockets/ClientSocket_Qt.cpp
index a866b33d00..768d2cfec5 100644
--- a/Common/Cpp/Sockets/ClientSocket_Qt.cpp
+++ b/Common/Cpp/Sockets/ClientSocket_Qt.cpp
@@ -107,9 +107,27 @@ size_t ClientSocket_Qt::send(const void* data, size_t bytes, std::chrono::millis
             ? std::chrono::steady_clock::time_point::max()
             : std::chrono::steady_clock::now() + timeout;
 
+    //  Register the send atomically with respect to "close()"/"connect()":
+    //  both detach "m_socket" under this same lock before they cancel the
+    //  in-flight sends. The packet copy above can take a while for large
+    //  payloads, so without this re-check a send could register *after*
+    //  "close()" already cancelled an (empty) list and queued the socket's
+    //  destruction: its "internal_send" call would then be posted after the
+    //  destruction event, Qt would silently discard it, and the wait below
+    //  (unbounded for the infinite default timeout) would never end. With
+    //  the re-check, a send either registers before the cancel (and gets
+    //  cancelled, waking the wait below) or sees the detached socket and
+    //  returns without queueing anything.
+    bool registered;
     {
         std::lock_guard<Mutex> lg(m_lock);
-        m_inflight_sends.push_back(send_data);
+        registered = (m_socket != nullptr);
+        if (registered){
+            m_inflight_sends.push_back(send_data);
+        }
+    }
+    if (!registered){
+        return 0;
     }
 
     emit internal_send(send_data);
@@ -120,13 +138,16 @@ size_t ClientSocket_Qt::send(const void* data, size_t bytes, std::chrono::millis
     //  write loop stops at the deadline, so anything beyond that is just
     //  the queued call waiting to start.
     //
-    //  The infinite default waits without a time bound: "done" is set by
-    //  either the event-thread write loop (which stops at the deadline,
-    //  on a socket error, or when Qt's userspace buffer accepts the
-    //  packet) or by "close()"/"connect()" cancelling the send before
-    //  the socket is destroyed, so the wait is always ended. The shared
-    //  ownership keeps the packet alive until the queued call runs
-    //  (or is discarded with the socket).
+    //  The infinite default waits without a time bound. The registration
+    //  above is atomic with the "close()"/"connect()" detach, so "done" is
+    //  set by either the event-thread write loop (which stops at the
+    //  deadline, on a socket error, or when Qt's userspace buffer accepts
+    //  the packet; it runs before the queued socket destruction, since the
+    //  event loop is FIFO) or by "close()"/"connect()" cancelling the
+    //  registered send - even when Qt discards the queued write loop with
+    //  the socket - so the wait always ends. The shared ownership keeps
+    //  the packet alive until the queued call runs (or is discarded with
+    //  the socket).
     {
         std::unique_lock<Mutex> lg(send_data->lock);
         if (timeout == std::chrono::milliseconds::max()){