fix(dev): make `register_modules` async by hyf0 · Pull Request #7289 · rolldown/rolldown

hyf0 · 2025-12-01T13:13:53Z

The issue is "when dev engine is in the process of generating hmr update, a new file change will cause the nodejs hang during the second hmr update generating"

In short, the hang is caused by deadlock.

Details

Rolldown stores the client information into client variable using DashMap, which is basically equivalent to Mutex<HashMap>.

During the process of generating hmr update

rolldown/crates/rolldown_dev/src/bundling_task.rs

Line 140 in 5ef49ad

let client_sessions: Vec<_> = self.dev_context.clients.iter().collect();

this line of code is equivalent to client.lock(), will means the lock of client will be hold during the whole generating process.

The first hmr update is generated successfully and is sent to vite to trigger the hmr process.

The browser loads the hmr patch and and sends message to vite node to register the new loaded module via code

rolldown/crates/rolldown_binding/src/binding_dev_engine.rs

Lines 182 to 184 in 0ce4a17

    
           pub fn register_modules(&self, client_id: String, modules: Vec<String>) { 
        
             self.inner.clients.entry(client_id).or_default().executed_modules.extend(modules); 
        
           }

nodejs calls register_modules

rolldown/crates/rolldown_binding/src/binding_dev_engine.rs

Line 183 in 0ce4a17

    
           self.inner.clients.entry(client_id).or_default().executed_modules.extend(modules);

This line of is also equivalent to client.lock().

In the meantime, dev engine is in the second hmr update generation, which holds the client lock already.

So nodejs needs to wait for this lock to get free, and this wait is a SYNCHRONOUS wait. The nodejs itself doesn't have chance to resolve pending promises anymore(like resolving a timeout timer), while the hmr generating is awaiting js transform hook to be finished.

All these things together cause a deadlock situation.

If we have #7287 in the first place, it will be much easier to locate the problem, becuase we could easily find out which function is the last called function from binding.js.

Visual Explanation of the Deadlock

The Circular Dependency

    ┌──────────────────────────────────────────────────────────┐
    │                                                          │
    ▼                                                          │
┌────────────┐         ┌────────────┐         ┌────────────┐   │
│  Task 2    │ awaits  │  Node.js   │  waits  │   Lock     │   │
│  (Rust)    │────────►│  transform │────────►│ (held by   │───┘
│            │         │  hook      │  SYNC   │  Task 2)   │
└────────────┘         └────────────┘         └────────────┘
      │                      ▲
      │                      │
      │    register_modules  │
      │    called by browser │
      │    (from Task 1's    │
      │     HMR patch)       │
      └──────────────────────┘

Sequence of Events

1. Task 1: Generate HMR #1 → Send to browser → Complete ✓
2. File changes again → Task 2 starts
3. Task 2: Acquire clients lock
4. Task 2: Call JS transform hook (await)
5. Browser: Receives HMR #1, loads patch, calls register_modules
6. Node.js: register_modules tries to acquire lock (SYNC)
7. Node.js: BLOCKED waiting for lock (Task 2 has it)
8. Task 2: Waiting for JS promise to resolve
9. Node.js: Can't process promises (event loop blocked)
10. DEADLOCK!

The Fix

Making register_modules async allows Node.js to yield control when waiting for the lock, keeping the event loop alive so it can resolve the transform hook promise.

Before (sync)	After (async)
Node.js blocks entirely waiting for lock	Node.js yields control back to event loop
Event loop frozen, can't process callbacks	Event loop continues, can process transform callback
DEADLOCK	Works correctly

hyf0 · 2025-12-01T13:14:08Z

fix(dev): make register_modules async #7289 👈 (View in Graphite)
main

How to use the Graphite Merge Queue

Add the label graphite: merge 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.

netlify · 2025-12-01T13:15:05Z

❌ Deploy Preview for rolldown-rs failed.

Name	Link
🔨 Latest commit	`13ea88c`
🔍 Latest deploy log	https://app.netlify.com/projects/rolldown-rs/deploys/692dabf044f6ab0008563410

github-actions · 2025-12-01T13:29:21Z

Benchmarks Rust

target: main(36bdcc4)
pr: 12-01-fix_dev_make_register_modules_async(13ea88c)

group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.03     75.3±1.93ms        ? ?/sec    1.00     72.9±2.66ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.02     82.1±3.21ms        ? ?/sec    1.00     80.8±3.84ms        ? ?/sec
bundle/bundle@rome_ts                                        1.03    116.8±3.10ms        ? ?/sec    1.00    113.0±2.22ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.03    130.1±3.28ms        ? ?/sec    1.00    126.2±2.40ms        ? ?/sec
bundle/bundle@threejs                                        1.00     41.8±2.70ms        ? ?/sec    1.00     41.6±1.19ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.01     45.6±1.69ms        ? ?/sec    1.00     45.2±0.94ms        ? ?/sec
bundle/bundle@threejs10x                                     1.03   424.4±10.91ms        ? ?/sec    1.00    411.3±7.71ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.00    477.5±7.56ms        ? ?/sec    1.01    480.7±9.80ms        ? ?/sec
scan/scan@rome_ts                                            1.02     91.0±3.33ms        ? ?/sec    1.00     89.2±2.67ms        ? ?/sec
scan/scan@threejs                                            1.00     29.2±0.52ms        ? ?/sec    1.04     30.2±0.93ms        ? ?/sec
scan/scan@threejs10x                                         1.00    307.8±5.81ms        ? ?/sec    1.02    314.0±7.04ms        ? ?/sec

Copilot

Pull request overview

This PR fixes a deadlock issue in the dev engine by making the register_modules method asynchronous. The deadlock occurred when the dev engine held a lock on the clients DashMap during HMR update generation while Node.js synchronously waited to acquire the same lock in register_modules, preventing the event loop from resolving pending promises needed by the HMR generation process.

Key Changes:

Changed register_modules from synchronous to async to prevent blocking the Node.js thread when acquiring the DashMap lock
Added clippy allow attributes with explanatory comment documenting the deadlock prevention rationale

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

crates/rolldown_binding/src/binding_dev_engine.rs

graphite-app · 2025-12-01T14:53:00Z

Merge activity

Dec 1, 2:52 PM UTC: hyf0 added this pull request to the Graphite merge queue.
Dec 1, 3:09 PM UTC: Merged by the Graphite merge queue.

The issue is "when dev engine is in the process of generating hmr update, a new file change will cause the nodejs hang during the second hmr update generating" In short, the hang is caused by deadlock. ## Details Rolldown stores the client information into `client` variable using `DashMap`, which is basically equivalent to `Mutex<HashMap>`. During the process of generating hmr update https://github.com/rolldown/rolldown/blob/5ef49ad615cfef1a9ebc97368546e1b9adbaf48d/crates/rolldown_dev/src/bundling_task.rs#L140 this line of code is equivalent to `client.lock()`, will means the lock of `client` will be hold during the whole generating process. The first hmr update is generated successfully and is sent to vite to trigger the hmr process. The browser loads the hmr patch and and sends message to vite node to register the new loaded module via code https://github.com/rolldown/rolldown/blob/0ce4a17c5ae1a95e331f8d38c5230742b09d1fd3/crates/rolldown_binding/src/binding_dev_engine.rs#L182-L184 nodejs calls `register_modules` https://github.com/rolldown/rolldown/blob/0ce4a17c5ae1a95e331f8d38c5230742b09d1fd3/crates/rolldown_binding/src/binding_dev_engine.rs#L183 This line of is also equivalent to `client.lock()`. In the meantime, dev engine is in the second hmr update generation, which holds the client lock already. So nodejs needs to wait for this lock to get free, and this wait is a **SYNCHRONOUS** wait. The nodejs itself doesn't have chance to resolve pending promises anymore(like resolving a timeout timer), while the hmr generating is awaiting js `transform` hook to be finished. All these things together cause a deadlock situation. --- If we have #7287 in the first place, it will be much easier to locate the problem, becuase we could easily find out which function is the last called function from `binding.js`. --- # Visual Explanation of the Deadlock ### The Circular Dependency ``` ┌──────────────────────────────────────────────────────────┐ │ │ ▼ │ ┌────────────┐ ┌────────────┐ ┌────────────┐ │ │ Task 2 │ awaits │ Node.js │ waits │ Lock │ │ │ (Rust) │────────►│ transform │────────►│ (held by │───┘ │ │ │ hook │ SYNC │ Task 2) │ └────────────┘ └────────────┘ └────────────┘ │ ▲ │ │ │ register_modules │ │ called by browser │ │ (from Task 1's │ │ HMR patch) │ └──────────────────────┘ ``` ### Sequence of Events ``` 1. Task 1: Generate HMR #1 → Send to browser → Complete ✓ 2. File changes again → Task 2 starts 3. Task 2: Acquire clients lock 4. Task 2: Call JS transform hook (await) 5. Browser: Receives HMR #1, loads patch, calls register_modules 6. Node.js: register_modules tries to acquire lock (SYNC) 7. Node.js: BLOCKED waiting for lock (Task 2 has it) 8. Task 2: Waiting for JS promise to resolve 9. Node.js: Can't process promises (event loop blocked) 10. DEADLOCK! ``` ### The Fix Making `register_modules` async allows Node.js to **yield control** when waiting for the lock, keeping the event loop alive so it can resolve the transform hook promise. | Before (sync) | After (async) | |---------------|---------------| | Node.js **blocks entirely** waiting for lock | Node.js **yields** control back to event loop | | Event loop frozen, can't process callbacks | Event loop continues, can process transform callback | | **DEADLOCK** | **Works correctly** |

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@sapphi-red

## [1.0.0-beta.53] - 2025-12-03 💥 Breaking Changes - Drop `i686-pc-windows-msvc` target support 🚀 Chunk Merging Optimization - Rolldown now automatically merges shared chunks when entries import each other (when `preserveEntrySignature` is not `strict`) ```shell Before: entry.js → imports → shared.js (common chunk) entry2.js → imports → shared.js Output: 3 chunks (entry.js, entry2.js, shared.js) After: entry.js → contains shared code entry2.js → imports → entry.js Output: 2 chunks (entry.js, entry2.js) ``` ### 💥 BREAKING CHANGES - drop `i686-pc-windows-msvc` target support (#7230) by @sapphi-red ### 🚀 Features - rolldown_plugin_vite_manifest: pass normalized options to `isLegacy` callback (#7321) by @shulaoda - plugin/vite-resolve: add `disableCache` option (#6763) by @sapphi-red - rolldown: export `createTokioRuntime` for tsdown (#7264) by @shulaoda - rolldown_plugin_vite_html: sync `moduleSideEffects` for already loaded modules (#7254) by @shulaoda - rolldown_plugin_vite_html: load module scripts with side effects to prevent tree-shaking (#7244) by @shulaoda - rolldown_plugin_vite_css_post: implement `cssScopeTo` for scoped CSS tree-shaking (#7240) by @shulaoda ### 🐛 Bug Fixes - export default class decl __name runtime insertion (#7316) by @IWANABETHATGUY - chunk side effects calculation (#7273) by @IWANABETHATGUY - node: `output.generateCode.preset: 'es2015'` should set `output.generateCode.symbols: true` by default (#7314) by @sapphi-red - skip name helper for classes with static name property (#7312) by @IWANABETHATGUY - preserve chunk imports relationship after chunk merging (#7303) by @shulaoda - dev: make `register_modules` async (#7289) by @hyf0 - preserve computed property in object destructuring (#7288) by @IWANABETHATGUY - support dynamic imports with shared dependencies (#7261) by @IWANABETHATGUY - call `defer_sync_scan_data` in non-incremental build mode (#7255) by @shulaoda - optimize chunk merging for shared entry points (#7194) by @IWANABETHATGUY - add indentation for UMD format output (#7263) by @IWANABETHATGUY - rolldown_plugin_vite_css_post: pass options to `isLegacy` callback for proper legacy detection (#7260) by @shulaoda - rolldown_plugin_vite_css_post: also detect `?inline=true` query for inlined CSS (#7245) by @shulaoda - rolldown_plugin_vite_css_post: distinguish empty CSS from no CSS (#7241) by @shulaoda - add Windows support for t-run command (#7242) by @IWANABETHATGUY - cjs: prevent duplicate require declarations for external modules with preserveModules (#7234) by @logaretm - rolldown_plugin_vite_resolve: resolve from root for virtual modules (#7236) by @sapphi-red - include entry level external modules in chunk exports (#7218) by @IWANABETHATGUY ### 🚜 Refactor - dev: make `removeClient` async (#7313) by @hyf0 - move chunk merging code out of code_splitting.rs (#7285) by @IWANABETHATGUY - extract common function util for chunk merging (#7271) by @IWANABETHATGUY - use iterative method to merge chunks (#7256) by @IWANABETHATGUY - use concat_string! instead of string replace for generating chunk level exports (#7247) by @IWANABETHATGUY ### 📚 Documentation - add warning to experimental.resolveNewUrlToAsset about JS/TS files (#7300) by @Copilot - add sequential hook execution difference in plugin-api.md (#7308) by @Copilot - add migration example from onwarn to onLog (#7299) by @Copilot - add migration example for manualChunks to advancedChunks (#7298) by @Copilot - deps: bump vitepress to fix build (#7307) by @sapphi-red - examples & text for experimental.resolveNewUrlToAsset (#7259) by @TheAlexLichter ### ⚡ Performance - rolldown_plugin_vite_css_post: lazily load `cssScopeTo` from JS module options (#7253) by @shulaoda - rolldown_plugin_vite_css_post: avoid unnecessary string clones in `resolve_asset_urls_in_css` (#7250) by @shulaoda ### 🧪 Testing - generate relative path like name in advanced chunks (#7267) by @IWANABETHATGUY - add test case for preserveEntrySignatures with re-exports (#7279) by @IWANABETHATGUY - add test262 integration tests (#7196) by @sapphi-red ### ⚙️ Miscellaneous Tasks - deps: update dependency rolldown-plugin-dts to v0.18.1 (#7304) by @renovate[bot] - enable tracing feature for napi (#7322) by @sapphi-red - deps: update napi (#7320) by @renovate[bot] - deps: update oxc (#7318) by @renovate[bot] - deps: update napi (#7317) by @renovate[bot] - deps: update oxc to v0.100.0 (#7301) by @renovate[bot] - deps: downgrade pnpm to 10.23.0 to fix Netlify build (#7306) by @shulaoda - add `trustPolicyExclude` for chokidar and semver (#7302) by @sapphi-red - update pnpm lockfile (#7291) by @IWANABETHATGUY - deps: update npm packages (#7272) by @renovate[bot] - deps: update rust crates (#7270) by @renovate[bot] - deps: update oxc (#7262) by @renovate[bot] - deps: update github-actions (#7269) by @renovate[bot] - deps: update dependency dprint-typescript to v0.95.13 (#7268) by @renovate[bot] - deps: update `html5gum` to 0.8.1 (#7265) by @shulaoda - rolldown: remove unused `getModuleOptions` from `PluginContext` (#7266) by @shulaoda - remove unnecessary justfile ignore (#7243) by @IWANABETHATGUY - deps: update oxc apps (#7238) by @renovate[bot] - add `nul` to workaround https://github.com/anthropics/claude-c… (#7237) by @IWANABETHATGUY - deps: update dependency valibot to v1.2.0 [security] (#7231) by @renovate[bot] - deps: update crate-ci/typos action to v1.40.0 (#7232) by @renovate[bot] ### ❤️ New Contributors * @logaretm made their first contribution in [#7234](#7234) Co-authored-by: shulaoda <[email protected]>

close #7311 refs #7289 ~~TODO: try this PR in more places.~~

github-actions bot assigned hyf0 Dec 1, 2025

hyf0 marked this pull request as ready for review December 1, 2025 13:27

Copilot AI review requested due to automatic review settings December 1, 2025 13:28

Copilot started reviewing on behalf of hyf0 December 1, 2025 13:28 View session

Copilot finished reviewing on behalf of hyf0 December 1, 2025 13:29

Copilot AI reviewed Dec 1, 2025

View reviewed changes

crates/rolldown_binding/src/binding_dev_engine.rs Show resolved Hide resolved

hyf0 force-pushed the 12-01-fix_dev_make_register_modules_async branch from 17bcbf0 to 15213a6 Compare December 1, 2025 13:42

hyf0 requested a review from sapphi-red December 1, 2025 14:16

sapphi-red approved these changes Dec 1, 2025

View reviewed changes

Copilot AI review requested due to automatic review settings December 1, 2025 14:53

graphite-app bot force-pushed the 12-01-fix_dev_make_register_modules_async branch from 15213a6 to 13ea88c Compare December 1, 2025 14:53

Copilot AI reviewed Dec 1, 2025

View reviewed changes

hyf0 added the graphite: merge label Dec 1, 2025

graphite-app bot merged commit 13ea88c into main Dec 1, 2025
30 of 34 checks passed

graphite-app bot removed the graphite: merge label Dec 1, 2025

graphite-app bot deleted the 12-01-fix_dev_make_register_modules_async branch December 1, 2025 15:09

hyf0 mentioned this pull request Dec 2, 2025

audit: Review sync NAPI bindings for deadlock safety #7311

Closed

8 tasks

This was referenced Dec 3, 2025

release: v1.0.0-beta.53 #7319

Closed

release: v1.0.0-beta.53 #7324

Merged

sapphi-red mentioned this pull request Feb 17, 2026

refactor: ban holding DashMap refs across awaits #8362

Merged

graphite-app bot pushed a commit that referenced this pull request Feb 19, 2026

refactor: ban holding DashMap refs across awaits (#8362)

edaac76

close #7311 refs #7289 ~~TODO: try this PR in more places.~~

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

fix(dev): make `register_modules` async#7289

fix(dev): make `register_modules` async#7289
graphite-app[bot] merged 1 commit intomainfrom
12-01-fix_dev_make_register_modules_async

hyf0 commented Dec 1, 2025 •

edited

Loading

Uh oh!

hyf0 commented Dec 1, 2025

Uh oh!

netlify bot commented Dec 1, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Dec 1, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

graphite-app bot commented Dec 1, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	pub fn register_modules(&self, client_id: String, modules: Vec<String>) {
	self.inner.clients.entry(client_id).or_default().executed_modules.extend(modules);
	}

Comments

Conversation

hyf0 commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Details

Visual Explanation of the Deadlock

The Circular Dependency

Sequence of Events

The Fix

Uh oh!

hyf0 commented Dec 1, 2025

How to use the Graphite Merge Queue

Uh oh!

netlify bot commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ Deploy Preview for rolldown-rs failed.

Uh oh!

github-actions bot commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks Rust

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

graphite-app bot commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hyf0 commented Dec 1, 2025 •

edited

Loading

netlify bot commented Dec 1, 2025 •

edited

Loading

github-actions bot commented Dec 1, 2025 •

edited

Loading

graphite-app bot commented Dec 1, 2025 •

edited

Loading