Skip to content

Harden nodex-api memory safety at N-API boundaries#5

Merged
uuhan merged 2 commits into
devfrom
copilot/review-memory-safety-issues
Apr 1, 2026
Merged

Harden nodex-api memory safety at N-API boundaries#5
uuhan merged 2 commits into
devfrom
copilot/review-memory-safety-issues

Conversation

Copilot AI commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

This PR addresses unsafe boundary behavior in the Rust ↔ Node-API layer, with focus on memory-safety risks (UB, aliasing, and ownership misuse).
The changes reduce footguns in core handle/data APIs while keeping the surface area mostly intact.

  • Unsafe out-parameter handling (napi_call!(?))

    • Prevents reading uninitialized out-pointers when N-API returns an error.
    • napi_call!(?) now returns (status, Option<value>) and only materializes value on success.
  • Reference ownership correctness (NapiRef)

    • Removed Clone from NapiRef to avoid accidental duplicate ownership of the same napi_ref.
    • This eliminates a double-delete hazard via Drop (napi_delete_reference).
  • Mutability/aliasing API hardening

    • Replaced &self -> &mut T patterns with explicit mutable accessors:
      • JsExternal::get(&self) -> &T + get_mut(&mut self) -> &mut T
      • NapiValueT::unwrap(&self) -> Option<&T> + unwrap_mut(&mut self) -> Option<&mut T>
      • NapiEnv::get_instance_data(&self) -> Option<&T> + get_instance_data_mut(&mut self) -> Option<&mut T>
    • This aligns API contracts with Rust aliasing rules and removes implicit mutable alias creation from immutable receivers.
  • Async work callback type safety

    • Fixed trait-object type mismatch in NapiAsyncWork completion path.
    • complete callback storage/retrieval now uses consistent FnMut(...) -> NapiResult<()> typing.
  • Call-site alignment

    • Updated demo usage to the new explicit mutable instance-data accessor.
// Before
if let Ok(Some(data)) = env.get_instance_data::<usize>() {
    *data = 200;
}

// After
if let Ok(Some(data)) = env.get_instance_data_mut::<usize>() {
    *data = 200;
}

@uuhan uuhan marked this pull request as ready for review April 1, 2026 15:49
@uuhan uuhan merged commit cd01f0b into dev Apr 1, 2026
12 checks passed
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.

2 participants