GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — lib: do not crash using workers with disabled shared array buffers by BridgeAR · Pull Request #41023 · nodejs/node

Conversation

BridgeAR commented Nov 29, 2021 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

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

Fixes: #39717

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bricss reacted with thumbs up emoji

All reactions

👍 1 reaction

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 the worker_threads module and Worker API. labels

Nov 29, 2021

jasnell approved these changes

Nov 29, 2021

View reviewed changes

tniessen approved these changes

Nov 29, 2021

View reviewed changes

tniessen left a comment

Copy link

Copy Markdown

Member

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

BridgeAR commented Nov 30, 2021

Copy link

Copy Markdown

Member Author

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.

tniessen reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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 a39f548 Compare

December 4, 2021 01:23

BridgeAR added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Dec 6, 2021

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Dec 6, 2021

ghost commented Dec 6, 2021

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

github-actions Bot mentioned this pull request

Dec 7, 2021

CI Reliability 2021-12-07 nodejs/reliability#137

Open

46 tasks

ghost commented Dec 7, 2021

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

github-actions Bot mentioned this pull request

Dec 8, 2021

CI Reliability 2021-12-08 nodejs/reliability#138

Open

47 tasks

ghost commented Dec 8, 2021

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

BridgeAR force-pushed the do-not-crash-with-disabled-shared-array-buffer branch from a39f548 to f26a4d0 Compare

December 8, 2021 18:40

BridgeAR added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Dec 8, 2021

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Dec 8, 2021

ghost commented Dec 8, 2021

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

github-actions Bot mentioned this pull request

Dec 9, 2021

CI Reliability 2021-12-09 nodejs/reliability#139

Open

46 tasks

ghost commented Dec 9, 2021

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

ghost commented Dec 11, 2021

Copy link

Copy Markdown

Member Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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 ca3ed36 Compare

November 12, 2022 01:49


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

ed3865f

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]>

BridgeAR force-pushed the do-not-crash-with-disabled-shared-array-buffer branch from 9c2ceb6 to ed3865f Compare

February 17, 2023 18:29

BridgeAR added request-ci

Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review.

author ready PRs with CI started, the required approvals, and no outstanding review comments. labels

Feb 17, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Feb 17, 2023

ghost commented Feb 17, 2023

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

aduh95 approved these changes

Feb 17, 2023

View reviewed changes

ghost commented Feb 17, 2023

Copy link

Copy Markdown

Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ghost commented Feb 17, 2023

Copy link

Copy Markdown

Member Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ghost commented Feb 18, 2023

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ghost commented Feb 18, 2023

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

aduh95 added the commit-queue PRs queued for automated landing through the Commit Queue. label

Feb 18, 2023

nodejs-github-bot added commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. and removed commit-queue PRs queued for automated landing through the Commit Queue. labels

Feb 18, 2023

ghost commented Feb 18, 2023

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

aduh95 merged commit fbd55a8 into nodejs:main

Feb 18, 2023

ghost commented Feb 18, 2023

Copy link

Copy Markdown

Contributor

Landed in fbd55a8

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

Reviewers

jasnell

jasnell approved these changes

tniessen

tniessen approved these changes

aduh95

aduh95 approved these changes

addaleax

Awaiting requested review from addaleax

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready

PRs with CI started, the required approvals, and no outstanding review comments.

commit-queue-failed

PRs whose Commit Queue landing failed and need manual intervention before retrying.

needs-ci

PRs that need a full CI run.

worker

Issues and PRs related to the worker_threads module and Worker API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Repl crash when SharedArrayBuffers are disabled

Uh oh!

There was an error while loading. Please reload this page.

6 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

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!

ghost commented Dec 6, 2021

Uh oh!

ghost commented Dec 7, 2021

Uh oh!

ghost commented Dec 8, 2021

Uh oh!

ghost commented Dec 8, 2021

Uh oh!

ghost commented Dec 9, 2021

Uh oh!

ghost commented Dec 11, 2021

Uh oh!

ghost commented Feb 17, 2023

Uh oh!

ghost commented Feb 17, 2023

Uh oh!

ghost commented Feb 17, 2023

Uh oh!

ghost commented Feb 18, 2023

Uh oh!

ghost commented Feb 18, 2023

Uh oh!

ghost commented Feb 18, 2023

Uh oh!

ghost commented Feb 18, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

BridgeAR commented Nov 29, 2021 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.