Skip to content

fix: prevent deadlock in DevEngine::close() by scoping bundler lock#8569

Merged
hyf0 merged 2 commits intoperf/share-tokio-runtime-fixture-testsfrom
copilot/sub-pr-8567
Mar 7, 2026
Merged

fix: prevent deadlock in DevEngine::close() by scoping bundler lock#8569
hyf0 merged 2 commits intoperf/share-tokio-runtime-fixture-testsfrom
copilot/sub-pr-8567

Conversation

Copy link
Contributor

Copilot AI commented Mar 7, 2026

DevEngine::close() held the bundler MutexGuard while awaiting the coordinator task to exit. If the coordinator's Close handler was waiting for a running bundling task — which also needs the bundler lock — this deadlocked, causing CI hangs.

Changes

  • crates/rolldown_dev/src/dev_engine.rs: Scope the bundler lock so it's dropped before coordinator_handle.await. The coordinator's Close handler may need to await an in-flight bundling task that contends on the same lock.
// Before: bundler guard lived until end of close(), including during coordinator await
let mut bundler = self.bundler.lock().await;
bundler.close().await?;
// ... coordinator_handle.await while still holding lock → potential deadlock

// After: explicit scope ensures lock is released before waiting for coordinator
{
    let mut bundler = self.bundler.lock().await;
    bundler.close().await?;
} // lock released here
let coordinator_state = self.coordinator_state.lock().await;
if let Some(coordinator_handle) = coordinator_state.handle.clone() {
    coordinator_handle.await; // safe: bundler lock is free
}
  • crates/rolldown_testing/src/integration_test.rs: Include test_folder_path in the close-failure panic message so failures are attributable to a specific fixture.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…ent deadlock

Co-authored-by: hyf0 <49502170+hyf0@users.noreply.github.com>
Copilot AI changed the title [WIP] WIP address CI hang issue and refine close logic fix: prevent deadlock in DevEngine::close() by scoping bundler lock Mar 7, 2026
Copilot finished work on behalf of hyf0 March 7, 2026 02:08
@hyf0 hyf0 marked this pull request as ready for review March 7, 2026 02:35
Copilot AI review requested due to automatic review settings March 7, 2026 02:35
@hyf0 hyf0 merged commit 9d09474 into perf/share-tokio-runtime-fixture-tests Mar 7, 2026
2 checks passed
@hyf0 hyf0 deleted the copilot/sub-pr-8567 branch March 7, 2026 02:35
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a potential deadlock in DevEngine::close() where the bundler MutexGuard was held across the coordinator_handle.await call. A bundling task spawned by the coordinator (after receiving messages queued before Close) could block indefinitely trying to acquire the same bundler lock, while the coordinator waited for that task to finish.

Changes:

  • Scope the bundler lock in DevEngine::close() so it's released before awaiting the coordinator handle
  • Improve the dev_engine.close() panic message in integration tests to include test_folder_path for better failure attribution

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
crates/rolldown_dev/src/dev_engine.rs Wraps bundler.lock()/bundler.close() in an explicit brace scope so the MutexGuard is dropped before coordinator_handle.await, eliminating the deadlock
crates/rolldown_testing/src/integration_test.rs Adds test_folder_path.display() to the close-failure panic message for easier test debugging

@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2026

Benchmarks Rust

  • target: perf/share-tokio-runtime-fixture-tests(650c9a2)
  • pr: copilot/sub-pr-8567(d34c891)
group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.00     75.8±1.72ms        ? ?/sec    1.07     80.9±1.83ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.00     84.0±1.70ms        ? ?/sec    1.05     88.2±2.12ms        ? ?/sec
bundle/bundle@rome_ts                                        1.00    156.7±3.94ms        ? ?/sec    1.02    159.4±4.65ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.00    173.7±4.81ms        ? ?/sec    1.02    176.3±4.72ms        ? ?/sec
bundle/bundle@threejs                                        1.03     72.2±2.64ms        ? ?/sec    1.00     70.4±2.10ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.01     77.9±1.39ms        ? ?/sec    1.00     77.4±1.18ms        ? ?/sec
bundle/bundle@threejs10x                                     1.00    746.9±5.12ms        ? ?/sec    1.03   769.3±11.53ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.00    856.8±5.56ms        ? ?/sec    1.03    884.1±9.94ms        ? ?/sec
scan/scan@rome_ts                                            1.00     75.2±2.00ms        ? ?/sec    1.04     78.2±1.70ms        ? ?/sec
scan/scan@threejs                                            1.00     26.7±0.42ms        ? ?/sec    1.04     27.8±0.44ms        ? ?/sec
scan/scan@threejs10x                                         1.00    261.6±4.25ms        ? ?/sec    1.04    271.3±3.33ms        ? ?/sec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants