Background
PR #7289 fixed a deadlock in the dev engine caused by register_modules being a synchronous NAPI function that acquired a DashMap lock. When JavaScript hooks called back into Rust while the lock was held, it caused a deadlock.
Problem
Synchronous NAPI functions that access shared mutable state (DashMap, Mutex, etc.) can cause deadlocks when:
- Rust holds a lock while calling JavaScript hooks
- JS hooks call back into Rust (via NAPI binding)
- That call tries to acquire the same/conflicting lock
Audit Results
The following sync NAPI functions access shared state and need review:
HIGH Risk
| Function |
File |
Issue |
remove_client |
crates/rolldown_binding/src/binding_dev_engine.rs |
Sync function calls .remove() on DashMap |
MEDIUM Risk
| Function |
File |
Issue |
register_plugins |
crates/rolldown_binding/src/parallel_js_plugin_registry.rs |
Sync access to static DashMap |
emit_file |
crates/rolldown_binding/src/binding_plugin_context.rs |
Sync mutation of shared context |
emit_chunk |
crates/rolldown_binding/src/binding_plugin_context.rs |
Sync mutation of shared context |
LOW Risk (monitor)
| Function |
File |
get_module_info |
crates/rolldown_binding/src/binding_plugin_context.rs |
get_module_ids |
crates/rolldown_binding/src/binding_plugin_context.rs |
add_watch_file |
crates/rolldown_binding/src/binding_plugin_context.rs |
get_file_name |
crates/rolldown_binding/src/binding_plugin_context.rs |
new (ParallelJsPluginRegistry) |
crates/rolldown_binding/src/parallel_js_plugin_registry.rs |
Resolution
For each sync NAPI function, one of two actions is needed:
Option A: Make it async
If the function accesses shared state that could be contended during JS hook execution:
#[napi]
#[allow(
clippy::unused_async,
clippy::allow_attributes,
reason = "Lock acquisition. Making async to avoid blocking Node.js thread."
)]
pub async fn function_name(&self, ...) {
// ...
}
Option B: Add SYNC-SAFE comment
If the function is safe to remain sync (e.g., only called during initialization, no lock contention possible):
#[napi]
// SYNC-SAFE: <reason why this won't cause deadlock>
pub fn function_name(&self, ...) {
// ...
}
Tasks
Related
Background
PR #7289 fixed a deadlock in the dev engine caused by
register_modulesbeing a synchronous NAPI function that acquired a DashMap lock. When JavaScript hooks called back into Rust while the lock was held, it caused a deadlock.Problem
Synchronous NAPI functions that access shared mutable state (DashMap, Mutex, etc.) can cause deadlocks when:
Audit Results
The following sync NAPI functions access shared state and need review:
HIGH Risk
remove_clientcrates/rolldown_binding/src/binding_dev_engine.rs.remove()on DashMapMEDIUM Risk
register_pluginscrates/rolldown_binding/src/parallel_js_plugin_registry.rsemit_filecrates/rolldown_binding/src/binding_plugin_context.rsemit_chunkcrates/rolldown_binding/src/binding_plugin_context.rsLOW Risk (monitor)
get_module_infocrates/rolldown_binding/src/binding_plugin_context.rsget_module_idscrates/rolldown_binding/src/binding_plugin_context.rsadd_watch_filecrates/rolldown_binding/src/binding_plugin_context.rsget_file_namecrates/rolldown_binding/src/binding_plugin_context.rsnew(ParallelJsPluginRegistry)crates/rolldown_binding/src/parallel_js_plugin_registry.rsResolution
For each sync NAPI function, one of two actions is needed:
Option A: Make it async
If the function accesses shared state that could be contended during JS hook execution:
Option B: Add SYNC-SAFE comment
If the function is safe to remain sync (e.g., only called during initialization, no lock contention possible):
Tasks
remove_client- HIGH priority refactor(dev): makeremoveClientasync #7313register_plugins- MEDIUM priorityemit_file- MEDIUM priorityemit_chunk- MEDIUM priorityget_module_info- LOW priorityget_module_ids- LOW priorityadd_watch_file- LOW priorityget_file_name- LOW priorityRelated
register_modulesasync #7289 - Fixedregister_modulesdeadlock