process,worker: ensure code after exit() effectless #45620

ywave620 · 2022-11-25T07:56:35Z

Cope with the delay(to the next function call) of v8::Isolate::TerminateExecution()

ywave620 · 2022-11-25T07:59:33Z

test/parallel/test-async-hooks-worker-asyncfn-terminate-4.js

@@ -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');


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.

nodejs-github-bot · 2022-11-26T10:52:29Z

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

nodejs-github-bot · 2022-11-27T07:58:57Z

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/.ncu

https://github.com/nodejs/node/actions/runs/3557316810

nodejs-github-bot · 2022-11-27T08:27:34Z

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

nodejs-github-bot · 2022-11-27T19:01:16Z

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

Cope with the delay(to the next function call) of v8::Isolate::TerminateExecution()

ywave620 · 2022-11-28T11:17:27Z

spurious failure

[ RUN      ] InspectorSocketTest.HostIPv6NonRoutable
../test/cctest/test_inspector_socket.cc:323: Failure
Value of: (!expectation.read_expected)
  Actual: true
Expected: false

fix typo

ignore exception that indicaties a termination in napi call

ywave620 · 2022-12-10T11:31:53Z

Could you take a look at the CI failure?

https://ci.nodejs.org/job/node-test-commit-arm-debug/5274/nodes=ubuntu2004_debug-arm64/testReport/junit/(root)/test/node_api_test_worker_terminate_test/

Fix. PTAL, especially change in src

nodejs-github-bot · 2022-12-10T12:38:41Z

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

ywave620 · 2022-12-12T01:55:54Z

src/js_native_api_v8.h

    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();


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().

nodejs-github-bot · 2022-12-12T16:06:15Z

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

nodejs-github-bot · 2022-12-25T09:54:13Z

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>

nodejs-github-bot added needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem. labels Nov 25, 2022

ywave620 commented Nov 25, 2022

View reviewed changes

addaleax approved these changes Nov 25, 2022

View reviewed changes

anonrig approved these changes Nov 25, 2022

View reviewed changes

aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. commit-queue Add this label to land a pull request using GitHub Actions. labels Nov 26, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 26, 2022

github-actions bot mentioned this pull request Nov 27, 2022

CI Reliability 2022-11-27 nodejs/reliability#441

Open

21 tasks

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 27, 2022

nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Nov 27, 2022

jasnell approved these changes Nov 27, 2022

View reviewed changes

cjihrig approved these changes Nov 27, 2022

View reviewed changes

github-actions bot mentioned this pull request Nov 28, 2022

CI Reliability 2022-11-28 nodejs/reliability#442

Open

20 tasks

process,worker: ensure code after exit() effectless

de62467

Cope with the delay(to the next function call) of v8::Isolate::TerminateExecution()

ywave620 force-pushed the ensure-exit branch from c7f350d to 5bf6a3c Compare November 28, 2022 09:16

ywave620 changed the title ~~process,worker: ensure code followed by exit() effectless~~ process,worker: ensure code after exit() effectless Nov 28, 2022

JungMinu approved these changes Nov 28, 2022

View reviewed changes

This was referenced Nov 29, 2022

CI Reliability 2022-11-29 nodejs/reliability#443

Open

CI Reliability 2022-11-30 nodejs/reliability#444

Open

CI Reliability 2022-12-01 nodejs/reliability#445

Open

CI Reliability 2022-12-02 nodejs/reliability#446

Open

process,worker: ensure code after exit() effectless

37cccd0

fix typo

ywave620 force-pushed the ensure-exit branch from 5bf6a3c to 37cccd0 Compare December 2, 2022 01:56

github-actions bot mentioned this pull request Dec 10, 2022

CI Reliability 2022-12-10 nodejs/reliability#453

Open

9 tasks

process,worker: ensure code after exit() effectless

0c91220

ignore exception that indicaties a termination in napi call

ywave620 force-pushed the ensure-exit branch from f3b73e2 to 0c91220 Compare December 10, 2022 11:31

daeyeon added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 10, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 10, 2022

This was referenced Dec 11, 2022

CI Reliability 2022-12-11 nodejs/reliability#454

Open

CI Reliability 2022-12-12 nodejs/reliability#455

Open

ywave620 commented Dec 12, 2022

View reviewed changes

daeyeon added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 12, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 12, 2022

github-actions bot mentioned this pull request Dec 13, 2022

CI Reliability 2022-12-13 nodejs/reliability#456

Open

13 tasks

ywave620 requested review from addaleax and aduh95 and removed request for addaleax December 25, 2022 03:05

aduh95 approved these changes Dec 25, 2022

View reviewed changes

aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Dec 25, 2022

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 25, 2022

nodejs-github-bot merged commit 8438f3b into nodejs:main Dec 25, 2022

RafaelGSS mentioned this pull request Jan 2, 2023

v19.4.0 proposal #46061

Merged

juanarbol mentioned this pull request Jan 28, 2023

V18.14.0 proposal #46396

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

process,worker: ensure code after exit() effectless #45620

process,worker: ensure code after exit() effectless #45620

ywave620 commented Nov 25, 2022

ywave620 Nov 25, 2022 •

edited

nodejs-github-bot commented Nov 26, 2022

nodejs-github-bot commented Nov 27, 2022

nodejs-github-bot commented Nov 27, 2022

nodejs-github-bot commented Nov 27, 2022

ywave620 commented Nov 28, 2022

ywave620 commented Dec 10, 2022 •

edited

nodejs-github-bot commented Dec 10, 2022

ywave620 Dec 12, 2022

nodejs-github-bot commented Dec 12, 2022

nodejs-github-bot commented Dec 25, 2022

process,worker: ensure code after exit() effectless #45620

process,worker: ensure code after exit() effectless #45620

Conversation

ywave620 commented Nov 25, 2022

ywave620 Nov 25, 2022 • edited

Choose a reason for hiding this comment

nodejs-github-bot commented Nov 26, 2022

nodejs-github-bot commented Nov 27, 2022

nodejs-github-bot commented Nov 27, 2022

nodejs-github-bot commented Nov 27, 2022

ywave620 commented Nov 28, 2022

ywave620 commented Dec 10, 2022 • edited

nodejs-github-bot commented Dec 10, 2022

ywave620 Dec 12, 2022

Choose a reason for hiding this comment

nodejs-github-bot commented Dec 12, 2022

nodejs-github-bot commented Dec 25, 2022

ywave620 Nov 25, 2022 •

edited

ywave620 commented Dec 10, 2022 •

edited