foster/denoland-deno
1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-12-22 07:14:47 -05:00
Code Issues Packages 1 Wiki Activity

fix(core): Have custom errors be created in the right realm (#17050)

Browse source 
Although PR #16366 did not fully revert `deno_core`'s support for
realms, since `JsRealm` still existed after that, it did remove the
`ContextState` struct, originally introduced in #14734. This change made
`js_build_custom_error_cb`, among other properties, a per-runtime
callback, rather than per-realm, which cause a bug where errors thrown
from an op would always be constructed in the main realm, rather than in
the current one.

This change adds back `ContextState` to fix this bug, adds back the
`known_realms` field of `JsRuntimeState` (needed to be able to drop the
callback when snapshotting), and also relands #14750, which adds the
`js_realm_sync_ops` test for this bug that was removed in #16366.
This commit is contained in:
Andreu Botella 2022-12-20 11:09:16 -08:00 committed by GitHub
parent 748ce0a435
commit fcca54d3ba
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 134 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
core/error.rs
View file

@ -1,6 +1,7 @@
// Copyright 2018-2022 the Deno authors. All rights reserved. MIT license.
use crate::runtime::GetErrorClassFn;
use crate::runtime::JsRealm;
use crate::runtime::JsRuntime;
use crate::source_map::apply_source_map;
use crate::source_map::get_source_line;
@ -98,7 +99,7 @@ pub fn to_v8_error<'a>(
error: &Error,
) -> v8::Local<'a, v8::Value> {
let tc_scope = &mut v8::TryCatch::new(scope);
let cb = JsRuntime::state(tc_scope)
let cb = JsRealm::state_from_scope(tc_scope)
.borrow()
.js_build_custom_error_cb
.clone()

148
core/runtime.rs
View file

