lib: do not crash using workers with disabled shared array buffers#41023

BridgeAR · 2021-11-29T21:01:08Z

This allows the repl to function normally while using the
--no-harmony-sharedarraybuffer V8 flag.

Signed-off-by: Ruben Bridgewater [email protected]

tniessen

It would be great if you could add a comment to these explaining why it's necessary since I don't think it's going to be obvious to someone reading the code. Especially the first occurrence that does not reference SAB directly.

BridgeAR · 2021-11-30T11:31:40Z

I missed the prior work by @codebytere #39718. I might also have found the issue that blocked the PR from being merged. I'll try that later on.

nodejs-github-bot · 2021-12-06T21:00:44Z

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

nodejs-github-bot · 2021-12-07T22:59:59Z

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

nodejs-github-bot · 2021-12-08T11:24:39Z

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

nodejs-github-bot · 2021-12-08T18:48:05Z

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

nodejs-github-bot · 2021-12-09T13:25:31Z

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

BridgeAR · 2021-12-11T11:52:38Z

I unblocked this again to move it forward. @jasnell @tniessen are you fine landing this instead of waiting for #39718?

This allows the repl to function normally while using the `--no-harmony-sharedarraybuffer` V8 flag. It also fixes using workers while using the `--no-harmony-atomics` V8 flag. Fixes: nodejs#39717 Signed-off-by: Ruben Bridgewater <[email protected]> Co-authored-by: Shelley Vohr <[email protected]>

nodejs-github-bot · 2023-02-17T18:36:30Z

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

codebytere · 2023-02-17T18:55:19Z

@BridgeAR is this still something you want to move forward? We still float the patch so it's definitely still useful to Electron!

BridgeAR · 2023-02-17T18:57:12Z

@codebytere I just rebased the branch and started the CI to land this :)

nodejs-github-bot · 2023-02-18T02:20:28Z

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

nodejs-github-bot · 2023-02-18T08:32:07Z

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

nodejs-github-bot · 2023-02-18T10:15:22Z

Commit Queue failed

