Skip to content

Commit

Permalink
Allow cloning Rc<ContextState> into new contexts (denoland#693)
Browse files Browse the repository at this point in the history
Required for  denoland#23305

Allowing cloning `Rc<ContextState>` into new contexts. 

During cleanup we assert that main context does not have more than one
strong count to `JsRuntimeState` but this may not be true when there are
more than one contexts that may not have been GC'ed yet. Hence,
JsRuntimeState is now stored as a pointer to OpCtx as it is guarnateed
to live longer than ops.
  • Loading branch information
littledivy authored Apr 11, 2024
1 parent 3a6b2ed commit bd2c8b8
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 6 deletions.
3 changes: 2 additions & 1 deletion core/extension_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ pub fn create_op_ctxs(
let op_count = op_decls.len();
let mut op_ctxs = Vec::with_capacity(op_count);

let runtime_state_ptr = runtime_state.as_ref() as *const _;
for (index, decl) in op_decls.into_iter().enumerate() {
let metrics_fn = op_metrics_factory_fn
.as_ref()
Expand All @@ -140,7 +141,7 @@ pub fn create_op_ctxs(
op_driver.clone(),
decl,
op_state.clone(),
runtime_state.clone(),
runtime_state_ptr,
get_error_class_fn,
metrics_fn,
);
Expand Down
1 change: 1 addition & 0 deletions core/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ pub use crate::ops_metrics::OpMetricsSummaryTracker;
pub use crate::path::strip_unc_prefix;
pub use crate::runtime::stats;
pub use crate::runtime::CompiledWasmModuleStore;
pub use crate::runtime::ContextState;
pub use crate::runtime::CreateRealmOptions;
pub use crate::runtime::CrossIsolateStore;
pub use crate::runtime::JsRuntime;
Expand Down
7 changes: 4 additions & 3 deletions core/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ pub struct OpCtx {
pub(crate) last_fast_error: UnsafeCell<Option<AnyError>>,

op_driver: Rc<OpDriverImpl>,
runtime_state: Rc<JsRuntimeState>,
runtime_state: *const JsRuntimeState,
}

impl OpCtx {
Expand All @@ -107,7 +107,7 @@ impl OpCtx {
op_driver: Rc<OpDriverImpl>,
decl: OpDecl,
state: Rc<RefCell<OpState>>,
runtime_state: Rc<JsRuntimeState>,
runtime_state: *const JsRuntimeState,
get_error_class_fn: GetErrorClassFn,
metrics_fn: Option<OpMetricsFn>,
) -> Self {
Expand Down Expand Up @@ -245,7 +245,8 @@ impl OpCtx {

/// Get the [`JsRuntimeState`] for this op.
pub(crate) fn runtime_state(&self) -> &JsRuntimeState {
&self.runtime_state
// SAFETY: JsRuntimeState outlives OpCtx
unsafe { &*self.runtime_state }
}
}

Expand Down
2 changes: 1 addition & 1 deletion core/runtime/jsrealm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl Hasher for IdentityHasher {
/// We may wish to experiment with alternative drivers in the future.
pub(crate) type OpDriverImpl = super::op_driver::FuturesUnorderedDriver;

pub(crate) struct ContextState {
pub struct ContextState {
pub(crate) task_spawner_factory: Arc<V8TaskSpawnerFactory>,
pub(crate) timers: WebTimers<(v8::Global<v8::Function>, u32)>,
pub(crate) js_event_loop_tick_cb:
Expand Down
2 changes: 1 addition & 1 deletion core/runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ mod tests;
pub const V8_WRAPPER_TYPE_INDEX: i32 = 0;
pub const V8_WRAPPER_OBJECT_INDEX: i32 = 1;

pub(crate) use jsrealm::ContextState;
pub use jsrealm::ContextState;
pub(crate) use jsrealm::JsRealm;
pub(crate) use jsrealm::OpDriverImpl;
pub use jsruntime::CompiledWasmModuleStore;
Expand Down

0 comments on commit bd2c8b8

Please sign in to comment.