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

src: fix finalization crash #38250

Closed
wants to merge 1 commit into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Apr 15, 2021

Fixes: #38040

@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. labels Apr 15, 2021
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 15, 2021

@jasnell jasnell added fast-track PRs that do not need to wait for 48 hours to land. and removed needs-ci PRs that need a full CI run. labels Apr 15, 2021
@jasnell
Copy link
Member Author

jasnell commented Apr 15, 2021

If this does fix #38040 and CI is good, would be good to fast-track. Please 👍🏻

@jasnell jasnell changed the title src: maybe fix finalization crash src: fix finalization crash Apr 15, 2021
jasnell added a commit that referenced this pull request Apr 15, 2021
PR-URL: #38250
Fixes: #38040
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@jasnell
Copy link
Member Author

jasnell commented Apr 15, 2021

Landed in fc20e83

@jasnell jasnell closed this Apr 15, 2021
@legendecas legendecas added the node-api Issues and PRs related to the Node-API. label Apr 16, 2021
@legendecas
Copy link
Member

@jasnell 👍 for quick fixing on the issue. Unfortunately, this patch doesn't ultimately fix the use-after-free issue on node-api finalization. #38000 is still required to fix the use-after-free on v8impl::Reference.

targos pushed a commit that referenced this pull request May 1, 2021
PR-URL: #38250
Fixes: #38040
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@danielleadams danielleadams mentioned this pull request May 3, 2021
danielleadams pushed a commit that referenced this pull request May 8, 2021
PR-URL: #38250
Fixes: #38040
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
legendecas pushed a commit to legendecas/node that referenced this pull request Mar 29, 2022
PR-URL: nodejs#38250
Fixes: nodejs#38040
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
richardlau pushed a commit that referenced this pull request Mar 30, 2022
PR-URL: #38250
Backport-PR-URL: #42512
Fixes: #38040
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@richardlau richardlau mentioned this pull request Mar 30, 2022
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++. fast-track PRs that do not need to wait for 48 hours to land. node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

local fatal error for test/js-native-api/test_object/test.js
5 participants