-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
process,worker: ensure code after exit() effectless #45620
Conversation
@@ -12,6 +12,7 @@ const { Worker } = require('worker_threads'); | |||
const workerData = new Int32Array(new SharedArrayBuffer(4)); | |||
const w = new Worker(` | |||
const { createHook } = require('async_hooks'); | |||
const { workerData } = require('worker_threads'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the original author forgot to add this import. And this is why the strictEqual
below paased, because adding to the workerData
raises an reference exception.
Commit Queue failed- Loading data for nodejs/node/pull/45620 ✔ Done loading data for nodejs/node/pull/45620 ----------------------------------- PR info ------------------------------------ Title process,worker: ensure code followed by exit() effectless (#45620) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch ywave620:ensure-exit -> nodejs:main Labels process, author ready, needs-ci Commits 1 - process,worker: ensure code followed by exit() effectless Committers 1 - ywave620 PR-URL: https://github.com/nodejs/node/pull/45620 Reviewed-By: Anna Henningsen Reviewed-By: Yagiz Nizipli ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/45620 Reviewed-By: Anna Henningsen Reviewed-By: Yagiz Nizipli -------------------------------------------------------------------------------- ℹ This PR was created on Fri, 25 Nov 2022 07:56:35 GMT ✔ Approvals: 2 ✔ - Anna Henningsen (@addaleax): https://github.com/nodejs/node/pull/45620#pullrequestreview-1194537461 ✔ - Yagiz Nizipli (@anonrig): https://github.com/nodejs/node/pull/45620#pullrequestreview-1194617601 ✖ Last GitHub CI failed ℹ Last Full PR CI on 2022-11-26T10:52:29Z: https://ci.nodejs.org/job/node-test-pull-request/48195/ - Querying data for job/node-test-pull-request/48195/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/3557316810 |
Cope with the delay(to the next function call) of v8::Isolate::TerminateExecution()
c7f350d
to
5bf6a3c
Compare
spurious failure
|
ignore exception that indicaties a termination in napi call
f3b73e2
to
0c91220
Compare
Fix. PTAL, especially change in |
env->isolate->ThrowException(value); | ||
} | ||
|
||
// i.e. whether v8 exited or is about to exit | ||
inline bool terminatedOrTerminating() { | ||
return this->isolate->IsExecutionTerminating() || !can_call_into_js(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that if this->isolate->IsExecutionTerminating()
is true, we can not call can_call_into_js()
, because that will extract the node::Environment
by calling v8 API on this->isolate
, which will again hit the assertion:
# Fatal error in ../deps/v8/src/api/api.cc, line 8635
# Debug check failed: !i_isolate->is_execution_terminating().
Landed in 8438f3b |
Cope with the delay(to the next function call) of v8::Isolate::TerminateExecution() PR-URL: #45620 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Cope with the delay(to the next function call) of v8::Isolate::TerminateExecution() PR-URL: #45620 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Cope with the delay(to the next function call) of v8::Isolate::TerminateExecution() PR-URL: #45620 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Cope with the delay(to the next function call) of v8::Isolate::TerminateExecution() PR-URL: #45620 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Cope with the delay(to the next function call) of v8::Isolate::TerminateExecution() PR-URL: #45620 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Cope with the delay(to the next function call) of v8::Isolate::TerminateExecution()