Skip to content

fix(ext/napi): don't free threadsafe function on abort while refs remain#36032

Merged
bartlomieju merged 1 commit into
mainfrom
fix/napi-tsfn-abort-double-free
Jul 14, 2026
Merged

fix(ext/napi): don't free threadsafe function on abort while refs remain#36032
bartlomieju merged 1 commit into
mainfrom
fix/napi-tsfn-abort-double-free

Conversation

@bartlomieju

Copy link
Copy Markdown
Member

Fixes the Windows panic reported in #35990:

thread 'main' panicked at ext\napi\node_api.rs:947:5:
assertion failed: self.is_closed.compare_exchange(false, true, Ordering::Relaxed, Ordering::Relaxed).is_ok()

napi_release_threadsafe_function with napi_tsfn_abort freed the TsFn
regardless of the remaining thread_count. When more than one thread still held
the function, aborting spawned the drop immediately, freeing the box out from
under the other reference holders. Those threads were left with a dangling
pointer, and a later release from one of them could deref the freed box and
spawn a second drop of the same allocation, producing a use-after-free and a
double free. On the second drop the is_closed compare-exchange in TsFn::drop
fails and the assertion panics.

Abort now only marks the function as closing (rejecting pending and future calls
and waking any callers blocked on a full queue). The box is freed exactly once,
when the last thread releases it and the thread count reaches zero. This matches
Node.js semantics, where abort stops further calls but destruction still waits
for every acquired thread to release. The non-abort path is unchanged: it
already only freed on the final release.

The reproduction is inherently timing and allocator dependent (the second drop
only trips the assert once the freed memory is observed as reusable), which is
why it surfaced on Windows under a real addon rather than deterministically. The
added regression test creates a tsfn with initial_thread_count = 2, aborts
from one thread while a second reference is still outstanding, and asserts the
finalizer runs exactly once.

napi_release_threadsafe_function with napi_tsfn_abort freed the TsFn
regardless of the remaining thread_count. When more than one thread still
held the function, that left the other threads with a dangling pointer;
a later release from one of them could deref the freed box and spawn a
second drop of the same allocation, causing a use-after-free and a double
free. On the double free the assert in TsFn::drop trips:

  self.is_closed.compare_exchange(false, true, ...).is_ok()

(node_api.rs:947), which is the Windows panic reported in #35990.

Abort now only marks the function as closing (rejecting pending and future
calls and waking blocked callers); the box is freed exactly once, when the
last thread releases it and the thread count reaches zero. This matches
Node.js, where abort stops further calls but destruction still waits for
every acquired thread to release.

Adds a regression test that aborts while a second reference is still
outstanding and asserts the finalizer runs exactly once.
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.

1 participant