Skip to content

node-api: execute tsfn finalizer after queue drains when aborted#61956

Open
KevinEady wants to merge 3 commits intonodejs:mainfrom
KevinEady:fix-tsfn-finalizer-and-queue-drain-order
Open

node-api: execute tsfn finalizer after queue drains when aborted#61956
KevinEady wants to merge 3 commits intonodejs:mainfrom
KevinEady:fix-tsfn-finalizer-and-queue-drain-order

Conversation

@KevinEady
Copy link
Contributor

@KevinEady KevinEady commented Feb 23, 2026

Execute a threadsafe function's finalizer after the queue drains. It may be the case that the finalizer will free the context, which might be needed in order to properly clean up the data in the call_js callback during abort. If the finalizer runs before the queue drains, it may point to invalid memory.

@KevinEady
node-api: execute tsfn finalizer after queue drains when aborted
e522e44 
A threadsafe function may utilize its context in its `call_js` callback.
The context should be valid during draining of the queue when the
threadsafe function is aborted.

Fixes: nodejs#60026
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 23, 2026

Review requested:

  • @nodejs/gyp
  • @nodejs/node-api

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. node-api Issues and PRs related to the Node-API. labels Feb 23, 2026
@KevinEady
Copy link
Contributor Author

KevinEady commented Feb 23, 2026 •
edited
Loading

Without the fix commit, the new test fails:

Assertion failed: (ctx->state == Context::State::kCalled), function tsfn_finalize, file binding.cc, line 51.
[1]    89473 abort      out/Debug/node test/node-api/test_threadsafe_function_abort/test.js

* thread #1, name = 'node-MainThread', queue = 'com.apple.main-thread', stop reason = hit program assert
  * frame #0: 0x000000018d7e311c libsystem_kernel.dylib`__pthread_kill + 8
    frame #1: 0x000000018d81acc0 libsystem_pthread.dylib`pthread_kill + 288
    frame #2: 0x000000018d72aa50 libsystem_c.dylib`abort + 180
    frame #3: 0x000000018d729d6c libsystem_c.dylib`__assert_rtn + 284
    frame #4: 0x000000010be8b618 binding.node`tsfn_finalize(env=0x000000015cadc750, (null)=0x0000000000000000, finalize_hint=0x000000015cadcfe0) at binding.cc:51:3
    frame #5: 0x00000001001ea580 node`void node_napi_env__::CallFinalizer<false>(this=0x000000016fdfe638, env=0x000000015cadc750)(napi_env__*, void*, void*), void*, void*)::'lambda'(napi_env__*)::operator()(napi_env__*) const at node_api.cc:97:27
    frame #6: 0x00000001001ea480 node`void napi_env__::CallIntoModule<void node_napi_env__::CallFinalizer<false>(void (*)(napi_env__*, void*, void*), void*, void*)::'lambda'(napi_env__*)&, void node_napi_env__::CallbackIntoModule<false, void node_napi_env__::CallFinalizer<false>(void (*)(napi_env__*, void*, void*), void*, void*)::'lambda'(napi_env__*)>(void node_napi_env__::CallFinalizer<false>(void (*)(napi_env__*, void*, void*), void*, void*)::'lambda'(napi_env__*)&&)::'lambda'(napi_env__*, v8::Local<v8::Value>)>(this=0x000000015cadc750, call=0x000000016fdfe638, handle_exception=0x000000016fdfe5df) at js_native_api_v8.h:93:5
    frame #7: 0x00000001001ea434 node`void node_napi_env__::CallbackIntoModule<false, void node_napi_env__::CallFinalizer<false>(void (*)(napi_env__*, void*, void*), void*, void*)::'lambda'(napi_env__*)>(this=0x000000015cadc750, call=0x000000016fdfe638) at node_api.cc:138:3
    frame #8: 0x00000001001ea2a4 node`void node_napi_env__::CallFinalizer<false>(this=0x000000015cadc750, cb=(binding.node`tsfn_finalize(napi_env__*, void*, void*) at binding.cc:49), data=0x0000000000000000, hint=0x000000015cadcfe0) at node_api.cc:96:3
    frame #9: 0x00000001001ea128 node`v8impl::(anonymous namespace)::ThreadSafeFunction::Finalize(this=0x000000015cadc8a0) at node_api.cc:469:12
    frame #10: 0x00000001001ea01c node`v8impl::(anonymous namespace)::ThreadSafeFunction::CloseHandlesAndMaybeDelete(this=0x000000015cadcb88, handle=0x000000015cadc950)::'lambda'(uv_handle_s*)::operator()(uv_handle_s*) const at node_api.cc:493:18
    frame #11: 0x00000001001e9f80 node`void node::Environment::CloseHandle<uv_handle_s, v8impl::(anonymous namespace)::ThreadSafeFunction::CloseHandlesAndMaybeDelete(bool)::'lambda'(uv_handle_s*)>(this=0x000000016fdfe7a7, handle=0x000000015cadc950)::ThreadSafeFunction::CloseHandlesAndMaybeDelete(bool)::'lambda'(uv_handle_s*))::'lambda'(uv_handle_s*)::operator()(uv_handle_s*) const at env-inl.h:266:5
    frame #12: 0x00000001001e9f04 node`void node::Environment::CloseHandle<uv_handle_s, v8impl::(anonymous namespace)::ThreadSafeFunction::CloseHandlesAndMaybeDelete(bool)::'lambda'(uv_handle_s*)>(uv_handle_s*, v8impl::(anonymous namespace)::ThreadSafeFunction::CloseHandlesAndMaybeDelete(bool)::'lambda'(uv_handle_s*))::'lambda'(uv_handle_s*)::__invoke(handle=0x000000015cadc950) at env-inl.h:262:52
    frame #13: 0x0000000101f81138 node`uv__finish_close(handle=0x000000015cadc950) at core.c:363:5
    frame #14: 0x0000000101f7d54c node`uv__run_closing_handles(loop=0x0000000107f1a718) at core.c:377:5
    frame #15: 0x0000000101f7d3b8 node`uv_run(loop=0x0000000107f1a718, mode=UV_RUN_ONCE) at core.c:475:5
    frame #16: 0x0000000100153fe0 node`node::Environment::CleanupHandles(this=0x000000015d042200) at env.cc:1267:5
    frame #17: 0x0000000100154aec node`node::Environment::RunCleanup(this=0x000000015d042200) at env.cc:1323:3
    frame #18: 0x00000001000653ac node`node::FreeEnvironment(env=0x000000015d042200) at environment.cc:533:10
    frame #19: 0x0000000100060180 node`node::FunctionDeleter<node::Environment, &node::FreeEnvironment(node::Environment*)>::operator()(this=0x000000016fdfecc0, pointer=0x000000015d042200) const at util.h:674:39
    frame #20: 0x000000010005b108 node`std::__1::unique_ptr<node::Environment, node::FunctionDeleter<node::Environment, &node::FreeEnvironment(node::Environment*)>>::reset[abi:nn210104](this=0x000000016fdfecc0, __p=0x0000000000000000) at unique_ptr.h:290:7
    frame #21: 0x000000010005da28 node`std::__1::unique_ptr<node::Environment, node::FunctionDeleter<node::Environment, &node::FreeEnvironment(node::Environment*)>>::~unique_ptr[abi:nn210104](this=0x000000016fdfecc0) at unique_ptr.h:259:71
    frame #22: 0x000000010005d948 node`std::__1::unique_ptr<node::Environment, node::FunctionDeleter<node::Environment, &node::FreeEnvironment(node::Environment*)>>::~unique_ptr[abi:nn210104](this=0x000000016fdfecc0) at unique_ptr.h:259:69
    frame #23: 0x000000010031ce94 node`node::NodeMainInstance::Run(this=0x000000016fdfed98) at node_main_instance.cc:101:1
    frame #24: 0x00000001001d3974 node`node::StartInternal(argc=5, argv=0x000000015cb040e0) at node.cc:1585:24
    frame #25: 0x00000001001d3520 node`node::Start(argc=5, argv=0x000000016fdff180) at node.cc:1592:27
    frame #26: 0x00000001026ca3d8 node`main(argc=5, argv=0x000000016fdff180) at node_main.cc:97:10
    frame #27: 0x000000018d4a1058 dyld`start + 2224

