fix: pair tokio runtime acquire and release on wasm to prevent premature shutdown#10363
Merged
graphite-app[bot] merged 1 commit intoJul 22, 2026
Conversation
Member
Author
How to use the Graphite Merge QueueAdd the label graphite: merge-when-ready to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
✅ Deploy Preview for rolldown-rs canceled.
|
shulaoda
force-pushed
the
07-20-fix_pair_tokio_runtime_acquire_and_release_on_wasm_to_prevent_premature_shutdown
branch
from
July 20, 2026 08:58
be5c42d to
8c5f077
Compare
@rolldown/browser
@rolldown/debug
rolldown
@rolldown/binding-android-arm64
@rolldown/binding-darwin-arm64
@rolldown/binding-darwin-x64
@rolldown/binding-freebsd-x64
@rolldown/binding-linux-arm-gnueabihf
@rolldown/binding-linux-arm64-gnu
@rolldown/binding-linux-arm64-musl
@rolldown/binding-linux-ppc64-gnu
@rolldown/binding-linux-s390x-gnu
@rolldown/binding-linux-x64-gnu
@rolldown/binding-linux-x64-musl
@rolldown/binding-openharmony-arm64
@rolldown/binding-wasm32-wasi
@rolldown/binding-win32-arm64-msvc
@rolldown/binding-win32-x64-msvc
commit: |
Merging this PR will not alter performance
Comparing 
Footnotes
|
shulaoda
force-pushed
the
07-20-fix_pair_tokio_runtime_acquire_and_release_on_wasm_to_prevent_premature_shutdown
branch
from
July 22, 2026 02:15
1821c51 to
6535e5d
Compare
IWANABETHATGUY
approved these changes
Jul 22, 2026
Member
|
One small thing on the It's not a regression (the old code released after the awaits too), but since the whole point here is to make acquire/release strictly paired, it'd be nice to actually guarantee the release fires. A async close(): Promise<void> {
const shouldRelease = !this.#asyncRuntimeReleased;
this.#asyncRuntimeReleased = true;
try {
await this.#stopWorkers?.();
await this.#bundler.close();
this.#stopWorkers = void 0;
} finally {
if (shouldRelease) shutdownAsyncRuntime();
}
}
|
hyfdev
approved these changes
Jul 22, 2026
Member
There was a problem hiding this comment.
The fix correctly pairs every startAsyncRuntime() with a single shutdownAsyncRuntime() on all code paths. With the count now seeded at zero, the runtime lives exactly as long as its holders — no phantom initial owner, no premature shutdown, no lost consumers. Removing the static flag and conditional re-acquire is a clean side benefit.
Each consumer guards against double-release with a synchronous claim before the first await. The scan() cleanup is properly idempotent for the error path that reaches it from both catch and finally. All existing lifecycle tests pass.
Member
|
Blocker: add a test case for FBM with WASI #8411 |
Member
Author
I will open a new PR in this stack |
Member
Author
Merge activity
|
…ure shutdown (#10363) ### Description Fixes #8411. Fixes #8747. On the wasm target the binding keeps a refcount (`ACTIVE_TASK_COUNT`) over the shared tokio runtime, and the runtime is torn down when the count reaches zero. The count was seeded at `1` and released by `RolldownBuild.close()`, `Watcher.close()` and `scan()` without any matching acquire, while `DevEngine` never touched it at all. As a result, the first `close()` in the process (for example Vite bundling `vite.config.ts` or the dep optimizer) dropped the count to zero and shut the runtime down under every remaining consumer. The next spawn then panicked with `Access tokio runtime failed in spawn`, which surfaces on StackBlitz as `RuntimeError: unreachable`. This PR makes the refcount strictly paired. Every JS object that outlives a single call and spawns onto the runtime now acquires on create and releases on close: - `ACTIVE_TASK_COUNT` is seeded at `0`, so the runtime lives exactly as long as its holders. - `RolldownBuild` acquires in the constructor and releases in `close()`. The `asyncRuntimeShutdown` static flag and the conditional re-start in `#build` are removed since they only existed to work around the unbalanced count. - `Watcher` acquires in the constructor. It previously only released. - `DevEngine` acquires in `create()` and releases in `close()`. It previously did neither, which is the direct cause of #8411. - `scan()` acquires before scanning and releases in `cleanup()`, which is now idempotent because the error path reaches it from both `catch` and `finally`. Double release is also guarded. `RolldownBuild.close()` and `DevEngine.close()` claim the release before their first await, so a second or concurrent `close()` cannot release twice, and the release happens only after `BindingBundler.close()` settles because that call itself spawns onto the runtime. All of this is a no-op on native targets, where `startAsyncRuntime` and `shutdownAsyncRuntime` compile to empty functions. `binding.d.cts` is regenerated from the updated doc comments. <!-- - What is this PR solving? Write a clear and concise description. - Reference the issues it solves (e.g. `fixes #123`). - What other alternatives have you explored? - Are there any parts you think require more attention from reviewers? Also, please make sure you do the following: - Read the Contributing Guidelines at https://rolldown.rs/contribution-guide/. - Check that there isn't already a PR that solves the problem the same way. If you find a duplicate, please help us review it. - Update the corresponding documentation if needed. - Include relevant tests that fail without this PR but pass with it. If the tests are not included, explain why. Thank you for contributing to Rolldown! -->
graphite-app
Bot
force-pushed
the
07-20-fix_pair_tokio_runtime_acquire_and_release_on_wasm_to_prevent_premature_shutdown
branch
from
July 22, 2026 06:46
6535e5d to
77c8ff9
Compare
graphite-app
Bot
deleted the
07-20-fix_pair_tokio_runtime_acquire_and_release_on_wasm_to_prevent_premature_shutdown
branch
July 22, 2026 06:54
graphite-app Bot
pushed a commit
that referenced
this pull request
Jul 22, 2026
### Description Follow-up to #10363, which must land first. Adds regression coverage for the `Access tokio runtime failed in spawn` crash class fixed there. This bug class was invisible to every existing CI test. The runtime acquire and release functions are no-ops on native targets, and no CI test actually executed the WASI binding: the WASI workflow builds the wasm binding and runs the stability test and the examples, but those load the native binding that the same job builds for tooling. So an unbalanced runtime release could only ever crash on StackBlitz, never in CI. The new `tests/wasi/runtime-lifecycle.mjs` is a plain node script following the `test:stability` pattern, wired into the WASI workflow right after the build step. It forces the real wasm binding and covers three sequences: - The #8411 shape: a build that gets closed (Vite bundling `vite.config.ts`), then a `DevEngine` is created and run. - The #8747 shape: a build that gets closed, then `watch()` runs until `BUNDLE_END`. - The invariant behind both: closing one consumer while a `DevEngine` is still alive must not tear the runtime down under the engine. Two details are load-bearing. `NAPI_RS_FORCE_WASI` is set to `error` inside the script before importing rolldown, because `true` silently falls back to the native binding when the wasi files are missing and the test would pass without testing anything. After the import the script also asserts via `require.cache` that `rolldown-binding.wasi.cjs` was really loaded. ### Verification Red and green were produced under identical conditions (same machine, same stable toolchain, napi 3.11.0 on both sides, wasm binding forced and asserted). On `main` (`aa73a6848`, without #10363) the test exits 1 in the first sequence with the exact issue signature: `Access tokio runtime failed in spawn` in napi's `tokio_runtime.rs`, followed by `RuntimeError: unreachable`. On top of #10363 all three sequences pass. To run locally: `pnpm build-wasi:debug` in `packages/rolldown` (dist only contains the wasi files when built with `TARGET=rolldown-wasi`), then `pnpm test:wasi-runtime` in `packages/rolldown/tests`. <!-- - What is this PR solving? Write a clear and concise description. - Reference the issues it solves (e.g. `fixes #123`). - What other alternatives have you explored? - Are there any parts you think require more attention from reviewers? Also, please make sure you do the following: - Read the Contributing Guidelines at https://rolldown.rs/contribution-guide/. - Check that there isn't already a PR that solves the problem the same way. If you find a duplicate, please help us review it. - Update the corresponding documentation if needed. - Include relevant tests that fail without this PR but pass with it. If the tests are not included, explain why. Thank you for contributing to Rolldown! -->
graphite-app Bot
pushed a commit
that referenced
this pull request
Jul 22, 2026
### Description Follow-up to #10363, addressing [this review comment](#10363 (comment)). #10363 made the wasm tokio runtime refcount strictly paired: acquire on create, release on close. But every release still ran as a plain statement after the awaited close calls. If any of those awaits rejects, the release is skipped and the acquire leaks. The count then stays above zero, the runtime is never torn down, and on wasm the park threads hang forever, which is the #8747 symptom in reverse. This is not a regression, the old code had the same shape, but it leaves the pairing guaranteed only on the success path. This PR wraps each release in `try/finally` so it fires no matter how the close settles: - `RolldownBuild.close()` - `DevEngine.close()` - the idempotent `cleanup()` in `scan()` - `Watcher.close()`, which the comment did not name but has the identical pattern The documented ordering is unchanged. `finally` runs only after the awaited close settles, whether it resolves or rejects, so the release still happens strictly after `BindingBundler.close()` is done spawning onto the runtime. The claim-before-first-await guard against double release is also untouched, and the whole change remains a no-op on native targets, where `startAsyncRuntime` and `shutdownAsyncRuntime` compile to empty functions. One pre-existing gap is deliberately left alone: when an early step rejects (for example `stopWorkers()`), the later close calls in the same method are still skipped. This PR only guarantees the runtime release. Making every cleanup step run independently is a separate concern. <!-- - What is this PR solving? Write a clear and concise description. - Reference the issues it solves (e.g. `fixes #123`). - What other alternatives have you explored? - Are there any parts you think require more attention from reviewers? Also, please make sure you do the following: - Read the Contributing Guidelines at https://rolldown.rs/contribution-guide/. - Check that there isn't already a PR that solves the problem the same way. If you find a duplicate, please help us review it. - Update the corresponding documentation if needed. - Include relevant tests that fail without this PR but pass with it. If the tests are not included, explain why. Thank you for contributing to Rolldown! -->
Brooooooklyn
added a commit
that referenced
this pull request
Jul 22, 2026
Catches the branch up to main so CI can run again (the PR was CONFLICTING against the advanced base) and pulls in the fixes that target this branch's red dev-server tests: #10380 test(dev): expect preserved hot.data in hmr-whole-chain-dispose — Vite's rolldown-canary client now preserves import.meta.hot.data; this branch had the stale pre-#10380 assertion. Taken from main. #10363/#10381 tokio runtime acquire/release pairing + release-on-reject. Conflicts resolved to this branch's side (--ours) — the branch's shared tokio-free runtime supersedes main's tokio-refcount work: - dev-engine.ts / experimental.ts / rolldown-build.ts / watch/watcher.ts: main wraps shutdownAsyncRuntime() in try/finally (#10381). This branch already releases via the RuntimeLease abstraction inside a `finally` (release-on-reject guaranteed) plus AggregateError cleanup — a strict superset, so #10381 is subsumed. Verified main touched only the lease/startAsyncRuntime lines in these files. - crates/rolldown_binding/src/lib.rs + generated binding.d.cts: main adds the ACTIVE_TASK_COUNT tokio refcount to start/shutdown_async_runtime (#10363). This branch removed tokio, so those exports are compatibility no-ops (napi env lifecycle owns teardown; the legacy tokio-WASI path uses the old binding's own code). Verified main's changes here are entirely the refcount + its doc comments. No lockfile/catalog conflicts: main touched no manifests in aa73a68..c07a55c. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda
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
4 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.
Description
Fixes #8411. Fixes #8747.
On the wasm target the binding keeps a refcount (
ACTIVE_TASK_COUNT) over the shared tokio runtime, and the runtime is torn down when the count reaches zero. The count was seeded at1and released byRolldownBuild.close(),Watcher.close()andscan()without any matching acquire, whileDevEnginenever touched it at all. As a result, the firstclose()in the process (for example Vite bundlingvite.config.tsor the dep optimizer) dropped the count to zero and shut the runtime down under every remaining consumer. The next spawn then panicked withAccess tokio runtime failed in spawn, which surfaces on StackBlitz asRuntimeError: unreachable.This PR makes the refcount strictly paired. Every JS object that outlives a single call and spawns onto the runtime now acquires on create and releases on close:
ACTIVE_TASK_COUNTis seeded at0, so the runtime lives exactly as long as its holders.RolldownBuildacquires in the constructor and releases inclose(). TheasyncRuntimeShutdownstatic flag and the conditional re-start in#buildare removed since they only existed to work around the unbalanced count.Watcheracquires in the constructor. It previously only released.DevEngineacquires increate()and releases inclose(). It previously did neither, which is the direct cause of [Panic]: when using bundledDev in stackblitz #8411.scan()acquires before scanning and releases incleanup(), which is now idempotent because the error path reaches it from bothcatchandfinally.Double release is also guarded.
RolldownBuild.close()andDevEngine.close()claim the release before their first await, so a second or concurrentclose()cannot release twice, and the release happens only afterBindingBundler.close()settles because that call itself spawns onto the runtime.All of this is a no-op on native targets, where
startAsyncRuntimeandshutdownAsyncRuntimecompile to empty functions.binding.d.ctsis regenerated from the updated doc comments.