X Tutup
The Wayback Machine - https://web.archive.org/web/20220509084348/https://github.com/nodejs/node/pull/38250
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

Jump to bottom

src: fix finalization crash #38250

Closed
wants to merge 1 commit into from
Closed

src: fix finalization crash #38250

wants to merge 1 commit into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Apr 15, 2021 •
edited

Fixes: #38040

@nodejs-github-bot nodejs-github-bot added c++ needs-ci labels Apr 15, 2021
@nodejs-github-bot
Copy link

@nodejs-github-bot nodejs-github-bot commented Apr 15, 2021 •
edited by jasnell

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

@jasnell jasnell mentioned this pull request Apr 15, 2021
Closed
@jasnell jasnell added fast-track and removed needs-ci labels Apr 15, 2021
@jasnell
Copy link
Author

@jasnell 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 issue Apr 15, 2021
@jasnell
src: fix finalization crash
fc20e83 
PR-URL: #38250
Fixes: #38040
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@jasnell
Copy link
Author

@jasnell jasnell commented Apr 15, 2021

Landed in fc20e83

@jasnell jasnell closed this Apr 15, 2021
@BethGriggs BethGriggs mentioned this pull request Apr 16, 2021
Merged
@legendecas legendecas added the node-api label Apr 16, 2021
@legendecas
Copy link

@legendecas legendecas commented Apr 16, 2021

@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.

@legendecas legendecas mentioned this pull request Apr 21, 2021
Closed
targos pushed a commit that referenced this issue May 1, 2021
@jasnell @targos
src: fix finalization crash
ec4be2d 
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
Merged
danielleadams pushed a commit that referenced this issue May 8, 2021
@jasnell @danielleadams
src: fix finalization crash
27ebcb7 
PR-URL: #38250
Fixes: #38040
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@legendecas legendecas mentioned this pull request Mar 29, 2022
Closed
legendecas pushed a commit to legendecas/node that referenced this issue Mar 29, 2022
@jasnell @legendecas
src: fix finalization crash
2a8bdd1 
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 issue Mar 30, 2022
@jasnell @richardlau
src: fix finalization crash
e707514 
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
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BethGriggs BethGriggs

@aduh95 aduh95

@targos targos

@richardlau richardlau

Assignees
No one assigned
Labels
c++ fast-track node-api
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

local fatal error for test/js-native-api/test_object/test.js
5 participants
@jasnell @nodejs-github-bot @legendecas @BethGriggs @aduh95
X Tutup