FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

wasi,worker: handle termination exception by bnoordhuis · Pull Request #33386 · nodejs/node · GitHub

/ node Public

wasi,worker: handle termination exception - #33386

Closed
bnoordhuis wants to merge 2 commits into
nodejs:masterfrom
bnoordhuis:fix33377
ConversationCommits (2)ChecksFiles changed
Closed

wasi,worker: handle termination exception#33386
bnoordhuis wants to merge 2 commits into
nodejs:masterfrom
bnoordhuis:fix33377

Conversation

bnoordhuis commented May 13, 2020

Copy link
Copy Markdown
Member

Fixes: #33377

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. worker Issues and PRs related to Worker support. labels May 13, 2020
addaleax requested changes May 13, 2020
View reviewed changes
Comment thread test/wasi/test-wasi-worker-terminate.js Outdated
Comment thread src/node_worker.cc Outdated

cjihrig commented May 16, 2020

Copy link
Copy Markdown
Contributor

@bnoordhuis are you still planning to work on this? I'm willing to implement #33386 (comment) if you'd like.

bnoordhuis added 2 commits May 16, 2020 11:52
src: remove BeforeExit callback list
5660096 
It obscures the fact that there is only a single BeforeExit action.
Just call that statically from `EmitBeforeExit()`.
wasi,worker: handle termination exception
c46bbdd 
Be careful when emitting the 'beforeExit' event. It's not allowed to
call into the runtime when a termination exception is pending.

Fixes: nodejs#33377
bnoordhuis commented May 16, 2020
View reviewed changes
Comment thread src/api/hooks.cc
void EmitBeforeExit(Environment* env) {
env->RunBeforeExitCallbacks();
TraceEventScope trace_scope(TRACING_CATEGORY_NODE1(environment),
"BeforeExit", env);

bnoordhuis May 16, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This changes the lifetime of the scope a little but makes it more correct, IMO. In the old location it only covered part of the BeforeExit sequence.

addaleax added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label May 20, 2020

nodejs-github-bot commented May 20, 2020 •
edited by BridgeAR
Loading

Copy link
Copy Markdown
Collaborator

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

BridgeAR commented May 23, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Landed in a3f47b1...d85a933 🎉

BridgeAR closed this May 23, 2020
BridgeAR pushed a commit that referenced this pull request May 23, 2020
src: remove BeforeExit callback list
8ddb6aa 
It obscures the fact that there is only a single BeforeExit action.
Just call that statically from `EmitBeforeExit()`.

PR-URL: #33386
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR pushed a commit that referenced this pull request May 23, 2020
wasi,worker: handle termination exception
d85a933 
Be careful when emitting the 'beforeExit' event. It's not allowed to
call into the runtime when a termination exception is pending.

Fixes: #33377

PR-URL: #33386
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this pull request Jun 18, 2020
src: remove BeforeExit callback list
f669ee2 
It obscures the fact that there is only a single BeforeExit action.
Just call that statically from `EmitBeforeExit()`.

PR-URL: #33386
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this pull request Jun 18, 2020
wasi,worker: handle termination exception
b8a696b 
Be careful when emitting the 'beforeExit' event. It's not allowed to
call into the runtime when a termination exception is pending.

Fixes: #33377

PR-URL: #33386
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this pull request Jun 18, 2020
src: remove BeforeExit callback list
d1e1dbf 
It obscures the fact that there is only a single BeforeExit action.
Just call that statically from `EmitBeforeExit()`.

PR-URL: #33386
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this pull request Jun 18, 2020
wasi,worker: handle termination exception
d153832 
Be careful when emitting the 'beforeExit' event. It's not allowed to
call into the runtime when a termination exception is pending.

Fixes: #33377

PR-URL: #33386
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere mentioned this pull request Jun 28, 2020
Merged
codebytere pushed a commit that referenced this pull request Jun 30, 2020
src: remove BeforeExit callback list
e0680cc 
It obscures the fact that there is only a single BeforeExit action.
Just call that statically from `EmitBeforeExit()`.

PR-URL: #33386
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this pull request Jun 30, 2020
wasi,worker: handle termination exception
3d7d6d4 
Be careful when emitting the 'beforeExit' event. It's not allowed to
call into the runtime when a termination exception is pending.

Fixes: #33377

PR-URL: #33386
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this pull request Jul 8, 2020
src: remove BeforeExit callback list
77b6298 
It obscures the fact that there is only a single BeforeExit action.
Just call that statically from `EmitBeforeExit()`.

PR-URL: #33386
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this pull request Jul 8, 2020
wasi,worker: handle termination exception
7df79f4 
Be careful when emitting the 'beforeExit' event. It's not allowed to
call into the runtime when a termination exception is pending.

Fixes: #33377

PR-URL: #33386
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere mentioned this pull request Jul 13, 2020
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
addaleax addaleax approved these changes
benjamingr benjamingr approved these changes
cjihrig cjihrig approved these changes
richardlau richardlau approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. worker Issues and PRs related to Worker support.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

WASI: Crash when terminating a worker thread

8 participants

Back | FazBrowse Home | New Git URL