@ -148,16 +148,21 @@ pub type SharedArrayBufferStore =
pub type CompiledWasmModuleStore = CrossIsolateStore<v8::CompiledWasmModule>;
#[derive(Default)]
pub(crate) struct ContextState {
pub(crate) js_build_custom_error_cb: Option<v8::Global<v8::Function>>,
}
/// Internal state for JsRuntime which is stored in one of v8::Isolate's
/// embedder slots.
pub struct JsRuntimeState {
global_realm: Option<JsRealm>,
known_realms: Vec<v8::Weak<v8::Context>>,
pub(crate) js_recv_cb: Option<v8::Global<v8::Function>>,
pub(crate) js_macrotask_cbs: Vec<v8::Global<v8::Function>>,
pub(crate) js_nexttick_cbs: Vec<v8::Global<v8::Function>>,
pub(crate) js_promise_reject_cb: Option<v8::Global<v8::Function>>,
pub(crate) js_format_exception_cb: Option<v8::Global<v8::Function>>,
pub(crate) js_build_custom_error_cb: Option<v8::Global<v8::Function>>,
pub(crate) has_tick_scheduled: bool,
pub(crate) js_wasm_streaming_cb: Option<v8::Global<v8::Function>>,
pub(crate) pending_promise_exceptions:
@ -388,7 +393,6 @@ impl JsRuntime {
js_nexttick_cbs: vec![],
js_promise_reject_cb: None,
js_format_exception_cb: None,
js_build_custom_error_cb: None,
has_tick_scheduled: false,
js_wasm_streaming_cb: None,
source_map_getter: options.source_map_getter,
@ -405,6 +409,7 @@ impl JsRuntime {
inspector: None,
op_ctxs: vec![].into_boxed_slice(),
global_realm: None,
known_realms: Vec::with_capacity(1),
}));
let weak = Rc::downgrade(&state_rc);
@ -517,6 +522,10 @@ impl JsRuntime {
(isolate, snapshot_options)
};
global_context
.open(&mut isolate)
.set_slot(&mut isolate, Rc::<RefCell<ContextState>>::default());
op_state.borrow_mut().put(isolate_ptr);
let inspector = if options.inspector {
Some(JsRuntimeInspector::new(
@ -533,9 +542,12 @@ impl JsRuntime {
.unwrap_or_else(|| Rc::new(NoopModuleLoader));
{
let mut state = state_rc.borrow_mut();
state.global_realm = Some(JsRealm(global_context));
state.global_realm = Some(JsRealm(global_context.clone()));
state.op_ctxs = op_ctxs;
state.inspector = inspector;
state
.known_realms
.push(v8::Weak::new(&mut isolate, global_context));
}
isolate.set_data(
Self::STATE_DATA_OFFSET,
@ -632,10 +644,19 @@ impl JsRuntime {
&self.state.borrow().op_ctxs,
self.snapshot_options,
);
context.set_slot(scope, Rc::<RefCell<ContextState>>::default());
self
.state
.borrow_mut()
.known_realms
.push(v8::Weak::new(scope, context));
JsRealm::new(v8::Global::new(scope, context))
};
self.init_extension_js(&realm)?;
self.init_realm_cbs(&realm);
Ok(realm)
}
@ -797,19 +818,36 @@ impl JsRuntime {
/// Grabs a reference to core.js' opresolve & syncOpsCache()
fn init_cbs(&mut self) {
let scope = &mut self.handle_scope();
let recv_cb =
Self::eval::<v8::Function>(scope, "Deno.core.opresolve").unwrap();
let recv_cb = v8::Global::new(scope, recv_cb);
let build_custom_error_cb =
Self::eval::<v8::Function>(scope, "Deno.core.buildCustomError")
.expect("Deno.core.buildCustomError is undefined in the realm");
let build_custom_error_cb = v8::Global::new(scope, build_custom_error_cb);
// Put global handles in state
let state_rc = JsRuntime::state(scope);
let mut state = state_rc.borrow_mut();
state.js_recv_cb.replace(recv_cb);
{
let scope = &mut self.handle_scope();
let recv_cb =
Self::eval::<v8::Function>(scope, "Deno.core.opresolve").unwrap();
let recv_cb = v8::Global::new(scope, recv_cb);
// Put global handle in state
let state_rc = JsRuntime::state(scope);
let mut state = state_rc.borrow_mut();
state.js_recv_cb.replace(recv_cb);
}
// Also run init_realm_cbs for the main realm.
// TODO(@andreubotella): Merge this method back with `init_realm_cbs` when
// `js_recv_cb` is moved to ContextState.
let global_realm = self.global_realm();
self.init_realm_cbs(&global_realm);
}
fn init_realm_cbs(&mut self, realm: &JsRealm) {
let build_custom_error_cb = {
let scope = &mut realm.handle_scope(self.v8_isolate());
let build_custom_error_cb =
Self::eval::<v8::Function>(scope, "Deno.core.buildCustomError")
.expect("Deno.core.buildCustomError is undefined in the realm");
v8::Global::new(scope, build_custom_error_cb)
};
// Put global handle in the realm's ContextState
let state = realm.state(self.v8_isolate());
state
.borrow_mut()
.js_build_custom_error_cb
.replace(build_custom_error_cb);
}
@ -873,14 +911,26 @@ impl JsRuntime {
}
// Drop other v8::Global handles before snapshotting
{
for weak_context in &self.state.clone().borrow().known_realms {
let v8_isolate = self.v8_isolate();
if let Some(context) = weak_context.to_global(v8_isolate) {
let realm = JsRealm::new(context.clone());
let realm_state = realm.state(v8_isolate);
std::mem::take(
&mut realm_state.borrow_mut().js_build_custom_error_cb,
);
context.open(v8_isolate).clear_all_slots(v8_isolate);
}
}
let mut state = self.state.borrow_mut();
std::mem::take(&mut state.js_recv_cb);
std::mem::take(&mut state.js_promise_reject_cb);
std::mem::take(&mut state.js_format_exception_cb);
std::mem::take(&mut state.js_wasm_streaming_cb);
std::mem::take(&mut state.js_build_custom_error_cb);
state.js_macrotask_cbs.clear();
state.js_nexttick_cbs.clear();
state.known_realms.clear();
}
let snapshot_creator = self.v8_isolate.take().unwrap();
@ -2246,6 +2296,25 @@ impl JsRealm {
&self.0
}
fn state(&self, isolate: &mut v8::Isolate) -> Rc<RefCell<ContextState>> {
self
.context()
.open(isolate)
.get_slot::<Rc<RefCell<ContextState>>>(isolate)
.unwrap()
.clone()
}
pub(crate) fn state_from_scope(
scope: &mut v8::HandleScope,
) -> Rc<RefCell<ContextState>> {
let context = scope.get_current_context();
context
.get_slot::<Rc<RefCell<ContextState>>>(scope)
.unwrap()
.clone()
}
pub fn handle_scope<'s>(
&self,
isolate: &'s mut v8::Isolate,
@ -4187,6 +4256,53 @@ Deno.core.ops.op_async_serialize_object_with_numbers_as_keys({
assert_eq!(ret, serde_v8::to_v8(scope, "Test").unwrap());
}
#[test]
fn js_realm_sync_ops() {
// Test that returning a ZeroCopyBuf and throwing an exception from a sync
// op result in objects with prototypes from the right realm. Note that we
// don't test the result of returning structs, because they will be
// serialized to objects with null prototype.
#[op]
fn op_test(fail: bool) -> Result<ZeroCopyBuf, Error> {
if !fail {
Ok(ZeroCopyBuf::empty())
} else {
Err(crate::error::type_error("Test"))
}
}
let mut runtime = JsRuntime::new(RuntimeOptions {
extensions: vec![Extension::builder().ops(vec![op_test::decl()]).build()],
get_error_class_fn: Some(&|error| {
crate::error::get_custom_error_class(error).unwrap()
}),
..Default::default()
});
let new_realm = runtime.create_realm().unwrap();
// Test in both realms
for realm in [runtime.global_realm(), new_realm].into_iter() {
let ret = realm
.execute_script(
runtime.v8_isolate(),
"",
r#"
const buf = Deno.core.ops.op_test(false);
try {
Deno.core.ops.op_test(true);
} catch(e) {
err = e;
}
buf instanceof Uint8Array && buf.byteLength === 0 &&
err instanceof TypeError && err.message === "Test"
"#,
)
.unwrap();
assert!(ret.open(runtime.v8_isolate()).is_true());
}
}
#[test]
fn test_array_by_copy() {
// Verify that "array by copy" proposal is enabled (https://github.com/tc39/proposal-change-array-by-copy)