Repository navigation
Sockets (Qt): fix send/close race that hangs send() forever - #1842
Merged
Merged
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.
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): the parallel test runner waits for every test to complete, and this test can never complete.
How this was found
A hang watchdog for the test runner (#1840) dumped the stuck state in
CI:
Running for 223s: Sockets (Qt) - close() cancels an in-flight send, with the send task parked incv.waitinsideClientSocket_Qt::send(both Qt event threads idle).Verification
during the payload copy): hung 100% before the fix, passes after
(25+ iterations, close at 0/15/100 ms).
CI green on all 6 platforms, e.g. Windows: 26,522 passed / 0 failed,
including all Sockets (Qt) and Sockets (WinSocket) tests.
Follows #1831 (close() cancels in-flight sends) and #1838 (runner on
the uncapped pool). Related: #1840 (the hang watchdog used to
diagnose this).