Skip to content

Commit

Permalink
store ContextState and ModuleMap in embedder slots (#772)
Browse files Browse the repository at this point in the history
Remove usage of rusty_v8 annex slots for storing `ContextState` and
`ModuleMap` and instead store them directly in a dedicated embedder
field.

Needed for denoland/deno#23976
  • Loading branch information
littledivy authored Jun 7, 2024
1 parent 21bc1c5 commit 98d52cf
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 11 deletions.
2 changes: 2 additions & 0 deletions core/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ pub use crate::runtime::JsRuntimeForSnapshot;
pub use crate::runtime::PollEventLoopOptions;
pub use crate::runtime::RuntimeOptions;
pub use crate::runtime::SharedArrayBufferStore;
pub use crate::runtime::CONTEXT_STATE_SLOT_INDEX;
pub use crate::runtime::MODULE_MAP_SLOT_INDEX;
pub use crate::runtime::V8_WRAPPER_OBJECT_INDEX;
pub use crate::runtime::V8_WRAPPER_TYPE_INDEX;
pub use crate::source_map::SourceMapData;
Expand Down
47 changes: 38 additions & 9 deletions core/runtime/jsrealm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ use std::rc::Rc;
use std::sync::Arc;
use v8::Handle;

pub const CONTEXT_STATE_SLOT_INDEX: i32 = 1;
pub const MODULE_MAP_SLOT_INDEX: i32 = 2;

// Hasher used for `unrefed_ops`. Since these are rolling i32, there's no
// need to actually hash them.
#[derive(Default)]
Expand Down Expand Up @@ -185,7 +188,26 @@ impl JsRealmInner {
std::mem::take(&mut *state.js_event_loop_tick_cb.borrow_mut());
std::mem::take(&mut *state.js_wasm_streaming_cb.borrow_mut());

self.context().open(isolate).clear_all_slots(isolate);
let ctx = self.context().open(isolate);
// SAFETY: Clear all embedder data
unsafe {
let ctx_state =
ctx.get_aligned_pointer_from_embedder_data(CONTEXT_STATE_SLOT_INDEX);
let _ = Box::from_raw(ctx_state as *mut Rc<ContextState>);

let module_map =
ctx.get_aligned_pointer_from_embedder_data(MODULE_MAP_SLOT_INDEX);
let _ = Box::from_raw(module_map as *mut Rc<ModuleMap>);
ctx.set_aligned_pointer_in_embedder_data(
CONTEXT_STATE_SLOT_INDEX,
std::ptr::null_mut(),
);
ctx.set_aligned_pointer_in_embedder_data(
MODULE_MAP_SLOT_INDEX,
std::ptr::null_mut(),
);
}
ctx.clear_all_slots(isolate);

// Expect that this context is dead (we only check this in debug mode)
// TODO(mmastrac): This check fails for some tests, will need to fix this
Expand All @@ -203,25 +225,32 @@ impl JsRealm {
scope: &mut v8::HandleScope,
) -> Rc<ContextState> {
let context = scope.get_current_context();
context.get_slot::<Rc<ContextState>>(scope).unwrap().clone()
// SAFETY: slot is valid and set during realm creation
unsafe {
let rc = context
.get_aligned_pointer_from_embedder_data(CONTEXT_STATE_SLOT_INDEX);
let rc = &*(rc as *const Rc<ContextState>);
rc.clone()
}
}

#[inline(always)]
pub(crate) fn module_map_from(scope: &mut v8::HandleScope) -> Rc<ModuleMap> {
let context = scope.get_current_context();
context.get_slot::<Rc<ModuleMap>>(scope).unwrap().clone()
// SAFETY: slot is valid and set during realm creation
unsafe {
let rc =
context.get_aligned_pointer_from_embedder_data(MODULE_MAP_SLOT_INDEX);
let rc = &*(rc as *const Rc<ModuleMap>);
rc.clone()
}
}

#[inline(always)]
pub(crate) fn exception_state_from_scope(
scope: &mut v8::HandleScope,
) -> Rc<ExceptionState> {
let context = scope.get_current_context();
context
.get_slot::<Rc<ContextState>>(scope)
.unwrap()
.exception_state
.clone()
Self::state_from_scope(scope).exception_state.clone()
}

#[cfg(test)]
Expand Down
20 changes: 18 additions & 2 deletions core/runtime/jsruntime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -869,7 +869,15 @@ impl JsRuntime {
);
}

context.set_slot(scope, context_state.clone());
// SAFETY: We need to initialize the slot. rusty_v8 currently segfaults
// when call `clear_all_slots`.
unsafe {
context.set_slot(scope, ());
context.set_aligned_pointer_in_embedder_data(
super::jsrealm::CONTEXT_STATE_SLOT_INDEX,
Box::into_raw(Box::new(context_state.clone())) as *mut c_void,
);
}

let inspector = if options.inspector {
Some(JsRuntimeInspector::new(scope, context, options.is_main))
Expand Down Expand Up @@ -910,7 +918,13 @@ impl JsRuntime {
}
}

context.set_slot(scope, module_map.clone());
// SAFETY: Set the module map slot in the context
unsafe {
context.set_aligned_pointer_in_embedder_data(
super::jsrealm::MODULE_MAP_SLOT_INDEX,
Box::into_raw(Box::new(module_map.clone())) as *mut c_void,
);
}

// ...we are ready to create a "realm" for the context...
let main_realm = {
Expand Down Expand Up @@ -1876,6 +1890,8 @@ fn create_context<'a>(
for middleware in global_template_middlewares {
global_object_template = middleware(scope, global_object_template);
}

global_object_template.set_internal_field_count(2);
let context = v8::Context::new_from_template(scope, global_object_template);
let scope = &mut v8::ContextScope::new(scope, context);

Expand Down
2 changes: 2 additions & 0 deletions core/runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ pub const V8_WRAPPER_OBJECT_INDEX: i32 = 1;
pub use jsrealm::ContextState;
pub(crate) use jsrealm::JsRealm;
pub(crate) use jsrealm::OpDriverImpl;
pub use jsrealm::CONTEXT_STATE_SLOT_INDEX;
pub use jsrealm::MODULE_MAP_SLOT_INDEX;
pub use jsruntime::CompiledWasmModuleStore;
pub use jsruntime::CreateRealmOptions;
pub use jsruntime::CrossIsolateStore;
Expand Down

0 comments on commit 98d52cf

Please sign in to comment.