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

legendecas · 2023-02-24T08:23:23Z

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

nodejs-github-bot · 2023-02-24T08:23:28Z

Review requested:

@nodejs/startup
@nodejs/realm

mcollina

lgtm

joyeecheung · 2023-02-24T21:40:12Z

src/api/environment.cc

+  // TODO(legendecas): Per-realm prepareStackTrace callback.
+  // If we are in a Realm that is not the principal Realm (e.g. ShadowRealm),
+  // skip the prepareStackTrace callback as the context's security token is
+  // likely to be different.


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.

test/parallel/test-shadow-realm-globals.js

test/common/globals.js

nodejs-github-bot · 2023-02-28T07:06:52Z

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

nodejs-github-bot · 2023-03-01T07:58:41Z

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

nodejs-github-bot · 2023-03-01T13:35:35Z

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

src/node_realm.cc

src/node_realm-inl.h

nodejs-github-bot · 2023-03-02T19:32:31Z

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

mcollina · 2023-03-06T14:06:18Z

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

test/parallel/test-shadow-realm-globals.js

nodejs-github-bot · 2023-03-13T03:38:39Z

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

nodejs-github-bot · 2023-03-13T06:05:12Z

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

nodejs-github-bot · 2023-03-14T09:57:23Z

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

legendecas · 2023-03-15T16:31:52Z

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>

RafaelGSS · 2023-04-07T13:43:50Z

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

test/parallel/test-shadow-realm-globals.js Outdated Show resolved Hide resolved

test/common/globals.js Outdated Show resolved Hide resolved

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

src/node_realm.cc Outdated Show resolved Hide resolved

src/node_realm-inl.h Outdated Show resolved Hide resolved

src/node_realm-inl.h Outdated Show resolved Hide resolved

joyeecheung reviewed Mar 2, 2023

View reviewed changes

src/node_realm-inl.h Outdated Show resolved Hide resolved

src/node_realm-inl.h Outdated Show resolved Hide resolved

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

test/parallel/test-shadow-realm-globals.js Show resolved Hide resolved

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

Open

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 PRs awaiting manual backport to the v18.x-staging branch. label Jun 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

legendecas commented Feb 24, 2023 •

edited

Loading

nodejs-github-bot commented Feb 24, 2023 •

edited by legendecas

Loading

mcollina left a comment

joyeecheung Feb 24, 2023

legendecas Feb 28, 2023

nodejs-github-bot commented Feb 28, 2023

nodejs-github-bot commented Mar 1, 2023

nodejs-github-bot commented Mar 1, 2023

nodejs-github-bot commented Mar 2, 2023

mcollina commented Mar 6, 2023

nodejs-github-bot commented Mar 13, 2023

nodejs-github-bot commented Mar 13, 2023

nodejs-github-bot commented Mar 14, 2023

legendecas commented Mar 15, 2023

RafaelGSS commented Apr 7, 2023

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

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

Conversation

legendecas commented Feb 24, 2023 • edited Loading

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

mcollina left a comment

Choose a reason for hiding this comment

joyeecheung Feb 24, 2023

Choose a reason for hiding this comment

legendecas Feb 28, 2023

Choose a reason for hiding this comment

nodejs-github-bot commented Feb 28, 2023

nodejs-github-bot commented Mar 1, 2023

nodejs-github-bot commented Mar 1, 2023

nodejs-github-bot commented Mar 2, 2023

mcollina commented Mar 6, 2023

nodejs-github-bot commented Mar 13, 2023

nodejs-github-bot commented Mar 13, 2023

nodejs-github-bot commented Mar 14, 2023

legendecas commented Mar 15, 2023

RafaelGSS commented Apr 7, 2023

legendecas commented Feb 24, 2023 •

edited

Loading

nodejs-github-bot commented Feb 24, 2023 •

edited by legendecas

Loading