- Loading data for nodejs/node/pull/41023 ✔ Done loading data for nodejs/node/pull/41023 ----------------------------------- PR info ------------------------------------ Title lib: do not crash using workers with disabled shared array buffers (#41023) Author Ruben Bridgewater (@BridgeAR) Branch BridgeAR:do-not-crash-with-disabled-shared-array-buffer -> nodejs:main Labels author ready, worker, needs-ci Commits 1 - lib: do not crash using workers with disabled shared array buffers Committers 1 - Ruben Bridgewater PR-URL: https://github.com/nodejs/node/pull/41023 Fixes: https://github.com/nodejs/node/issues/39717 Reviewed-By: James M Snell Reviewed-By: Tobias Nießen Reviewed-By: Antoine du Hamel ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/41023 Fixes: https://github.com/nodejs/node/issues/39717 Reviewed-By: James M Snell Reviewed-By: Tobias Nießen Reviewed-By: Antoine du Hamel -------------------------------------------------------------------------------- ℹ This PR was created on Mon, 29 Nov 2021 21:01:08 GMT ✔ Approvals: 3 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/41023#pullrequestreview-818336803 ✔ - Tobias Nießen (@tniessen) (TSC): https://github.com/nodejs/node/pull/41023#pullrequestreview-818394046 ✔ - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/41023#pullrequestreview-1304031055 ✖ Last GitHub CI failed ℹ Last Full PR CI on 2023-02-18T08:32:07Z: https://ci.nodejs.org/job/node-test-pull-request/49675/ - Querying data for job/node-test-pull-request/49675/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu

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

aduh95 · 2023-02-18T10:18:12Z

Landed in fbd55a8

This allows the repl to function normally while using the `--no-harmony-sharedarraybuffer` V8 flag. It also fixes using workers while using the `--no-harmony-atomics` V8 flag. Fixes: #39717 Signed-off-by: Ruben Bridgewater <[email protected]> Co-authored-by: Shelley Vohr <[email protected]> PR-URL: #41023 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

nodejs/node#41023

* chore: bump node in DEPS to v18.16.0 * build,test: add proper support for IBM i nodejs/node#46739 * lib: enforce use of trailing commas nodejs/node#46881 * src: add initial support for single executable applications nodejs/node#45038 * lib: do not crash using workers with disabled shared array buffers nodejs/node#41023 * src: remove shadowed variable in OptionsParser::Parse nodejs/node#46672 * src: allow embedder control of code generation policy nodejs/node#46368 * src: allow optional Isolate termination in node::Stop() nodejs/node#46583 * lib: fix BroadcastChannel initialization location nodejs/node#46864 * chore: fixup patch indices * chore: sync filenames.json * fix: add simdutf dep to src/inspector BUILD.gn - nodejs/node#46471 - nodejs/node#46472 * deps: replace url parser with Ada nodejs/node#46410 * tls: support automatic DHE nodejs/node#46978 * fixup! src: add initial support for single executable applications * http: unify header treatment nodejs/node#46528 * fix: libc++ buffer overflow in string_view ctor nodejs/node#46410 * test: include strace openat test nodejs/node#46150 * fixup! fixup! src: add initial support for single executable applications --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <[email protected]>

BridgeAR requested a review from addaleax November 29, 2021 21:01

nodejs-github-bot added needs-ci PRs that need a full CI run. worker Issues and PRs related to Worker support. labels Nov 29, 2021

jasnell approved these changes Nov 29, 2021
View reviewed changes

tniessen approved these changes Nov 29, 2021
View reviewed changes

BridgeAR added the blocked PRs that are blocked by other issues or PRs. label Nov 30, 2021

BridgeAR force-pushed the do-not-crash-with-disabled-shared-array-buffer branch 3 times, most recently from 73059c2 to a39f548Compare December 4, 2021 01:23

BridgeAR added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 6, 2021

github-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 6, 2021

github-actionsbot mentioned this pull request Dec 7, 2021
CI Reliability 2021-12-07 nodejs/reliability#137
Open
46 tasks

github-actionsbot mentioned this pull request Dec 8, 2021
CI Reliability 2021-12-08 nodejs/reliability#138
Open
47 tasks

BridgeAR force-pushed the do-not-crash-with-disabled-shared-array-buffer branch from a39f548 to f26a4d0Compare December 8, 2021 18:40

BridgeAR added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 8, 2021

github-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 8, 2021

github-actionsbot mentioned this pull request Dec 9, 2021
CI Reliability 2021-12-09 nodejs/reliability#139
Open
46 tasks

This was referenced Dec 10, 2021
CI Reliability 2021-12-10 nodejs/reliability#140
Open
CI Reliability 2021-12-11 nodejs/reliability#141
Open

BridgeAR removed the blocked PRs that are blocked by other issues or PRs. label Dec 11, 2021

This was referenced Dec 12, 2021
CI Reliability 2021-12-12 nodejs/reliability#142
Open
CI Reliability 2021-12-13 nodejs/reliability#143
Open
CI Reliability 2021-12-14 nodejs/reliability#144
Open

This was referenced Sep 1, 2022
CI Reliability 2022-09-01 nodejs/reliability#364
Open
CI Reliability 2022-09-02 nodejs/reliability#365
Open
CI Reliability 2022-09-03 nodejs/reliability#366
Open

codebytere mentioned this pull request Sep 13, 2022
repl,worker: fix crash when SharedArrayBuffer disabled #39718
Closed

Trott force-pushed the main branch from 2d76238 to ca3ed36Compare November 12, 2022 01:49

BridgeAR force-pushed the do-not-crash-with-disabled-shared-array-buffer branch from 9c2ceb6 to ed3865fCompare February 17, 2023 18:29

BridgeAR added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Feb 17, 2023

github-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 17, 2023

aduh95 approved these changes Feb 17, 2023
View reviewed changes

aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 18, 2023

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Feb 18, 2023

aduh95 merged commit fbd55a8 into nodejs:mainFeb 18, 2023

MylesBorins mentioned this pull request Feb 19, 2023
v19.7.0 proposal #46725
Merged

danielleadams mentioned this pull request Apr 11, 2023
v18.16.0 release proposal #47502
Merged

codebytere added a commit to electron/electron that referenced this pull request Apr 14, 2023
lib: do not crash using workers with disabled shared array buffers
7705571
nodejs/node#41023

codebytere added a commit to electron/electron that referenced this pull request Apr 17, 2023
lib: do not crash using workers with disabled shared array buffers
a1b4f3b
nodejs/node#41023

Uh oh!

lib: do not crash using workers with disabled shared array buffers#41023

lib: do not crash using workers with disabled shared array buffers #41023

Uh oh!

Conversation

BridgeAR commented Nov 29, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tniessen left a comment

Choose a reason for hiding this comment

Uh oh!

BridgeAR commented Nov 30, 2021

Uh oh!

nodejs-github-bot commented Dec 6, 2021

Uh oh!

nodejs-github-bot commented Dec 7, 2021

Uh oh!

nodejs-github-bot commented Dec 8, 2021

Uh oh!

nodejs-github-bot commented Dec 8, 2021

Uh oh!

nodejs-github-bot commented Dec 9, 2021

Uh oh!

BridgeAR commented Dec 11, 2021

Uh oh!

nodejs-github-bot commented Feb 17, 2023

Uh oh!

codebytere commented Feb 17, 2023

Uh oh!

BridgeAR commented Feb 17, 2023

Uh oh!

nodejs-github-bot commented Feb 18, 2023

Uh oh!

nodejs-github-bot commented Feb 18, 2023

Uh oh!

nodejs-github-bot commented Feb 18, 2023

Uh oh!

aduh95 commented Feb 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

BridgeAR commented Nov 29, 2021•
edited
Loading