node-api: make tsfn accept napi_finalize once more by gabrielschulhof · Pull Request #51801 · nodejs/node

gabrielschulhof · 2024-02-18T17:08:52Z

The thread-safe function's finalizer is not called in conjunction with the garbage collection of a JS value. In fact, it keeps a strong reference to the JS function it is expected to call. Thus, it is safe to make calls that affect GC state from its body.

nodejs-github-bot · 2024-02-18T17:08:57Z

Review requested:

@nodejs/node-api

mhdawson

LGTM

nodejs-github-bot · 2024-02-20T15:29:41Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57201/

nodejs-github-bot · 2024-02-20T19:24:15Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57213/

nodejs-github-bot · 2024-02-20T22:08:18Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57219/

nodejs-github-bot · 2024-02-21T00:23:04Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57225/

nodejs-github-bot · 2024-02-21T09:49:25Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57243/

nodejs-github-bot · 2024-02-22T01:00:53Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57279/

nodejs-github-bot · 2024-02-25T18:25:21Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57411/

nodejs-github-bot · 2024-02-26T08:50:08Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57427/

nodejs-github-bot · 2024-02-29T20:56:54Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57514/

The thread-safe function's finalizer is not called in conjunction with the garbage collection of a JS value. In fact, it keeps a strong reference to the JS function it is expected to call. Thus, it is safe to make calls that affect GC state from its body.

nodejs-github-bot · 2024-03-01T05:46:45Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57518/

nodejs-github-bot · 2024-03-01T16:38:26Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57537/

nodejs-github-bot · 2024-03-08T05:35:24Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57629/

nodejs-github-bot · 2024-03-08T16:36:11Z

Commit Queue failed

- Loading data for nodejs/node/pull/51801
✔ Done loading data for nodejs/node/pull/51801
----------------------------------- PR info ------------------------------------
Title node-api: make tsfn accept napi_finalize once more (#51801)
⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch gabrielschulhof:tsfn-undo-nogc-finalizer -> nodejs:main
Labels c++, node-api
Commits 1
- node-api: make tsfn accept napi_finalize once more
Committers 1
- Gabriel Schulhof PR-URL: https://github.com/nodejs/node/pull/51801
Reviewed-By: Michael Dawson Reviewed-By: Chengzhong Wu ------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/51801
Reviewed-By: Michael Dawson Reviewed-By: Chengzhong Wu --------------------------------------------------------------------------------
⚠ Commits were pushed since the last approving review:
⚠ - node-api: make tsfn accept napi_finalize once more
ℹ This PR was created on Sun, 18 Feb 2024 17:08:52 GMT
✔ Approvals: 2
✔ - Michael Dawson (@mhdawson) (TSC): https://github.com/nodejs/node/pull/51801#pullrequestreview-1890738462
✔ - Chengzhong Wu (@legendecas) (TSC): https://github.com/nodejs/node/pull/51801#pullrequestreview-1894793505
✔ Last GitHub CI successful
ℹ Last Full PR CI on 2024-03-08T05:35:24Z: https://ci.nodejs.org/job/node-test-pull-request/57629/
- Querying data for job/node-test-pull-request/57629/
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/8206250115

gabrielschulhof · 2024-03-08T16:46:21Z

@legendecas, @mhdawson can you please give the PR another look?

mhdawson

Still LGTM

nodejs-github-bot · 2024-03-09T05:58:43Z

Landed in f0e6acd

The thread-safe function's finalizer is not called in conjunction with the garbage collection of a JS value. In fact, it keeps a strong reference to the JS function it is expected to call. Thus, it is safe to make calls that affect GC state from its body. PR-URL: #51801 Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

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 18, 2024

gabrielschulhof requested review from legendecas, mhdawson and vmoroz February 18, 2024 17:09

mhdawson approved these changes Feb 20, 2024
View reviewed changes

mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 20, 2024

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 20, 2024

gabrielschulhof force-pushed the tsfn-undo-nogc-finalizer branch from 6957cd6 to b098fc1Compare February 21, 2024 09:48

legendecas approved these changes Feb 22, 2024
View reviewed changes

toyobayashi mentioned this pull request Feb 23, 2024
feat: segregate nogc APIs from rest via type system toyobayashi/emnapi#110
Merged

gabrielschulhof force-pushed the tsfn-undo-nogc-finalizer branch from b098fc1 to 9a9266cCompare March 1, 2024 05:45

gabrielschulhof added commit-queue Add this label to land a pull request using GitHub Actions. and removed needs-ci PRs that need a full CI run. labels Mar 8, 2024

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 8, 2024

nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Mar 8, 2024

mhdawson approved these changes Mar 8, 2024
View reviewed changes

gabrielschulhof added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Mar 9, 2024

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 9, 2024

nodejs-github-bot merged commit f0e6acd into nodejs:mainMar 9, 2024

KevinEady mentioned this pull request Mar 11, 2024
[v18.x-backport] Backport feature flags and nogc types 2 #51804
Closed

mhdawson added lts-watch-v18.x labels Mar 15, 2024

This was referenced Apr 12, 2024
Release plan - v20.x Maintenance LTS nodejs/Release#855
Closed
Release plan - v18.x Maintenance LTS nodejs/Release#737
Closed

RafaelGSS mentioned this pull request Apr 15, 2024
2024-04-24, Version 22.0.0 (Current) #52505
Merged

marco-ippolito mentioned this pull request May 2, 2024
v20.13.0 proposal #52793
Merged

richardlau added backported-to-v18.x and removed lts-watch-v18.x labels May 16, 2024

richardlau mentioned this pull request May 16, 2024
v18.20.3 proposal #53028
Merged

Uh oh!

Uh oh!

Conversation

gabrielschulhof commented Feb 18, 2024

Uh oh!

nodejs-github-bot commented Feb 18, 2024

Uh oh!

mhdawson left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Feb 20, 2024

Uh oh!

nodejs-github-bot commented Feb 20, 2024

Uh oh!

nodejs-github-bot commented Feb 20, 2024

Uh oh!

nodejs-github-bot commented Feb 21, 2024

Uh oh!

nodejs-github-bot commented Feb 21, 2024

Uh oh!

nodejs-github-bot commented Feb 22, 2024

Uh oh!

nodejs-github-bot commented Feb 25, 2024

Uh oh!

nodejs-github-bot commented Feb 26, 2024

Uh oh!

nodejs-github-bot commented Feb 29, 2024

Uh oh!

nodejs-github-bot commented Mar 1, 2024

Uh oh!

nodejs-github-bot commented Mar 1, 2024

Uh oh!

nodejs-github-bot commented Mar 8, 2024

Uh oh!

nodejs-github-bot commented Mar 8, 2024

Uh oh!

gabrielschulhof commented Mar 8, 2024

Uh oh!

mhdawson left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Mar 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants