worker: implement worker.moveMessagePortToContext() by addaleax · Pull Request #26497 · nodejs/node · GitHub

src,lib: allow running multiple per-context files

Create an lib/internal/per_context/ directory that can
host multiple files which we execute for each context.

src,lib: make DOMException available in all Contexts

This allows using DOMException from Node.js code for any
vm.Context.

worker: implement worker.moveMessagePortToContext()

This enables using MessagePorts in different vm.Contexts,
aiding with the isolation that the vm module seeks to provide.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

@addaleax build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/2800/pipeline

/cc @nodejs/workers

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

@chjj I’m not a 100 % sure but this might also make it easier to implement something closer to a “real” WebWorker-style Environment in Node.js, because you can control the global object and still have MessagePorts in it?

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

Rebased, new CI: https://ci.nodejs.org/job/node-test-pull-request/21369/

/cc @nodejs/workers

The code generally LGTM but I have some questions

Can we use context->GetExtrasBindingObject() to store these somehow instead of using v8 privates (which are...an experimental feature. Use at your own risk.)? V8 already puts isTraceCategoryEnabled and trace in there.

We’ve been using V8 privates extensively for a while, I don’t think they’re going away (and if they did we’d have bigger problems, e.g. N-API relies on them as well).

But yes, if you prefer, we could use the extras binding object for that … I feel like that’s mostly intended for actual v8-extras, but in theory nothing stops us, and I don’t mind making the switch if you prefer.

@addaleax I don't have very strong opinions about the choices here...although I wonder what does this look like in the heap snapshot? Do they show up?

BTW, we probably need to do something similar for the Environment persistent handles (save them temporarily to the global proxy) when we implement snapshots...

Rebased again, CI: https://ci.nodejs.org/job/node-test-pull-request/21464/

CI is green.

I’ll land this tomorrow if there are no objections, but I’d appreciate another review from @nodejs/workers.

@addaleax, yeah, that's a possibility. Someone could replicate the web worker scope exactly with vm I suppose. It's not something I'm personally interested in though since I think I'm more focused on making the browser behave like node.js instead of the other way around (though, my approach seems to be less popular these days).

- nodejs/node#26497

Create an `lib/internal/per_context/` directory that can host multiple files which we execute for each context. PR-URL: nodejs#26497 Reviewed-By: James M Snell <jasnell@gmail.com>

This allows using `DOMException` from Node.js code for any `vm.Context`. PR-URL: nodejs#26497 Reviewed-By: James M Snell <jasnell@gmail.com>

This enables using `MessagePort`s in different `vm.Context`s, aiding with the isolation that the `vm` module seeks to provide. Refs: ayojs/ayo#111 PR-URL: nodejs#26497 Reviewed-By: James M Snell <jasnell@gmail.com>

Sigh… Needed to update this because of test/parallel/test-bootstrap-modules.js.

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

Landed in 0752a18...fe8972a

Create an `lib/internal/per_context/` directory that can host multiple files which we execute for each context. PR-URL: #26497 Reviewed-By: James M Snell <jasnell@gmail.com>

This allows using `DOMException` from Node.js code for any `vm.Context`. PR-URL: #26497 Reviewed-By: James M Snell <jasnell@gmail.com>

This enables using `MessagePort`s in different `vm.Context`s, aiding with the isolation that the `vm` module seeks to provide. Refs: ayojs/ayo#111 PR-URL: #26497 Reviewed-By: James M Snell <jasnell@gmail.com>

Create an `lib/internal/per_context/` directory that can host multiple files which we execute for each context. PR-URL: #26497 Reviewed-By: James M Snell <jasnell@gmail.com>

This allows using `DOMException` from Node.js code for any `vm.Context`. PR-URL: #26497 Reviewed-By: James M Snell <jasnell@gmail.com>

This enables using `MessagePort`s in different `vm.Context`s, aiding with the isolation that the `vm` module seeks to provide. Refs: ayojs/ayo#111 PR-URL: #26497 Reviewed-By: James M Snell <jasnell@gmail.com>

