Skip to content

Conversation

gabrielschulhof
Copy link
Contributor

Heed the comment to not use fields of a Reference after calling its
finalize callback, because such a call may destroy the Reference.

Fixes: #19673

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. dont-land-on-v4.x node-api Issues and PRs related to the Node-API. labels Apr 1, 2018
Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

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

LGTM, but a test for this would be really nice.

Heed the comment to not use fields of a Reference after calling its
finalize callback, because such a call may destroy the Reference.

Fixes: nodejs#19673
@gabrielschulhof
Copy link
Contributor Author

@cjihrig I updated a test to check this code path as well.

@cjihrig
Copy link
Contributor

cjihrig commented Apr 2, 2018

Great, thanks!

@gabrielschulhof
Copy link
Contributor Author

@gabrielschulhof
Copy link
Contributor Author

I believe the CI failures are unrelated.

@gabrielschulhof
Copy link
Contributor Author

Landed in 1a5a19d.

gabrielschulhof pushed a commit that referenced this pull request Apr 3, 2018
Heed the comment to not use fields of a Reference after calling its
finalize callback, because such a call may destroy the Reference.

Fixes: #19673
PR-URL: #19718
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@gabrielschulhof gabrielschulhof deleted the n-api-back-up-env branch April 3, 2018 13:39
targos pushed a commit that referenced this pull request Apr 3, 2018
Heed the comment to not use fields of a Reference after calling its
finalize callback, because such a call may destroy the Reference.

Fixes: #19673
PR-URL: #19718
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@targos targos mentioned this pull request Apr 4, 2018
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Apr 12, 2018
Heed the comment to not use fields of a Reference after calling its
finalize callback, because such a call may destroy the Reference.

Fixes: nodejs#19673
PR-URL: nodejs#19718
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Apr 16, 2018
Heed the comment to not use fields of a Reference after calling its
finalize callback, because such a call may destroy the Reference.

Fixes: nodejs#19673
PR-URL: nodejs#19718
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Apr 16, 2018
Heed the comment to not use fields of a Reference after calling its
finalize callback, because such a call may destroy the Reference.

Fixes: #19673
Backport-PR-URL: #19447
PR-URL: #19718
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Apr 16, 2018
MylesBorins pushed a commit that referenced this pull request May 1, 2018
Heed the comment to not use fields of a Reference after calling its
finalize callback, because such a call may destroy the Reference.

Fixes: #19673
Backport-PR-URL: #19265
PR-URL: #19718
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@MylesBorins MylesBorins mentioned this pull request May 2, 2018
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++. node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

n-api - failure in node-addon-api testing after change in node core
4 participants