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.

Sign up for GitHub

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

Jump to bottom

doc: clarify napi_finalize behavior #42461

Merged
merged 2 commits into from
Mar 27, 2022
Merged

doc: clarify napi_finalize behavior #42461

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.

@mildsunrise
doc: clarify napi_finalize behavior
f428d2d 
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 •
edited by mhdawson
Loading

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
@mildsunrise
Update object_wrap.md
9bdc3db 
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
@mildsunrise
doc: clarify ObjectWrap weak ref behavior
6b8892a 
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 mildsunrise mentioned this pull request Mar 25, 2022
Merged
Trott
Trott reviewed Mar 25, 2022
View reviewed changes
doc/api/n-api.md Outdated Show resolved Hide resolved
mhdawson
mhdawson approved these changes Mar 25, 2022
View reviewed changes
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
@mildsunrise
doc: clarify ObjectWrap weak ref behavior (#1155)
8c46a95 
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
@mildsunrise @juanarbol
doc: clarify napi_finalize behavior
f63ccee 
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
@mildsunrise @juanarbol
doc: clarify napi_finalize behavior
f9f6c8e 
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
Closed
@juanarbol juanarbol mentioned this pull request Apr 5, 2022
Merged
juanarbol pushed a commit that referenced this pull request Apr 6, 2022
@mildsunrise @juanarbol
doc: clarify napi_finalize behavior
3c3684d 
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
@mildsunrise @xtx1130
doc: clarify napi_finalize behavior
158bd0a 
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
@mildsunrise @juanarbol
doc: clarify napi_finalize behavior
aecc77b 
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 juanarbol mentioned this pull request May 31, 2022
Closed
danielleadams pushed a commit that referenced this pull request Jun 27, 2022
@mildsunrise @danielleadams
doc: clarify napi_finalize behavior
543ac0b 
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
@mildsunrise @targos
doc: clarify napi_finalize behavior
c94569a 
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
@mildsunrise @targos
doc: clarify napi_finalize behavior
89e3ced 
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 targos mentioned this pull request Aug 3, 2022
Merged
kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this pull request Aug 24, 2022
@kevindavies8
doc: clarify ObjectWrap weak ref behavior (#1155)
8d6798c 
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
@Marlyfleitas
doc: clarify ObjectWrap weak ref behavior (#1155)
b21cfa9 
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
@wroy7860
doc: clarify ObjectWrap weak ref behavior (#1155)
c976be4 
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
@mildsunrise @guangwong
doc: clarify napi_finalize behavior
d5fde6c 
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
doc: clarify ObjectWrap weak ref behavior (#1155)
981a0ad 
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
Reviewers

@Trott Trott Trott approved these changes

@mhdawson mhdawson mhdawson approved these changes

@legendecas legendecas legendecas approved these changes

@tniessen tniessen tniessen approved these changes

@bnb bnb bnb approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@mildsunrise @nodejs-github-bot @jasnell @bnb @Trott @tniessen @legendecas @mhdawson