fix(abort): park a dropped signal's receivers instead of spinning - #45
Merged
Merged
Conversation
AbortReceiver::recv looped on watch::Receiver::changed() and ignored its result. Once the AbortSignal, which owns the watch sender, was dropped without aborting, changed() returned Err on every poll and the loop never waited: whichever task awaited recv() ran at full CPU. fetch keeps such a task alive for every request that carries a signal (the abort bridge), so an app that gives its requests an AbortController pinned a JS worker for each signal it let go. recv() now parks once the channel is closed, since a dropped signal can no longer abort. fetch's abort bridge also ends when the transport drops its receiver, instead of outliving every request that carried a signal.
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.
Summary
AbortReceiver::recvlooped onwatch::Receiver::changed()and ignored its result. When theAbortSignal(the watch sender) is dropped without aborting,changed()returnsErron every poll, so the loop never waits and the task awaitingrecv()runs at full CPU.recv()now parks once the channel is closed, because a dropped signal can never abort.fetchspawns an abort bridge (select!overrecv()and a stop notify) for every request that carries a signal. On a normal response the bridge is never stopped, so it outlived the request, and once the JS signal was collected it became one of those spinning tasks. The bridge now also ends when the transport drops its receiver (tx.closed()).a_dropped_signal_parks_its_receivers_instead_of_spinning: countsrecv()polls for 50 ms after the signal is dropped.Validation
CIpassed: fmt, typegen, msrv, host-verify QuickJS (Windows / Ubuntu / macOS), host-verify JavaScriptCore (macOS), andjscore-source-*on macOS / Linux / Windows.cargo test -p rong_abort --features quickjs(6 passed; the new test fails without the fix withrecv() was polled 301 times in 50ms),cargo fmt --check -p rong_abort -p rong_http, andcargo clippy -p rong_abort -p rong_http --lib --tests --no-default-features --features rong_abort/quickjs,rong_http/quickjs -- -D warnings.cargo make ci-verify.rong_http'stest_fetchfails the same 13 of 19 JS cases on this machine with and without this change (local test server requests return "fetch failed"), so abort-during-body-read is left to host CI.Not applicable (no changes in those areas): JSC artifact pin, npm packages, ArkJS / OHOS, Harmony.
Release Impact
User-facing bugfix: a dropped
AbortSignalcould pin a JS worker at 100% CPU, andfetchleft its abort-bridge task alive after the request. No crate version bump in this PR (same as other module fixes). Changelog androng_abort/rong_httppatch publish can follow so LingXia can drop a[patch].Notes
AbortControllerfor its timeout. On the phone, JS worker threads sat at ~100% CPU, and the number grew from one to four as requests accumulated. Asampleof the macOS Runner showed the hot worker insiderong_http::fetch→rong_abort::abort_signal::recv→tokio::sync::watch→coop.