Repository navigation
Keep socket test waits off the main thread - #1016
Merged
Merged
Conversation
TestStore installs a process-global hook that runs Task.detached on the main thread. A blocking wait inside such a task stopped the main-actor work it waited for, so the wait timed out when routing lost the race. Run the blocking work in CLISocketServerTests on a dispatch queue.
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.
Problem
CLISocketServerTests/nativeHookRoutesAfterPeerClosesBeforeMainActorHandling()failed on CI withExpectation failed: didRoute(run). This is a test defect, not a product defect.TestStoreinstalls a process-global hook that runsTask.detachedon the main thread. The test blocked on a semaphore inside such a task. When routing lost the race, the blocked main thread stopped the main-actor work that the semaphore waited for, and the wait used its full 10 s timeout.Two more tests wait on main-actor work in the same way:
descriptorDuplicationFailureRejectsConnectionBeforeRouting()closingPeerCancelsInFlightWaitRequest()Fix
Add a
runBlockinghelper that runs blocking work on a dispatch queue, and use it for all blocking waits and sends inCLISocketServerTests. A longer timeout cannot fix this: the main thread stays blocked for the full wait.Only
App/Tests/CLISocketServerTests.swiftchanges.Verification
To force the race, I temporarily turned the hook on and added a 50 ms delay on the accept queue after
peerClosed.signal(). These edits are not in this PR.make check,make test, andmake build-apppass with the final code.