Skip to content

audit: Review sync NAPI bindings for deadlock safety #7311

@hyf0

Description

@hyf0

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:

  1. Rust holds a lock while calling JavaScript hooks
  2. JS hooks call back into Rust (via NAPI binding)
  3. 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

  • remove_client - HIGH priority refactor(dev): make removeClient async #7313
  • register_plugins - MEDIUM priority
  • emit_file - MEDIUM priority
  • emit_chunk - MEDIUM priority
  • get_module_info - LOW priority
  • get_module_ids - LOW priority
  • add_watch_file - LOW priority
  • get_file_name - LOW priority

Related

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

Priority

None yet

Effort

None yet

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions