-
Notifications
You must be signed in to change notification settings - Fork 5.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(core): Move optional callbacks from JsRuntimeState
to ContextState
#15599
refactor(core): Move optional callbacks from JsRuntimeState
to ContextState
#15599
Conversation
…textState` The `JsRuntimeState` struct stores a number of JS callbacks that are used either in the event loop or when interacting with V8. Some of these callback fields are vectors of callbacks, and therefore could plausibly store at least one callback per realm. However, some of those fields are `Option<v8::Global<v8::Function>>`, which would make the callbacks set by a realm override the one that might have been set by a different realm. As it turns out, all of the current such optional callbacks (`js_promise_reject_cb`, `js_format_exception_cb` and `js_wasm_streaming_cb`) are only used from inside a realm, and therefore this change makes it so such callbacks can only be set from inside a realm, and will only affect that realm. Towards denoland#13239.
Most of these callbacks aren't really tested in unit tests but in the |
Yes, I think that's fine for now |
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.
LGTM, thank you @andreubotella
… to `ContextState` (denoland#15599)" This reverts commit 307d84c.
…textState` The `JsRuntimeState` struct stores a number of JS callbacks that are used either in the event loop or when interacting with V8. Some of these callback fields are vectors of callbacks, and therefore could plausibly store at least one callback per realm. However, some of those fields are `Option<v8::Global<v8::Function>>`, which would make the callbacks set by a realm override the one that might have been set by a different realm. As it turns out, all of the current such optional callbacks (`js_promise_reject_cb`, `js_format_exception_cb` and `js_wasm_streaming_cb`) are only used from inside a realm, and therefore this change makes it so such callbacks can only be set from inside a realm, and will only affect that realm. This is a reland of denoland#15599. Towards denoland#13239.
…textState` (#17422) The `JsRuntimeState` struct stores a number of JS callbacks that are used either in the event loop or when interacting with V8. Some of these callback fields are vectors of callbacks, and therefore could plausibly store at least one callback per realm. However, some of those fields are `Option<v8::Global<v8::Function>>`, which would make the callbacks set by a realm override the one that might have been set by a different realm. As it turns out, all of the current such optional callbacks (`js_promise_reject_cb`, `js_format_exception_cb` and `js_wasm_streaming_cb`) are only used from inside a realm, and therefore this change makes it so such callbacks can only be set from inside a realm, and will only affect that realm. This is a reland of #15599. Towards #13239.
…textState` (#17422) The `JsRuntimeState` struct stores a number of JS callbacks that are used either in the event loop or when interacting with V8. Some of these callback fields are vectors of callbacks, and therefore could plausibly store at least one callback per realm. However, some of those fields are `Option<v8::Global<v8::Function>>`, which would make the callbacks set by a realm override the one that might have been set by a different realm. As it turns out, all of the current such optional callbacks (`js_promise_reject_cb`, `js_format_exception_cb` and `js_wasm_streaming_cb`) are only used from inside a realm, and therefore this change makes it so such callbacks can only be set from inside a realm, and will only affect that realm. This is a reland of #15599. Towards #13239.
The
JsRuntimeState
struct stores a number of JS callbacks that are used either in the event loop or when interacting with V8. Some of these callback fields are vectors of callbacks, and therefore could plausibly store at least one callback per realm. However, some of those fields areOption<v8::Global<v8::Function>>
, which would make the callbacks set by a realm override the one that might have been set by a different realm.As it turns out, all of the current such optional callbacks (
js_promise_reject_cb
,js_format_exception_cb
andjs_wasm_streaming_cb
) are only used from inside a realm, and therefore this change makes it so such callbacks can only be set from inside a realm, and will only affect that realm.Towards #13239.