Skip to content

Comments

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

Open
KevinEady wants to merge 2 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 2 commits intonodejs:mainfrom
KevinEady:fix-tsfn-finalizer-and-queue-drain-order

Conversation

@KevinEady
Copy link
Contributor

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.

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

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

Without the fix commit, the new test fails:

Context: create called
tsfn_finalize: env=0x145e28c70 finalize_data=0x0 finalize_hint=0x145e28c00
Context: destructor called
[1]    39761 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 = signal SIGABRT
  * 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: 0x000000010bea7a4c binding.node`std::__1::__throw_bad_function_call[abi:v160006]() at function.h:74:5
    frame #4: 0x000000010bea7a1c binding.node`std::__1::__function::__value_func<int (void*)>::operator()[abi:v160006](this=0x000000012a69b960, __args=0x000000016fdfe570) const at function.h:509:13
    frame #5: 0x000000010bea2574 binding.node`std::__1::function<int (void*)>::operator()(this=0x000000012a69b960, __arg=0x000000012a69b9a0) const at function.h:1156:12
    frame #6: 0x000000010bea250c binding.node`tsfn_callback(env=0x0000000000000000, js_cb=0x0000000000000000, ctx_p=0x000000012a69b940, data=0x000000012a69b9a0) at binding.cc:53:64
    frame #7: 0x00000001001ea358 node`v8impl::(anonymous namespace)::ThreadSafeFunction::EmptyQueueAndMaybeDelete(this=0x000000012a69bbe0) at node_api.cc:318:7
    frame #8: 0x00000001001ea13c node`v8impl::(anonymous namespace)::ThreadSafeFunction::Finalize(this=0x000000012a69bbe0) at node_api.cc:471:5
    frame #9: 0x00000001001ea01c node`v8impl::(anonymous namespace)::ThreadSafeFunction::CloseHandlesAndMaybeDelete(this=0x000000012a69bf68, handle=0x000000012a69bc90)::'lambda'(uv_handle_s*)::operator()(uv_handle_s*) const at node_api.cc:493:18
    frame #10: 0x00000001001e9f80 node`void node::Environment::CloseHandle<uv_handle_s, v8impl::(anonymous namespace)::ThreadSafeFunction::CloseHandlesAndMaybeDelete(bool)::'lambda'(uv_handle_s*)>(this=0x000000016fdfe777, handle=0x000000012a69bc90)::ThreadSafeFunction::CloseHandlesAndMaybeDelete(bool)::'lambda'(uv_handle_s*))::'lambda'(uv_handle_s*)::operator()(uv_handle_s*) const at env-inl.h:266:5
    frame #11: 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=0x000000012a69bc90) at env-inl.h:262:52
    frame #12: 0x0000000101f81138 node`uv__finish_close(handle=0x000000012a69bc90) at core.c:363:5
    frame #13: 0x0000000101f7d54c node`uv__run_closing_handles(loop=0x0000000107f1a718) at core.c:377:5
    frame #14: 0x0000000101f7d3b8 node`uv_run(loop=0x0000000107f1a718, mode=UV_RUN_ONCE) at core.c:475:5
    frame #15: 0x0000000100153fe0 node`node::Environment::CleanupHandles(this=0x000000012a83dc00) at env.cc:1267:5
    frame #16: 0x0000000100154aec node`node::Environment::RunCleanup(this=0x000000012a83dc00) at env.cc:1323:3
    frame #17: 0x00000001000653ac node`node::FreeEnvironment(env=0x000000012a83dc00) at environment.cc:533:10
    frame #18: 0x0000000100060180 node`node::FunctionDeleter<node::Environment, &node::FreeEnvironment(node::Environment*)>::operator()(this=0x000000016fdfec90, pointer=0x000000012a83dc00) const at util.h:674:39
    frame #19: 0x000000010005b108 node`std::__1::unique_ptr<node::Environment, node::FunctionDeleter<node::Environment, &node::FreeEnvironment(node::Environment*)>>::reset[abi:nn210104](this=0x000000016fdfec90, __p=0x0000000000000000) at unique_ptr.h:290:7
    frame #20: 0x000000010005da28 node`std::__1::unique_ptr<node::Environment, node::FunctionDeleter<node::Environment, &node::FreeEnvironment(node::Environment*)>>::~unique_ptr[abi:nn210104](this=0x000000016fdfec90) at unique_ptr.h:259:71
    frame #21: 0x000000010005d948 node`std::__1::unique_ptr<node::Environment, node::FunctionDeleter<node::Environment, &node::FreeEnvironment(node::Environment*)>>::~unique_ptr[abi:nn210104](this=0x000000016fdfec90) at unique_ptr.h:259:69
    frame #22: 0x000000010031ce94 node`node::NodeMainInstance::Run(this=0x000000016fdfed68) at node_main_instance.cc:101:1
    frame #23: 0x00000001001d3974 node`node::StartInternal(argc=5, argv=0x000000012a634990) at node.cc:1585:24
    frame #24: 0x00000001001d3520 node`node::Start(argc=5, argv=0x000000016fdff150) at node.cc:1592:27
    frame #25: 0x00000001026ca3d8 node`main(argc=5, argv=0x000000016fdff150) at node_main.cc:97:10
    frame #26: 0x000000018d4a1058 dyld`start + 2224

With the fix commit, the process completes as expected:

Context: create called
Context: get called
tsfn_callback: env=0x0 data=1
Context: deleter called
tsfn_finalize: env=0x124a64c10 finalize_data=0x0 finalize_hint=0x124a64da0
Context: destructor called

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

Codecov Report

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

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #61956      +/-   ##
==========================================
+ Coverage   88.84%   89.77%   +0.93%     
==========================================
  Files         674      674              
  Lines      204957   205608     +651     
  Branches    39309    39408      +99     
==========================================
+ Hits       182087   184586    +2499     
+ Misses      15088    13275    -1813     
+ Partials     7782     7747      -35     
Files with missing lines Coverage Δ
src/node_api.cc 75.24% <100.00%> (+0.09%) ⬆️

... and 179 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Projects

Status: Need Triage

Development

Successfully merging this pull request may close these issues.

2 participants