With the fix commit, the process completes as expected.

@KevinEady
test: add node-api tsfn test with freed context after abort
d52e75c 
Add a test where a threadsafe function's `call_js` callback uses its
context which is freed in the threadsafe function's finalizer.
@KevinEady KevinEady force-pushed the fix-tsfn-finalizer-and-queue-drain-order branch from 3ff5d15 to d52e75c Compare February 23, 2026 18:59
@codecov
Copy link

codecov bot commented Feb 23, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.66%. Comparing base (da5efc4) to head (e9fec8b).
⚠️ Report is 178 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #61956      +/-   ##
==========================================
+ Coverage   88.84%   89.66%   +0.82%     
==========================================
  Files         674      676       +2     
  Lines      204957   206465    +1508     
  Branches    39309    39538     +229     
==========================================
+ Hits       182087   185135    +3048     
+ Misses      15088    13453    -1635     
- Partials     7782     7877      +95
Files with missing lines Coverage Δ
src/node_api.cc 75.49% <100.00%> (+0.33%) ⬆️

... and 252 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@KevinEady KevinEady moved this from Need Triage to In Progress in Node-API Team Project Feb 26, 2026
toyobayashi added a commit to toyobayashi/emnapi that referenced this pull request Mar 6, 2026
@toyobayashi
fix: execute tsfn finalizer after queue drains when aborted (nodejs/n…
46a82b7 
…ode#61956)
legendecas
legendecas reviewed Mar 6, 2026
View reviewed changes
toyobayashi added a commit to toyobayashi/emnapi that referenced this pull request Mar 10, 2026
@toyobayashi
fix: execute tsfn finalizer after queue drains when aborted (nodejs/n…
5e3e8a9 
…ode#61956)
@KevinEady KevinEady force-pushed the fix-tsfn-finalizer-and-queue-drain-order branch from bc464e2 to e9fec8b Compare March 12, 2026 21:09
@legendecas legendecas added request-ci Add this label to start a Jenkins CI on a PR. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Mar 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@legendecas legendecas legendecas approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. node-api Issues and PRs related to the Node-API. request-ci Add this label to start a Jenkins CI on a PR.

Projects

Node-API Team Project
Status: In Progress

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@KevinEady @nodejs-github-bot @legendecas