Repository navigation
[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
Closed
[DO NOT MERGE] Sockets (Qt): fix send/close race that hangs send() forever (CI test-hang root cause)#1839pa-devbot wants to merge 4 commits into
pa-devbot wants to merge 4 commits into
Conversation
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).
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). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes the intermittent CI test-runner hang (job sits until the 8h timeout, zero log progress).