fix(watch): avoid deadlock when closing from event callback#9475
Open
DaZuiZui wants to merge 1 commit into
Open
fix(watch): avoid deadlock when closing from event callback#9475DaZuiZui wants to merge 1 commit into
DaZuiZui wants to merge 1 commit into
Conversation
Merging this PR will not alter performance
Comparing 
Footnotes
|
hyf0
added a commit
that referenced
this pull request
Jun 23, 2026
## Summary - add an out-of-band close notification so the coordinator remains responsive while awaiting consumer event callbacks - stop waiting for an in-flight `event`, `change`, or `restart` callback when that callback closes its own watcher, then run the normal complete close sequence - preserve sequential async listener behavior and the contract that `await watcher.close()` settles only after hooks, bundler cleanup, and the `close` event finish - document the re-entrant close lifecycle and add an end-to-end regression test ## Root cause The coordinator awaited the JavaScript event callback, while a callback awaiting `watcher.close()` waited for that same coordinator to finish: ```text coordinator -> JS callback -> watcher.close() -> coordinator ``` The queued `WatcherMsg::Close` could not be processed until the callback returned, so neither side could make progress. ## Design `Watcher::close()` now both queues the normal state-machine message and wakes a dedicated close notification. Consumer callback dispatch waits on the callback and that notification together. If close wins, only the Rust-side wait for the callback is dropped; the JavaScript promise continues running while the coordinator executes the existing close hooks, bundler cleanup, and close event. The original close promise then resolves and the callback resumes. This differs from the request-only approach in #9475: a re-entrant `close()` does not resolve early before cleanup has completed. Fixes #9462. ## Tests - `just build-rolldown` - `just build-rolldown-test-dev-server` - `RUST_BACKTRACE=1 ROLLDOWN_TEST=1 pnpm --filter rolldown-tests test:watcher` (36 passed) - `cargo test -p rolldown_watcher --lib` (17 passed) - `cargo clippy -p rolldown_watcher --all-targets -- --deny warnings` - `just lint-repo` - `vp check`
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.
Root Cause
watcher.close()deadlocked when it was awaited from inside a watcher event callback.The watch coordinator emits events by awaiting the JS listener. In the reported reproduction, the listener receives
BUNDLE_ENDand then awaitswatcher.close(). The oldclose()implementation sentWatcherMsg::Closeand immediately waited for the coordinator future to finish.That creates a re-entrant wait cycle:
watcher.close()to resolve;watcher.close()is waiting for the same coordinator to finish;So the close request was queued correctly, but the coordinator never got a chance to observe it.
Fix
This PR separates "request close" from "wait until closed" for this re-entrant path.
request_close()method in the Rust watcher and expose it to JS asrequestClose().WatcherEmitteris currently dispatching an event callback.watcher.close()is called during event dispatch, stop JS workers and callrequestClose()without awaiting the coordinator from inside the callback.watcher.close()calls outside event callbacks.watcher.close()insideBUNDLE_END.Fixes #9462.
Tests
PATH="$HOME/.cargo/bin:$PATH" just build-rolldownPATH="$HOME/.cargo/bin:$PATH" cargo test -p rolldown_watcher --libPATH="$HOME/.cargo/bin:$PATH" RUST_BACKTRACE=1 ROLLDOWN_TEST=1 pnpm --filter rolldown-tests exec vitest run watch/watch.test.ts --reporter verbose --hideSkippedTestsawait watcher.close()inside aBUNDLE_ENDcallbackPATH="$HOME/.cargo/bin:$PATH" just lint-repoPATH="$HOME/.cargo/bin:$PATH" just rollreachedlint-publint; Rust tests, Node tests, watch/dev-watch tests, Rollup compatibility tests, formatting, and base lint passed. The final local failure was@rolldown/browserpublint becausepackages/browser/dist/*was unavailable; rebuilding it was blocked by the local Rust toolchain not being able to installwasm32-wasip1-threadsstd for Rust 1.95.0.