src: define per-isolate internal bindings registration callback - #45547
Closed
legendecas wants to merge 2 commits into
Closed
src: define per-isolate internal bindings registration callback#45547legendecas wants to merge 2 commits into
legendecas wants to merge 2 commits into
Conversation
Copy link
Copy Markdown
Member
Sorry, something went wrong.
Copy link
Copy Markdown
Collaborator
|
Review requested:
|
Sorry, something went wrong.
legendecas
force-pushed
the
shadow-realm/per-realm-binding
branch
from
d115a82 to
c24d797
Compare
November 22, 2022 07:43
legendecas
force-pushed
the
shadow-realm/per-realm-binding
branch
2 times, most recently
from
06e2c24 to
bbecbcd
Compare
November 22, 2022 07:52
legendecas
marked this pull request as ready for review
November 22, 2022 17:26
legendecas
force-pushed
the
shadow-realm/per-realm-binding
branch
from
bbecbcd to
6311182
Compare
November 22, 2022 17:42
joyeecheung
reviewed
Nov 24, 2022
Comment thread
src/env-inl.h
Outdated
Comment thread
src/node_binding.cc
Outdated
Comment thread
src/node_builtins.cc
Outdated
legendecas
force-pushed
the
shadow-realm/per-realm-binding
branch
2 times, most recently
from
f762397 to
e07122b
Compare
November 29, 2022 18:01
joyeecheung
reviewed
Dec 2, 2022
Comment thread
src/node_binding.cc
Outdated
Comment thread
src/node_worker.cc
Outdated
Comment thread
src/node_builtins.cc
Outdated
legendecas
force-pushed
the
shadow-realm/per-realm-binding
branch
from
e07122b to
0dc40ad
Compare
December 6, 2022 14:52
joyeecheung
approved these changes
Dec 14, 2022
joyeecheung
left a comment
Copy link
Copy Markdown
Member
There was a problem hiding this comment.
LGTM with a nit
Sorry, something went wrong.
Comment thread
src/util.cc
Outdated
Copy link
Copy Markdown
Member
Author
|
@nodejs/realm PTAL, thank you! |
Sorry, something went wrong.
cjihrig
approved these changes
Dec 15, 2022
jasnell
approved these changes
Dec 19, 2022
Bindings that need to be loaded in distinct contexts of node::Realm in the same node::Environment instance needs to share the per-isolate templates. Add a new binding registration callback, which is called with per-IsolateData. This allows bindings to define templates and share them across realms eagerly, and avoid accidental modification on the templates when the per-context callback is called multiple times. This also tracks loaded bindings and built-in modules with node::Realm.
Create worker binding templates in the per-isolate initialization.
legendecas
force-pushed
the
shadow-realm/per-realm-binding
branch
from
0dc40ad to
f264321
Compare
December 19, 2022 18:02
Copy link
Copy Markdown
Collaborator
Sorry, something went wrong.
14 tasks
Copy link
Copy Markdown
Collaborator
Sorry, something went wrong.
This was referenced Dec 21, 2022
legendecas
added a commit
that referenced
this pull request
Dec 28, 2022
Bindings that need to be loaded in distinct contexts of node::Realm in the same node::Environment instance needs to share the per-isolate templates. Add a new binding registration callback, which is called with per-IsolateData. This allows bindings to define templates and share them across realms eagerly, and avoid accidental modification on the templates when the per-context callback is called multiple times. This also tracks loaded bindings and built-in modules with node::Realm. PR-URL: #45547 Refs: #42528 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
targos
pushed a commit
that referenced
this pull request
Jan 1, 2023
Bindings that need to be loaded in distinct contexts of node::Realm in the same node::Environment instance needs to share the per-isolate templates. Add a new binding registration callback, which is called with per-IsolateData. This allows bindings to define templates and share them across realms eagerly, and avoid accidental modification on the templates when the per-context callback is called multiple times. This also tracks loaded bindings and built-in modules with node::Realm. PR-URL: #45547 Refs: #42528 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Merged
RafaelGSS
pushed a commit
that referenced
this pull request
Jan 4, 2023
Bindings that need to be loaded in distinct contexts of node::Realm in the same node::Environment instance needs to share the per-isolate templates. Add a new binding registration callback, which is called with per-IsolateData. This allows bindings to define templates and share them across realms eagerly, and avoid accidental modification on the templates when the per-context callback is called multiple times. This also tracks loaded bindings and built-in modules with node::Realm. PR-URL: #45547 Refs: #42528 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
RafaelGSS
pushed a commit
that referenced
this pull request
Jan 5, 2023
Bindings that need to be loaded in distinct contexts of node::Realm in the same node::Environment instance needs to share the per-isolate templates. Add a new binding registration callback, which is called with per-IsolateData. This allows bindings to define templates and share them across realms eagerly, and avoid accidental modification on the templates when the per-context callback is called multiple times. This also tracks loaded bindings and built-in modules with node::Realm. PR-URL: #45547 Refs: #42528 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Copy link
Copy Markdown
Member
|
This is not landing cleanly in the v18.x line 💔 |
Sorry, something went wrong.
Copy link
Copy Markdown
Member
Author
|
Backport to v18.x depends on #46336. |
Sorry, something went wrong.
Copy link
Copy Markdown
Member
Author
|
Backport to v18.x depends on #44488. |
Sorry, something went wrong.
targos
pushed a commit
that referenced
this pull request
Nov 10, 2023
Bindings that need to be loaded in distinct contexts of node::Realm in the same node::Environment instance needs to share the per-isolate templates. Add a new binding registration callback, which is called with per-IsolateData. This allows bindings to define templates and share them across realms eagerly, and avoid accidental modification on the templates when the per-context callback is called multiple times. This also tracks loaded bindings and built-in modules with node::Realm. PR-URL: #45547 Refs: #42528 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
sercher
added a commit
to sercher/graaljs
that referenced
this pull request
Apr 25, 2024
Bindings that need to be loaded in distinct contexts of node::Realm in the same node::Environment instance needs to share the per-isolate templates. Add a new binding registration callback, which is called with per-IsolateData. This allows bindings to define templates and share them across realms eagerly, and avoid accidental modification on the templates when the per-context callback is called multiple times. This also tracks loaded bindings and built-in modules with node::Realm. PR-URL: nodejs/node#45547 Refs: nodejs/node#42528 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
sercher
added a commit
to sercher/graaljs
that referenced
this pull request
Apr 25, 2024
Bindings that need to be loaded in distinct contexts of node::Realm in the same node::Environment instance needs to share the per-isolate templates. Add a new binding registration callback, which is called with per-IsolateData. This allows bindings to define templates and share them across realms eagerly, and avoid accidental modification on the templates when the per-context callback is called multiple times. This also tracks loaded bindings and built-in modules with node::Realm. PR-URL: nodejs/node#45547 Refs: nodejs/node#42528 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reviewers
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.
Bindings that need to be loaded in distinct contexts of node::Realm in
the same node::Environment instance needs to share the per-isolate
templates.
Add a new binding registration callback, which is called with
per-IsolateData. This allows bindings to define templates and
share them across realms eagerly, and avoid accidental modification on
the templates when the per-context callback is called multiple times.
This also tracks loaded bindings and built-in modules with node::Realm.
Refs: #42528