Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: clarify napi_finalize behavior #42461

Merged
merged 2 commits into from
Mar 27, 2022

Conversation

mildsunrise
Copy link
Member

We currently defer finalizer callbacks until the loop is idle.

Warn users that the weak reference returned by napi_wrap() isn't guaranteed to be valid just because the finalizer hasn't yet been called.

We currently defer finalizer callbacks until the loop is idle.
Warn users that the weak reference returned by napi_wrap()
isn't guaranteed to be valid just because the finalizer hasn't
yet been called.
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 25, 2022

Review requested:

  • @nodejs/node-api

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. node-api Issues and PRs related to the Node-API. labels Mar 25, 2022
mildsunrise added a commit to mildsunrise/node-addon-api that referenced this pull request Mar 25, 2022
Refs: nodejs/node#42461

We currently defer finalizer callbacks until the loop is idle.
Warn users that the weak reference on ObjectWraps
isn't guaranteed to be valid just because the object hasn't
been finalized (destructed) yet.
mildsunrise added a commit to mildsunrise/node-addon-api that referenced this pull request Mar 25, 2022
Refs: nodejs/node#42461

We currently defer finalizer callbacks until the loop is idle.
Warn users that the weak reference on ObjectWraps
isn't guaranteed to be valid just because the object hasn't
been finalized (destructed) yet.
doc/api/n-api.md Outdated Show resolved Hide resolved
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jasnell jasnell added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 25, 2022
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Mar 25, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/42461
✔  Done loading data for nodejs/node/pull/42461
----------------------------------- PR info ------------------------------------
Title      doc: clarify napi_finalize behavior (#42461)
Author     Alba Mendez  (@mildsunrise)
Branch     mildsunrise:doc-napi-finalize -> nodejs:master
Labels     doc, node-api
Commits    2
 - doc: clarify napi_finalize behavior
 - fixup
Committers 1
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/42461
Reviewed-By: Rich Trott 
Reviewed-By: Tierney Cyren 
Reviewed-By: Tobias Nießen 
Reviewed-By: Michael Dawson 
Reviewed-By: Chengzhong Wu 
Reviewed-By: James M Snell 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/42461
Reviewed-By: Rich Trott 
Reviewed-By: Tierney Cyren 
Reviewed-By: Tobias Nießen 
Reviewed-By: Michael Dawson 
Reviewed-By: Chengzhong Wu 
Reviewed-By: James M Snell 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 25 Mar 2022 02:04:52 GMT
   ✔  Approvals: 6
   ✔  - Rich Trott (@Trott) (TSC): https://github.com/nodejs/node/pull/42461#pullrequestreview-921112010
   ✔  - Tierney Cyren (@bnb): https://github.com/nodejs/node/pull/42461#pullrequestreview-921208653
   ✔  - Tobias Nießen (@tniessen) (TSC): https://github.com/nodejs/node/pull/42461#pullrequestreview-921726561
   ✔  - Michael Dawson (@mhdawson) (TSC): https://github.com/nodejs/node/pull/42461#pullrequestreview-921801271
   ✔  - Chengzhong Wu (@legendecas): https://github.com/nodejs/node/pull/42461#pullrequestreview-921802565
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/42461#pullrequestreview-922219479
   ✖  This PR needs to wait 26 more hours to land
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/2042793481

@mildsunrise mildsunrise added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. 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 27, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 27, 2022
@nodejs-github-bot nodejs-github-bot merged commit 01a2300 into nodejs:master Mar 27, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in 01a2300

@mildsunrise mildsunrise deleted the doc-napi-finalize branch March 27, 2022 14:07
mhdawson pushed a commit to nodejs/node-addon-api that referenced this pull request Mar 31, 2022
Refs: nodejs/node#42461

We currently defer finalizer callbacks until the loop is idle.
Warn users that the weak reference on ObjectWraps
isn't guaranteed to be valid just because the object hasn't
been finalized (destructed) yet.
juanarbol pushed a commit that referenced this pull request Apr 4, 2022
We currently defer finalizer callbacks until the loop is idle.
Warn users that the weak reference returned by napi_wrap()
isn't guaranteed to be valid just because the finalizer hasn't
yet been called.

PR-URL: #42461
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tierney Cyren <hello@bnb.im>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol pushed a commit to juanarbol/node that referenced this pull request Apr 5, 2022
We currently defer finalizer callbacks until the loop is idle.
Warn users that the weak reference returned by napi_wrap()
isn't guaranteed to be valid just because the finalizer hasn't
yet been called.

PR-URL: nodejs#42461
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tierney Cyren <hello@bnb.im>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@juanarbol juanarbol mentioned this pull request Apr 5, 2022
@juanarbol juanarbol mentioned this pull request Apr 5, 2022
juanarbol pushed a commit that referenced this pull request Apr 6, 2022
We currently defer finalizer callbacks until the loop is idle.
Warn users that the weak reference returned by napi_wrap()
isn't guaranteed to be valid just because the finalizer hasn't
yet been called.

PR-URL: #42461
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tierney Cyren <hello@bnb.im>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
xtx1130 pushed a commit to xtx1130/node that referenced this pull request Apr 25, 2022
We currently defer finalizer callbacks until the loop is idle.
Warn users that the weak reference returned by napi_wrap()
isn't guaranteed to be valid just because the finalizer hasn't
yet been called.

PR-URL: nodejs#42461
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tierney Cyren <hello@bnb.im>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol pushed a commit that referenced this pull request May 31, 2022
We currently defer finalizer callbacks until the loop is idle.
Warn users that the weak reference returned by napi_wrap()
isn't guaranteed to be valid just because the finalizer hasn't
yet been called.

PR-URL: #42461
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tierney Cyren <hello@bnb.im>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams pushed a commit that referenced this pull request Jun 27, 2022
We currently defer finalizer callbacks until the loop is idle.
Warn users that the weak reference returned by napi_wrap()
isn't guaranteed to be valid just because the finalizer hasn't
yet been called.

PR-URL: #42461
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tierney Cyren <hello@bnb.im>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jul 11, 2022
We currently defer finalizer callbacks until the loop is idle.
Warn users that the weak reference returned by napi_wrap()
isn't guaranteed to be valid just because the finalizer hasn't
yet been called.

PR-URL: #42461
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tierney Cyren <hello@bnb.im>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jul 31, 2022
We currently defer finalizer callbacks until the loop is idle.
Warn users that the weak reference returned by napi_wrap()
isn't guaranteed to be valid just because the finalizer hasn't
yet been called.

PR-URL: #42461
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tierney Cyren <hello@bnb.im>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this pull request Aug 24, 2022
Refs: nodejs/node#42461

We currently defer finalizer callbacks until the loop is idle.
Warn users that the weak reference on ObjectWraps
isn't guaranteed to be valid just because the object hasn't
been finalized (destructed) yet.
Marlyfleitas added a commit to Marlyfleitas/node-api-addon-Development that referenced this pull request Aug 26, 2022
Refs: nodejs/node#42461

We currently defer finalizer callbacks until the loop is idle.
Warn users that the weak reference on ObjectWraps
isn't guaranteed to be valid just because the object hasn't
been finalized (destructed) yet.
wroy7860 added a commit to wroy7860/addon-api-benchmark-node that referenced this pull request Sep 19, 2022
Refs: nodejs/node#42461

We currently defer finalizer callbacks until the loop is idle.
Warn users that the weak reference on ObjectWraps
isn't guaranteed to be valid just because the object hasn't
been finalized (destructed) yet.
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
We currently defer finalizer callbacks until the loop is idle.
Warn users that the weak reference returned by napi_wrap()
isn't guaranteed to be valid just because the finalizer hasn't
yet been called.

PR-URL: nodejs/node#42461
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tierney Cyren <hello@bnb.im>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this pull request Aug 11, 2023
Refs: nodejs/node#42461

We currently defer finalizer callbacks until the loop is idle.
Warn users that the weak reference on ObjectWraps
isn't guaranteed to be valid just because the object hasn't
been finalized (destructed) yet.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. doc Issues and PRs related to the documentations. node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants