src: bootstrap Web [Exposed=*] APIs in the shadow realm by legendecas · Pull Request #46809 · nodejs/node · GitHub

This is the initial work to bootstrap Web interfaces that are defined
with extended attributes [Exposed=*].

The ShadowRealm instances are garbage-collected once it is
unreachable. However, V8 can not infer the reference cycles between
the per-realm strong persistent function handles and the realm's
context handle. To allow the context to be gc-ed once it is not
reachable, the per-realm persistent handles are attached to the
context's global object and the persistent handles are set as weak.

Refs: #42528

Review requested:

@nodejs/startup
@nodejs/realm

lgtm

Actually, why do we need different security tokens? Can we just set the non-principal realm's security token to be the same as the one of the principal realm, like what we do for vm contexts? IIUC security tokens in browsers are generally intended for cross-origin global proxies, which isn't really a thing for Node.js. For shadow realms, the cross-realm access is guarded with wrapped functions and callable boundaries. For Node.js realms, do we care about cross-realm object access?

It is true that object exchange across the ShadowRealm boundary is limited with wrapped functions. However, it is still possible for host hooks like the prepareStackTrace callback here to leak objects to the principal realm's userland Error.prepareStackTrace override.

It doesn't hurt to set the security token to be the principal realm's. But I don't see the reason to allow the access either.

I've updated the comment to point out that this branch is intended to avoid calling the principal realm's Error.prepareStackTrace override instead of the security token mismatches.

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

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

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

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

CI is has a relevant test failure:

node:assert:125
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
+ actual - expected

+ [
+   'BroadcastChannel'
+ ]
- []
    at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux/test/parallel/test-shadow-realm-globals.js:27:8)
    at Module._compile (node:internal/modules/cjs/loader:1287:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1341:10)
    at Module.load (node:internal/modules/cjs/loader:1145:32)
    at Module._load (node:internal/modules/cjs/loader:984:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:83:12)
    at node:internal/main/run_main_module:23:47 {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: [ 'BroadcastChannel' ],
  expected: [],
  operator: 'deepStrictEqual'
}

Node.js v20.0.0-pre

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

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

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

Landed in d0153ae...e6b4d30

PR-URL: #46809 Refs: #42528 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

This is the initial work to bootstrap Web interfaces that are defined with extended attributes `[Exposed=*]`. The ShadowRealm instances are garbage-collected once it is unreachable. However, V8 can not infer the reference cycles between the per-realm strong persistent function handles and the realm's context handle. To allow the context to be gc-ed once it is not reachable, the per-realm persistent handles are attached to the context's global object and the persistent handles are set as weak. PR-URL: #46809 Refs: #42528 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

PR-URL: #46809 Refs: #42528 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

This is the initial work to bootstrap Web interfaces that are defined with extended attributes `[Exposed=*]`. The ShadowRealm instances are garbage-collected once it is unreachable. However, V8 can not infer the reference cycles between the per-realm strong persistent function handles and the realm's context handle. To allow the context to be gc-ed once it is not reachable, the per-realm persistent handles are attached to the context's global object and the persistent handles are set as weak. PR-URL: #46809 Refs: #42528 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

This is breaking v19.x-staging. Could you please create a manual backport? See: #47441 (comment)

PR-URL: nodejs#46809 Refs: nodejs#42528 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

This is the initial work to bootstrap Web interfaces that are defined with extended attributes `[Exposed=*]`. The ShadowRealm instances are garbage-collected once it is unreachable. However, V8 can not infer the reference cycles between the per-realm strong persistent function handles and the realm's context handle. To allow the context to be gc-ed once it is not reachable, the per-realm persistent handles are attached to the context's global object and the persistent handles are set as weak. PR-URL: nodejs#46809 Refs: nodejs#42528 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Feb 24, 2023

legendecas added the realm Issues and PRs related to the ShadowRealm API and node::Realm label Feb 24, 2023

legendecas force-pushed the shadowrealm/builtin branch from 9801507 to b5bf1ac Compare February 24, 2023 08:26

mcollina approved these changes Feb 24, 2023

View reviewed changes

joyeecheung reviewed Feb 24, 2023

View reviewed changes

jasnell approved these changes Feb 24, 2023

View reviewed changes

cjihrig approved these changes Feb 26, 2023

View reviewed changes

Comment thread test/parallel/test-shadow-realm-globals.js Outdated

Comment thread test/common/globals.js Outdated

legendecas force-pushed the shadowrealm/builtin branch from b5bf1ac to dee17ae Compare February 28, 2023 06:49

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 28, 2023

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

github-actions Bot mentioned this pull request Mar 1, 2023

CI Reliability 2023-03-01 nodejs/reliability#510

Open

15 tasks

github-actions Bot mentioned this pull request Mar 2, 2023

CI Reliability 2023-03-02 nodejs/reliability#511

Open

15 tasks

joyeecheung reviewed Mar 2, 2023

View reviewed changes

Comment thread src/node_realm.cc Outdated

Comment thread src/node_realm-inl.h Outdated

Comment thread src/node_realm-inl.h Outdated

joyeecheung reviewed Mar 2, 2023

View reviewed changes

Comment thread src/node_realm-inl.h Outdated

Comment thread src/node_realm-inl.h Outdated

This was referenced Mar 3, 2023

CI Reliability 2023-03-03 nodejs/reliability#512

Open

CI Reliability 2023-03-04 nodejs/reliability#513

Open

CI Reliability 2023-03-05 nodejs/reliability#514

Open

CI Reliability 2023-03-06 nodejs/reliability#515

Open

cjihrig reviewed Mar 6, 2023

View reviewed changes

Comment thread test/parallel/test-shadow-realm-globals.js Outdated

github-actions Bot mentioned this pull request Mar 7, 2023

CI Reliability 2023-03-07 nodejs/reliability#516

Open

19 tasks

legendecas force-pushed the shadowrealm/builtin branch from dee17ae to b204e2f Compare March 7, 2023 18:27

legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 8, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 8, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 13, 2023

github-actions Bot mentioned this pull request Mar 14, 2023

CI Reliability 2023-03-14 nodejs/reliability#523

Open

24 tasks

github-actions Bot mentioned this pull request Mar 15, 2023

CI Reliability 2023-03-15 nodejs/reliability#524

Open

22 tasks

legendecas closed this Mar 15, 2023

legendecas deleted the shadowrealm/builtin branch March 15, 2023 16:32

This was referenced Mar 16, 2023

CI Reliability 2023-03-16 nodejs/reliability#525

Open

CI Reliability 2023-03-17 nodejs/reliability#526

Open

CI Reliability 2023-03-18 nodejs/reliability#527

Open

anonrig mentioned this pull request Mar 31, 2023

url: use ada::url_aggregator for parsing urls #47339

Merged

joyeecheung mentioned this pull request Apr 1, 2023

memory leak in shadow realms #47353

Closed

This was referenced Apr 3, 2023

2023-04-18, Version 20.0.0 (Current) #47381

Merged

v19.9.0 proposal #47441

Merged

richardlau mentioned this pull request Apr 7, 2023

[v19.x] deps: V8: cherry-pick f09dde9fbb54 #47239

Closed

RafaelGSS added the backport-requested-v19.x label Apr 7, 2023

This was referenced Apr 7, 2023

test: move test-shadow-realm-gc.js to known_issues #47355

Merged

src: bootstrap prepare stack trace callback in shadow realm #47107

Merged

bootstrap: store internal loaders in C++ via a binding #47215

Closed

legendecas mentioned this pull request Apr 12, 2023

[v19.x] src: bootstrap Web [Exposed=*] APIs in the shadow realm #47530

Closed

danielleadams added the backport-requested-v18.x label Jun 11, 2023

src: bootstrap Web [Exposed=*] APIs in the shadow realm by legendecas · Pull Request #46809 · nodejs/node · GitHub

Uh oh!

Uh oh!

src: bootstrap Web [Exposed=*] APIs in the shadow realm - #46809

src: bootstrap Web [Exposed=*] APIs in the shadow realm#46809legendecas wants to merge 3 commits intonodejs:mainnodejs/node:mainfrom legendecas:shadowrealm/builtinlegendecas/node:shadowrealm/builtinCopy head branch name to clipboard

Conversation

legendecas commented Feb 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Feb 24, 2023 • edited by legendecas Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

joyeecheung Feb 24, 2023

Choose a reason for hiding this comment

Uh oh!

legendecas Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Feb 28, 2023

Uh oh!

nodejs-github-bot commented Mar 1, 2023

Uh oh!

nodejs-github-bot commented Mar 1, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Mar 2, 2023

Uh oh!

mcollina commented Mar 6, 2023

Uh oh!

Uh oh!

nodejs-github-bot commented Mar 13, 2023

Uh oh!

nodejs-github-bot commented Mar 13, 2023

Uh oh!

nodejs-github-bot commented Mar 14, 2023

Uh oh!

legendecas commented Mar 15, 2023

Uh oh!

RafaelGSS commented Apr 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

src: bootstrap Web [Exposed=*] APIs in the shadow realm#46809
legendecas wants to merge 3 commits into
nodejs:mainfrom
legendecas:shadowrealm/builtin

legendecas commented Feb 24, 2023 •
edited
Loading

nodejs-github-bot commented Feb 24, 2023 •
edited by legendecas
Loading