Repository navigation
fix(zero-swift): send WebSocket frames off the main actor - #107
Merged
Merged
Conversation
After 30 s the test records how many frames the server got and what the client's writer logged, so a stall shows whether a client send or the server's receive stopped. Amp-Thread-ID: https://ampcode.com/threads/T-c513a037-a257-45e0-b9a9-15c15edfe859 Co-authored-by: tmm <tmm@tmm.dev>
The first report showed the frames arriving late, not lost, but the writer's log runs on the main actor and couldn't tell a slow send from a busy main actor. The test now times each send in a wrapping socket, counts the server's frames off the main actor, and measures the main actor's longest wait. Amp-Thread-ID: https://ampcode.com/threads/T-c513a037-a257-45e0-b9a9-15c15edfe859 Co-authored-by: tmm <tmm@tmm.dev>
The frame order test's report showed each send finishing in at most 7 ms but only 107 of 200 starting in 30 s, with the main actor waiting up to 6.6 s: the writer ran on the main actor and waited for it after every frame. It now runs detached. A new test blocks the main actor for a second after queuing 200 frames; without the fix the server gets none. Amp-Thread-ID: https://ampcode.com/threads/T-c513a037-a257-45e0-b9a9-15c15edfe859 Co-authored-by: tmm <tmm@tmm.dev>
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.
keepsTheOrderOfFramesSentBeforeAndAfterTheOpentimed out on main (b7c61fb, Swift (macOS)). This PR first added timing to the test; its CI run then showed the cause:Every send finished quickly and the server got each frame at once, but the sends started slowly.
TransportConnection's writer ran on the main actor and went back to it after each frame, so while the main actor was busy (here, other tests in the suite), every frame waited its turn. In an app, a busy main thread would hold up mutations the same way.The writer now runs in a detached task, still one at a time, so frames and the close after them keep their order.
sendsWhileTheMainActorIsBusy: opens a socket, queues 200 frames, blocks the main actor for 1 s, and expects the server to have all 200. Without the fix it gets 0.keepsTheOrderOfFramesSentBeforeAndAfterTheOpenkeeps the timing report (sends timed in a wrapping socket, server frames counted off the main actor, the main actor's longest wait) for anything else that stalls.Checked on a Mac (Xcode 26.6): both tests 30/30, three full suites pass; Linux
ZeroTests|ZeroVirtualTests|E2ETestspass. Patch changeset.