Notable changes: * crypto * Allow deriving public from private keys (Tobias Nießen) [#26278](#26278). * events * Added a `once` function to use `EventEmitter` with promises (Matteo Collina) [#26078](#26078). * tty * Added a `hasColors` method to `WriteStream` (Ruben Bridgewater) [#26247](#26247). * Added NO_COLOR and FORCE_COLOR support (Ruben Bridgewater) [#26485](#26485). * v8 * Added `v8.getHeapSnapshot` and `v8.writeHeapSnapshot` to generate snapshots in the format used by tools such as Chrome DevTools (James M Snell) [#26501](#26501). * worker * Added `worker.moveMessagePortToContext`. This enables using MessagePorts in different vm.Contexts, aiding with the isolation that the vm module seeks to provide (Anna Henningsen) [#26497](#26497). * C++ API * `AddPromiseHook` is now deprecated. This API was added to fill an use case that is served by `async_hooks`, since that has `Promise` support (Anna Henningsen) [#26529](#26529). * Added a `Stop` API to shut down Node.js while it is running (Gireesh Punathil) [#21283](#21283). * meta * [Gireesh Punathil](https://github.com/gireeshpunathil) is now a member of the Technical Steering Committee [#26657](#26657). * Added [Yongsheng Zhang](https://github.com/ZYSzys) to collaborators [#26730](#26730). PR-URL: #26949

addaleax added vm Issues and PRs related to the vm subsystem. semver-minor PRs that contain new features and should be released in the next minor version. worker Issues and PRs related to Worker support. labels Mar 7, 2019

nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Mar 7, 2019

addaleax force-pushed the worker-messageport-move branch 4 times, most recently from b58679b to 1153455 Compare March 7, 2019 15:47

jasnell approved these changes Mar 7, 2019

View reviewed changes

addaleax requested a review from joyeecheung March 8, 2019 11:44

BridgeAR added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Mar 8, 2019

gengjiawen reviewed Mar 8, 2019

View reviewed changes

Comment thread test/parallel/test-worker-message-port-move.js Outdated

gengjiawen approved these changes Mar 8, 2019

View reviewed changes

addaleax force-pushed the worker-messageport-move branch from e8b1b4a to bd0ef7a Compare March 9, 2019 11:55

joyeecheung reviewed Mar 9, 2019

View reviewed changes

addaleax force-pushed the worker-messageport-move branch 3 times, most recently from 76070c3 to 8937fd6 Compare March 12, 2019 15:49

chjj added a commit to chjj/bthreads that referenced this pull request Mar 14, 2019

bthreads: expose moveMessagePortToContext.

fb36b9d

- nodejs/node#26497

addaleax added 2 commits March 15, 2019 15:18

src,lib: allow running multiple per-context files

92cddba

Create an `lib/internal/per_context/` directory that can host multiple files which we execute for each context. PR-URL: nodejs#26497 Reviewed-By: James M Snell <jasnell@gmail.com>

src,lib: make DOMException available in all Contexts

a15fd18

This allows using `DOMException` from Node.js code for any `vm.Context`. PR-URL: nodejs#26497 Reviewed-By: James M Snell <jasnell@gmail.com>

worker: implement worker.moveMessagePortToContext()

23bf4ce

This enables using `MessagePort`s in different `vm.Context`s, aiding with the isolation that the `vm` module seeks to provide. Refs: ayojs/ayo#111 PR-URL: nodejs#26497 Reviewed-By: James M Snell <jasnell@gmail.com>

addaleax force-pushed the worker-messageport-move branch from 8937fd6 to 23bf4ce Compare March 15, 2019 14:34

addaleax closed this Mar 15, 2019

addaleax deleted the worker-messageport-move branch March 15, 2019 15:57

addaleax added a commit that referenced this pull request Mar 15, 2019

src,lib: make DOMException available in all Contexts

b0de48e

This allows using `DOMException` from Node.js code for any `vm.Context`. PR-URL: #26497 Reviewed-By: James M Snell <jasnell@gmail.com>

targos pushed a commit that referenced this pull request Mar 27, 2019

src,lib: make DOMException available in all Contexts

1e669b2

This allows using `DOMException` from Node.js code for any `vm.Context`. PR-URL: #26497 Reviewed-By: James M Snell <jasnell@gmail.com>

targos mentioned this pull request Mar 27, 2019

v11.13.0 release proposal #26949

Merged

BethGriggs added the lts-watch-v10.x label Aug 20, 2019

BethGriggs added the backport-requested-v10.x label Oct 17, 2019

worker: implement worker.moveMessagePortToContext() by addaleax · Pull Request #26497 · nodejs/node · GitHub

Uh oh!

Uh oh!

worker: implement worker.moveMessagePortToContext() - #26497

worker: implement worker.moveMessagePortToContext() #26497addaleax wants to merge 3 commits intonodejs:masternodejs/node:masterfrom addaleax:worker-messageport-moveaddaleax/node:worker-messageport-moveCopy head branch name to clipboard

Conversation

addaleax commented Mar 7, 2019

src,lib: allow running multiple per-context files

src,lib: make DOMException available in all Contexts

worker: implement worker.moveMessagePortToContext()

Checklist

Uh oh!

nodejs-github-bot commented Mar 7, 2019

Uh oh!

addaleax commented Mar 7, 2019

Uh oh!

addaleax commented Mar 7, 2019

Uh oh!

BridgeAR commented Mar 8, 2019

Uh oh!

Uh oh!

addaleax commented Mar 9, 2019

Uh oh!

addaleax commented Mar 9, 2019

Uh oh!

joyeecheung left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

joyeecheung Mar 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

addaleax Mar 10, 2019

Choose a reason for hiding this comment

Uh oh!

joyeecheung Mar 13, 2019

Choose a reason for hiding this comment

Uh oh!

addaleax commented Mar 12, 2019

Uh oh!

addaleax commented Mar 13, 2019

Uh oh!

chjj commented Mar 13, 2019

Uh oh!

addaleax commented Mar 15, 2019

Uh oh!

addaleax commented Mar 15, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

worker: implement worker.moveMessagePortToContext() #26497
addaleax wants to merge 3 commits into
nodejs:masterfrom
addaleax:worker-messageport-move

joyeecheung Mar 9, 2019 •
edited
Loading