Skip to content

node-api: fix crash on re-entering TSFN finalization during env shutdown - #65967

Open
legendecas wants to merge 1 commit into
nodejs:mainfrom
legendecas:node-api-tsfn-reentry
Open

legendecas wants to merge 1 commit into
nodejs:mainfrom
legendecas:node-api-tsfn-reentry

Conversation

@legendecas

Copy link
Copy Markdown
Member

Fix the TSFN finalization re-entrance. state = kClosed marks a TSFN as ready to be
deleted, so a TSFN should not set this state before calling into user code synchronously,
which could call into napi_release_threadsafe_function, and delete the TSFN
synchronously in the middle of ThreadSafeFunction::MaybeDelete.

Fixes: #65100

@nodejs-github-bot

nodejs-github-bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Collaborator

Review requested:

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

@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 Node-API. labels Sep 10, 2026
@legendecas
legendecas force-pushed the node-api-tsfn-reentry branch 2 times, most recently from b3c624f to 043d783 Compare September 10, 2026 21:12
@codecov

codecov Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.21%. Comparing base (6193e15) to head (742384c).
⚠️ Report is 31 commits behind head on main.

Files with missing lines Patch % Lines
src/node_api.cc 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65967      +/-   ##
==========================================
+ Coverage   90.20%   90.21%   +0.01%     
==========================================
  Files         785      785              
  Lines      269347   269348       +1     
  Branches    51509    51509              
==========================================
+ Hits       242971   243000      +29     
+ Misses      16890    16858      -32     
- Partials     9486     9490       +4     
Files with missing lines Coverage Δ
src/node_api.cc 75.48% <66.66%> (+0.15%) ⬆️

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

@legendecas legendecas moved this from Need Triage to In Progress in Node-API Team Project Sep 11, 2026
Comment thread src/node_api.cc
Signed-off-by: Chengzhong Wu <cwu631@bloomberg.net>
Signed-off-by: Chengzhong Wu <legendecas@gmail.com>
@legendecas
legendecas force-pushed the node-api-tsfn-reentry branch from 043d783 to 742384c Compare September 13, 2026 20:29
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

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 Node-API.

Projects

Status: In Progress

3 participants