fix(turbo-tasks): let parking_lot block on wasm instead of panicking - #97860
Open
sokra wants to merge 1 commit into
Open
fix(turbo-tasks): let parking_lot block on wasm instead of panicking#97860sokra wants to merge 1 commit into
sokra wants to merge 1 commit into
Conversation
Contributor
Tests PassedCommit: c58ce2a |
Contributor
Stats from current PR🔴 3 regressions
📊 All Metrics📖 Metrics GlossaryDev Server Metrics:
Build Metrics:
Change Thresholds:
⚡ Dev Server
📦 Dev Server (Webpack) (Legacy)📦 Dev Server (Webpack)
⚡ Production Builds
📦 Production Builds (Webpack) (Legacy)📦 Production Builds (Webpack)
📦 Bundle SizesBundle Sizes⚡ TurbopackClient Main Bundles
Server Middleware
Build DetailsBuild Manifests
Build Cache
📦 WebpackClient Main Bundles
Polyfills
Pages
Server Edge SSR
Middleware
Build DetailsBuild Manifests
Build Cache
🔄 Shared (bundler-independent)Runtimes
📎 Tarball URLCommit: 2f009c8 |
sokra
force-pushed
the
codex/turbopack-wasm/parking-lot-atomics
branch
from
August 25, 2026 12:09
ddcdb03 to
45b02ef
Compare
sokra
force-pushed
the
codex/turbopack-wasm/parking-lot-atomics
branch
from
August 25, 2026 13:04
45b02ef to
78d2875
Compare
sokra
force-pushed
the
codex/turbopack-wasm/parking-lot-atomics
branch
from
August 25, 2026 14:23
78d2875 to
e49ddd5
Compare
sokra
force-pushed
the
codex/turbopack-wasm/parking-lot-atomics
branch
from
August 25, 2026 16:49
e49ddd5 to
bf40dd6
Compare
sokra
force-pushed
the
codex/turbopack-wasm/parking-lot-atomics
branch
from
August 25, 2026 16:58
bf40dd6 to
439383a
Compare
sokra
force-pushed
the
codex/turbopack-wasm/parking-lot-atomics
branch
from
August 26, 2026 12:36
439383a to
2f009c8
Compare
sokra
force-pushed
the
codex/turbopack-wasm/parking-lot-atomics
branch
from
August 28, 2026 08:01
2f009c8 to
7665272
Compare
sokra
force-pushed
the
codex/turbopack-wasm/parking-lot-atomics
branch
2 times, most recently
from
September 8, 2026 11:04
551b095 to
a96be6d
Compare
sokra
force-pushed
the
codex/turbopack-wasm/parking-lot-atomics
branch
from
September 8, 2026 20:57
a96be6d to
148523b
Compare
sokra
force-pushed
the
codex/turbopack-wasm/parking-lot-atomics
branch
from
September 9, 2026 01:19
148523b to
03c83a9
Compare
sokra
marked this pull request as ready for review
September 9, 2026 08:25
sokra
force-pushed
the
codex/turbopack-wasm/parking-lot-atomics
branch
from
September 9, 2026 08:25
03c83a9 to
8dc6f73
Compare
Member
Author
|
Looks like the CI job |
bgw
approved these changes
Sep 10, 2026
| /// Timing-based on purpose: a lock that returned immediately without blocking would satisfy a | ||
| /// pure handover assertion, so real elapsed time is what distinguishes blocking from spinning | ||
| /// past the guard. | ||
| #[test] |
Member
There was a problem hiding this comment.
This is fine, but probably overkill, as there are tons of other tests that will fail if parking_lot is misconfigured. This just reads as a unit test for parking_lot.
Comment on lines
+337
to
+353
|
|
||
| // How much of the input gets mutated depends on the chunking, which is derived from | ||
| // `available_parallelism()`: each chunk stops at its first error, so items after one in the | ||
| // same chunk are never visited. Asserting `[11, 12, 13, 14, 15]` would only hold where the | ||
| // chunk size is 1, and fails anywhere parallelism is reported as 1 — a single-core machine, | ||
| // or wasm. Assert the parallelism-independent contract instead: every item is either | ||
| // untouched or incremented exactly once, and the first one was visited. | ||
| assert_eq!(input.len(), 5); | ||
| for (i, &value) in input.iter().enumerate() { | ||
| let original = i as i32 + 1; | ||
| assert!( | ||
| value == original || value == original + 10, | ||
| "input[{i}] = {value}, expected {original} or {}", | ||
| original + 10 | ||
| ); | ||
| } | ||
| assert_eq!(input[0], 11); |
Member
There was a problem hiding this comment.
Sure, but I think this also really waters down the test, this really only tests that the first iteration does any sort of mutation. The original test was asserting that we try to start multiple jobs in parallel.
What if we just call available_parallelism() here, and size the test appropriately from that? On wasm the test might still be pretty useless, but at least it would still be useful on other platforms.
Otherwise, I think we should just delete the test. In this current state it seems pretty useless.
sokra
force-pushed
the
codex/turbopack-wasm/parking-lot-atomics
branch
from
September 12, 2026 13:11
8dc6f73 to
55c346f
Compare
`parking_lot_core` picks its thread parker in `thread_parker/mod.rs`. The
working wasm parker (`wasm_atomic.rs`, built on `memory_atomic_wait32` /
`memory_atomic_notify`) is selected only under
all(feature = "nightly", target_family = "wasm", target_feature = "atomics")
Otherwise it falls back to `wasm.rs`, whose `park()` is
`panic!("Parking not supported on this platform")`. `wasm32-wasip1-threads`
does report `target_feature = "atomics"`, so the only thing missing was the
feature — every contended lock panicked, which is why 35 tests were ignored
on wasm.
Two changes are needed and neither alone is enough:
- bump `parking_lot_core` 0.9.8 -> 0.9.12, because 0.9.8's atomic parker needs
`feature(stdsimd)`, removed from Rust long ago;
- enable `parking_lot`'s `nightly` feature for wasm targets only (it forwards
to `parking_lot_core/nightly`). 0.9.12 *still* gates the atomic parker behind
it, so a bump on its own keeps selecting the panicking stub silently.
Verified on the resolved feature graph rather than the manifest: `nightly` is
present for `wasm32-wasip1-threads` and absent for the host.
`contended_mutex_blocks_and_hands_over` is the regression test, built so it
cannot pass vacuously: a barrier guarantees the waiter contends, the waiter
must block for a measurable interval, and it must observe the holder's write.
With the feature removed it fails at `thread_parker/wasm.rs:26`.
33 of the 35 ignores are removed. The other two --
`scope::tests::test_scope_runs_in_parallel` and
`priority_runner::tests::test_mixed_cpu_bound_and_waiting_tasks` -- now carry
an accurate reason: their work completes in ~500ms of an 800ms serial sum, so
the parallelism is real, but dropping a multi-thread tokio runtime while their
blocking helpers are still alive deadlocks on wasm. That was previously hidden,
because `panic = abort` killed the process at the failing assertion before
teardown ran. It is fixed in the wasm runtime layer.
turbo-tasks 54 pass / 28 ignored -> 72 pass / 11 ignored
turbo-tasks-backend 66 pass / 32 ignored -> 82 pass / 15 ignored
native 0 ignored, unchanged
Also fixes a latent test bug unrelated to wasm: `test_parallel_try_for_each_mut`
asserted every element was incremented, which only holds when the chunk size is
1. Chunking comes from `available_parallelism() * 4` and each chunk stops at its
first error, so with parallelism 1 the chunk size is 2 and every second item is
skipped -- it would fail on any single-core machine. It now asserts the
parallelism-independent contract.
Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>
Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
This also removes the 16 temporary `scope_unbounded` ignores that the
previous layer had to add: with a working parker, 12 of them pass, 3 are
re-labelled `no unwinding on wasm` (they use `catch_unwind` with an
intentional panic, which cannot work on a `panic = abort` target), and 1
becomes the runtime-teardown reason. The net effect on that module is the
4 ignores this layer would have added anyway.
sokra
force-pushed
the
codex/turbopack-wasm/parking-lot-atomics
branch
from
September 12, 2026 16:13
55c346f to
c58ce2a
Compare
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
2 participants
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?
Makes
parking_lotlocks actually block on wasm instead of panicking, and removes 33 of the 35ignores that panic had forced onto the wasm test suites.Why?
parking_lot_corepicks its thread parker inthread_parker/mod.rs. The working wasm parker(
wasm_atomic.rs, built onmemory_atomic_wait32/memory_atomic_notify) is selected only underOtherwise it falls back to
wasm.rs, whosepark()ispanic!("Parking not supported on this platform").wasm32-wasip1-threadsdoes reporttarget_feature = "atomics", so the only missing piece was the feature — every contended lockpanicked, which is why so many tests were ignored on wasm.
How?
Two changes, and neither alone is sufficient:
parking_lot_core0.9.8 → 0.9.12, because 0.9.8's atomic parker needsfeature(stdsimd),removed from Rust long ago;
parking_lot'snightlyfeature for wasm targets only (it forwards toparking_lot_core/nightly). 0.9.12 still gates the atomic parker behind it, so a bump on its ownkeeps selecting the panicking stub silently.
Verified on the resolved feature graph rather than the manifest —
nightlyis present forwasm32-wasip1-threadsand absent for the host.Testing
contended_mutex_blocks_and_hands_overis built so it cannot pass vacuously: a barrier guarantees thewaiter actually contends, the waiter must block for a measurable interval, and it must observe the
holder's write. Negative control: with the feature removed it fails at
parking_lot_core-0.9.12/src/thread_parker/wasm.rs:26with "Parking not supported on this platform".Two of the old ignores remain, under a different and verified cause: their work completes in ~500 ms
of an 800 ms serial sum, so the parallelism is real, but dropping a multi-thread tokio runtime while
their blocking helpers are still alive deadlocks on wasm. That was previously hidden because
panic = abortkilled the process at the failing assertion before teardown ran. It is fixed in thefollow-up runtime layer.
Also fixes a latent bug unrelated to wasm:
test_parallel_try_for_each_mutasserted every element wasincremented, which only holds when the chunk size is 1. Chunking comes from
available_parallelism() * 4and each chunk stops at its first error, so with parallelism 1 the chunksize is 2 and every second item is skipped — it would fail on any single-core machine. It now asserts
the parallelism-independent contract.