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

src: disallow JS execution during exit() #35020

Closed
wants to merge 1 commit into from
Closed

src: disallow JS execution during exit() #35020

wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

@addaleax addaleax commented Sep 2, 2020

This is just an extra safeguard – no JS should run once we’ve
decided that we are going to shutdown the process, but we’re opening
a handle scope in order to access V8, so make sure that our operations
do not lead to JS accidentally being run.

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

@addaleax
src: disallow JS execution during exit()
c65534e 
This is just an extra safeguard – no JS should run once we’ve
decided that we are going to shutdown the process, but we’re opening
a handle scope in order to access V8, so make sure that our operations
do not lead to JS accidentally being run.
@addaleax addaleax added the lib / src Issues and PRs related to general changes in the lib or src directory. label Sep 2, 2020
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Sep 2, 2020
@addaleax addaleax added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 2, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 2, 2020
@nodejs-github-bot
Copy link
Collaborator

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

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 2, 2020
@nodejs-github-bot
Copy link
Collaborator

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

@codebytere
Copy link
Member

Landed in 8bdbacf

@codebytere codebytere closed this Sep 4, 2020
codebytere pushed a commit that referenced this pull request Sep 4, 2020
@addaleax @codebytere
src: disallow JS execution during exit()
8bdbacf 
This is just an extra safeguard – no JS should run once we’ve
decided that we are going to shutdown the process, but we’re opening
a handle scope in order to access V8, so make sure that our operations
do not lead to JS accidentally being run.

PR-URL: #35020
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
richardlau pushed a commit that referenced this pull request Sep 7, 2020
@addaleax @richardlau
src: disallow JS execution during exit()
7ce9690 
This is just an extra safeguard – no JS should run once we’ve
decided that we are going to shutdown the process, but we’re opening
a handle scope in order to access V8, so make sure that our operations
do not lead to JS accidentally being run.

PR-URL: #35020
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
@richardlau richardlau mentioned this pull request Sep 7, 2020
Merged
4 tasks
richardlau pushed a commit that referenced this pull request Sep 7, 2020
@addaleax @richardlau
src: disallow JS execution during exit()
73ef3f2 
This is just an extra safeguard – no JS should run once we’ve
decided that we are going to shutdown the process, but we’re opening
a handle scope in order to access V8, so make sure that our operations
do not lead to JS accidentally being run.

PR-URL: #35020
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
addaleax added a commit that referenced this pull request Sep 22, 2020
@addaleax
src: disallow JS execution during exit()
ae800da 
This is just an extra safeguard – no JS should run once we’ve
decided that we are going to shutdown the process, but we’re opening
a handle scope in order to access V8, so make sure that our operations
do not lead to JS accidentally being run.

PR-URL: #35020
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
addaleax added a commit that referenced this pull request Sep 22, 2020
@addaleax
src: disallow JS execution during exit()
00e020b 
This is just an extra safeguard – no JS should run once we’ve
decided that we are going to shutdown the process, but we’re opening
a handle scope in order to access V8, so make sure that our operations
do not lead to JS accidentally being run.

PR-URL: #35020
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
@codebytere codebytere mentioned this pull request Sep 28, 2020
Merged
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
@addaleax @joesepi
src: disallow JS execution during exit()
2ac8e3c 
This is just an extra safeguard – no JS should run once we’ve
decided that we are going to shutdown the process, but we’re opening
a handle scope in order to access V8, so make sure that our operations
do not lead to JS accidentally being run.

PR-URL: nodejs#35020
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@codebytere codebytere codebytere approved these changes

@cjihrig cjihrig cjihrig approved these changes

@devnexen devnexen devnexen approved these changes

@devsnek devsnek devsnek approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@addaleax @nodejs-github-bot @codebytere @jasnell @cjihrig @devnexen @devsnek