Skip to content

[DO NOT MERGE] Sockets (Qt): fix send/close race that hangs send() forever (CI test-hang root cause) - #1839

Closed
pa-devbot wants to merge 4 commits into
mainfrom
qt-socket-close-send-race-only
Closed

pa-devbot wants to merge 4 commits into
mainfrom
qt-socket-close-send-race-only

Conversation

@pa-devbot

@pa-devbot pa-devbot commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

What

Fixes the intermittent CI test-runner hang (job sits until the 8h timeout, zero log progress).

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.)

Registers 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.

## Verification

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 Qt event threads idle in poll, the send task parked in
cv.wait). Passes after the fix (25+ iterations, close at 0/15/100 ms).

Local full-suite run (Qt 6.10.2, aarch64) completes with this fix.

## Related

- #1840 - hang watchdog + all-thread stack dumps for the test runner
  (the diagnostics this was diagnosed with; its CI runs show the
  watchdog firing on this exact test: the dump reports
  "Running for 223s: Sockets (Qt) - close() cancels an in-flight
  send" and the parked frame is
  std::condition_variable::wait inside ClientSocket_Qt::send).
- Follows #1831 (close() cancels in-flight sends) and #1838 (runner on
  the uncapped pool).

[DO NOT MERGE] - debug/investigation PR, kept open so CI keeps running on it.

## Update: Windows CI hang (separate issue)

The two Windows test steps of the first commit (9cddbae14) ran ~1 h
against a ~3 min baseline for this step - a Windows hang that is NOT the
Qt send/close race (this branch fixes that, and all 4 non-Windows
platforms pass, including the socket tests). The hang watchdog has now
been added to this branch (cherry-picked from #1840) so the next Windows
run self-terminates ~60 s after progress stops and dumps the stuck
state; the dump will identify the (probably different) culprit.

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.
If no test COMPLETES for 60 s (PA_HANG_WATCHDOG overrides in seconds;
0 disables), the command-line test runner now dumps its own state
(pending/running/completed counts, per-running-test elapsed time, thread
pool queue/busy counts, file logger queue depth) and a stack trace of
every thread in the process straight to the stderr file descriptor, then
exits with code 124. This turns a hung CI runner - which would otherwise
sit until the 8 h job timeout - into a failed job with a full diagnostic
dump.

- HangWatchdog: dedicated watchdog thread (not on the test pools, so it
  fires even when every test thread is blocked). POSIX dumps capture
  each thread's backtrace() by sending it SIGUSR2 (Linux signals every
  /proc tid; macOS signals a registry of program-created threads);
  Windows suspends each thread and walks it with StackWalk64 + SymFromAddr.
- ParallelUnitTestRunner: diagnostics_state() snapshot and
  note_test_completed() per finished test.
- ThreadPool: queued_tasks()/busy_threads() accessors; FileLogger:
  pending_count()/max_queue_size().

CI note (separate change, needs workflow-scope push): to make the dump
readable in CI logs, keep the Windows PDBs, add -g on ubuntu-clang and
-rdynamic on Linux/macOS, and upload the Linux/macOS builds for offline
addr2line. Without those, frames resolve to module+offset only.

[DEBUG: do not merge] -- part of the CI-hang investigation; see
qt-socket-close-send-race-only for the suspected root-cause fix.
…gistry

THREADLIST32/Thread32First are not available in the CI SDK/MSVC
configuration (and SymFunction is not in the public dbghelp header, and
STACKFRAME64 has no Machine field on x64), so enumerate the dump's
threads from the program's own registry instead (register_thread() now
records the thread id on Windows, as it does on macOS): the runner,
pool, event-thread and logger threads - i.e., the ones that matter for a
hang - are all program-created and registered. Library-created threads
are not captured, noted in the dump as on macOS.

The StackWalk64 callback is now a local stub that just continues.
The CI SDK's dbghelp.h has the 9-parameter StackWalk64 (no frame-walk
callback, stack frame before context) and lacks the PSTACKFRAME64 /
PSYMBOL_INFO pointer typedefs, so drop the callback entirely and stop
using the pointer types (SYMBOL_INFO / STACKFRAME64 are used by value).
@pa-devbot

Copy link
Copy Markdown
Collaborator Author

Superseded: the socket fix is now on its own in #1842 and the watchdog in #1840. This PR contained both (the watchdog was temporarily folded in to diagnose a non-reproducing Windows test-step stall). Closing in favor of the two focused PRs — all CI evidence from this branch carries over (#1842's runs re-verified the fix on all 6 platforms).

@pa-devbot pa-devbot closed this Nov 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant