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: add Triagers section to table of contents in GOVERNANCE.md #34504

Merged
merged 2 commits into from
Jul 24, 2020
Merged

doc: add Triagers section to table of contents in GOVERNANCE.md #34504

merged 2 commits into from
Jul 24, 2020

Conversation

Trott
Copy link
Member

@Trott Trott commented Jul 24, 2020

Checklist

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Jul 24, 2020
@targos
Copy link
Member

targos commented Jul 24, 2020

Fast-track ?

@targos targos added the fast-track PRs that do not need to wait for 48 hours to land. label Jul 24, 2020
Gabriel Schulhof and others added 2 commits July 23, 2020 23:28
n-api: run all finalizers via SetImmediate()
a74a6e3 
Throwing an exception from a finalizer can cause the following fatal
error:

Error: async hook stack has become corrupted (actual: 2, expected: 0)
 1: 0x970b5a node::InternalCallbackScope::~InternalCallbackScope()
    [./node]
 2: 0x99dda0 node::Environment::RunTimers(uv_timer_s*) [./node]
 3: 0x13d8b22  [./node]
 4: 0x13dbe42 uv_run [./node]
 5: 0xa57974 node::NodeMainInstance::Run() [./node]
 6: 0x9dbc17 node::Start(int, char**) [./node]
 7: 0x7f4965417f43 __libc_start_main [/lib64/libc.so.6]
 8: 0x96f4ae _start [./node]

By nodejs#34341 (comment),
calling into JS from a finalizer and/or throwing exceptions from there
is not advised, because the stack may or may not be set up for JS
execution. The best solution is to run the user's finalizer from a
`SetImmediate()` callback.

Signed-off-by: Gabriel Schulhof <[email protected]>
Fixes: nodejs#34341
PR-URL: nodejs#34386
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
@Trott
doc: add Triagers section to table of contents in GOVERNANCE.md
001a4a7 
PR-URL: nodejs#34504
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Harshitha K P <[email protected]>
@Trott
Copy link
Member Author

Trott commented Jul 24, 2020

Landed in 001a4a7

@Trott Trott merged commit 001a4a7 into nodejs:master Jul 24, 2020
@Trott Trott deleted the tri2 branch July 24, 2020 14:25
MylesBorins pushed a commit that referenced this pull request Jul 27, 2020
@Trott @MylesBorins
doc: add Triagers section to table of contents in GOVERNANCE.md
2297d74 
PR-URL: #34504
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Harshitha K P <[email protected]>
@ruyadorno ruyadorno mentioned this pull request Jul 28, 2020
Merged
addaleax pushed a commit that referenced this pull request Sep 22, 2020
@Trott @addaleax
doc: add Triagers section to table of contents in GOVERNANCE.md
4cb6c6c 
PR-URL: #34504
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Harshitha K P <[email protected]>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
@Trott @addaleax
doc: add Triagers section to table of contents in GOVERNANCE.md
cef1284 
PR-URL: #34504
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Harshitha K P <[email protected]>
@codebytere codebytere mentioned this pull request Sep 28, 2020
Merged
@mhdawson mhdawson mentioned this pull request Jan 18, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HarshithaKP HarshithaKP HarshithaKP approved these changes

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

@targos targos targos approved these changes

Assignees
No one assigned
Labels
fast-track PRs that do not need to wait for 48 hours to land. meta Issues and PRs related to the general management of the project.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Trott @targos @gireeshpunathil @HarshithaKP @nodejs-github-bot