From 71645e553cef5e19098a1f807838649755fa65de Mon Sep 17 00:00:00 2001 From: ldm0 Date: Wed, 7 Oct 2026 02:43:27 +0800 Subject: [PATCH] refactor(v8): let contexts own Web API intrinsic identities Store constructors, prototypes and public interface objects in a Context-owned private array. Keep only publication metadata in Rust and remove the navigation-time handle conversion. Verify identity across detachment and collection while Rust metadata remains alive. Validation: cargo fmt --all; cargo clippy --workspace --all-targets --all-features -- -D warnings; cargo nextest run --no-fail-fast (19,651 passed, 16 skipped). --- .../exposed_interfaces/install.rs | 2 +- .../exposed_interfaces/materialize.rs | 2 +- .../exposed_interfaces/mod.rs | 2 - .../exposed_interfaces/realm_registry.rs | 221 +++++++++++++----- .../exposed_interfaces/tests.rs | 2 +- .../exposed_interfaces/tests/finalization.rs | 4 +- .../src/native_bridge/identity.rs | 1 - 7 files changed, 166 insertions(+), 68 deletions(-) diff --git a/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/install.rs b/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/install.rs index 230d3c8d57..1c57fe488b 100644 --- a/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/install.rs +++ b/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/install.rs @@ -217,7 +217,7 @@ pub(crate) fn capture_eager_intrinsic_interfaces<'s>( .entry(metadata.id) .ok_or_else(|| anyhow!("interface id is out of range"))? { - RealmInterfaceEntry::Ready(_) => continue, + RealmInterfaceEntry::Ready => continue, RealmInterfaceEntry::Failed | RealmInterfaceEntry::Materializing => { return Err(anyhow!( "eager intrinsic `{}` is not ready for capture", diff --git a/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/materialize.rs b/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/materialize.rs index 13a2daa39b..160d58dc84 100644 --- a/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/materialize.rs +++ b/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/materialize.rs @@ -79,7 +79,7 @@ pub(super) fn materialize_interface<'s>( .entry(id) .ok_or_else(|| anyhow!("interface state is out of range"))? { - RealmInterfaceEntry::Ready(_) => { + RealmInterfaceEntry::Ready => { return realm .public_interface(scope, id) .map(Into::into) diff --git a/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/mod.rs b/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/mod.rs index fdeda3af90..888e685109 100644 --- a/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/mod.rs +++ b/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/mod.rs @@ -30,5 +30,3 @@ pub(super) use metadata::constructor_spec_is_eager; #[cfg(test)] pub(crate) use metadata::dedicated_worker_lazy_interface_names_for_test; pub(in crate::context_bootstrap) use template_registry::ExposedInterfaceTemplateRegistry; - -pub(crate) use realm_registry::retain_intrinsic_interfaces_in_realm; diff --git a/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/realm_registry.rs b/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/realm_registry.rs index c735cf8d09..4338291e7d 100644 --- a/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/realm_registry.rs +++ b/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/realm_registry.rs @@ -5,12 +5,12 @@ use anyhow::{Result, anyhow}; use super::metadata::{InterfaceId, RealmKind}; -/// Only a completely initialized interface retains intrinsic handles and is -/// visible to callers. State and identities share one realm-owned entry. +/// Publication state contains no V8 roots. The Context owns intrinsic identities +/// through a private array on its extras object, including after Window detachment. pub(super) enum RealmInterfaceEntry { Uninitialized, Materializing, - Ready(RealmInterfaceObjects), + Ready, Failed, } @@ -19,10 +19,13 @@ pub(super) struct IntrinsicInterfaceRegistry { entries: RefCell>, } -pub(super) struct RealmInterfaceObjects { - constructor: crate::util::RealmObjectHandle, - prototype: crate::util::RealmObjectHandle, - public_interface: crate::util::RealmObjectHandle, +const INTRINSIC_OBJECTS_SLOT: &str = "__moliWebApiIntrinsicObjects"; +const OBJECTS_PER_INTERFACE: usize = 3; + +fn intrinsic_objects<'s>(scope: &mut v8::PinScope<'s, '_>) -> Option> { + let anchor = scope.get_current_context().get_extras_binding_object(scope); + crate::util::get_private_value(scope, anchor, INTRINSIC_OBJECTS_SLOT) + .and_then(|value| v8::Local::try_from(value).ok()) } impl IntrinsicInterfaceRegistry { @@ -135,44 +138,89 @@ impl IntrinsicInterfaceRegistry { prototype: v8::Local<'s, v8::Object>, public_interface: v8::Local<'s, v8::Object>, ) -> Result<()> { - let mut entries = self.entries.borrow_mut(); - let interface_count = entries.len(); - let slot = entries.get_mut(id.index()).ok_or_else(|| { - anyhow!( - "interface id {} is out of range for {interface_count} entries", - id.index() - ) - })?; - match slot { - RealmInterfaceEntry::Ready(_) => { - return Err(anyhow!("interface id {} is already published", id.index())); + { + let entries = self.entries.borrow(); + let slot = entries.get(id.index()).ok_or_else(|| { + anyhow!( + "interface id {} is out of range for {} entries", + id.index(), + entries.len() + ) + })?; + match slot { + RealmInterfaceEntry::Ready => { + return Err(anyhow!("interface id {} is already published", id.index())); + } + RealmInterfaceEntry::Failed => { + return Err(anyhow!( + "a previous materialization of interface id {} failed", + id.index() + )); + } + RealmInterfaceEntry::Uninitialized | RealmInterfaceEntry::Materializing => {} } - RealmInterfaceEntry::Failed => { - return Err(anyhow!( - "a previous materialization of interface id {} failed", - id.index() - )); - } - RealmInterfaceEntry::Uninitialized | RealmInterfaceEntry::Materializing => {} } - *slot = RealmInterfaceEntry::Ready(RealmInterfaceObjects { - constructor: crate::util::RealmObjectHandle::new(scope, constructor), - prototype: crate::util::RealmObjectHandle::new(scope, prototype), - public_interface: crate::util::RealmObjectHandle::new(scope, public_interface), - }); + let objects = if let Some(objects) = intrinsic_objects(scope) { + objects + } else { + let count = self + .entries + .borrow() + .len() + .checked_mul(OBJECTS_PER_INTERFACE) + .and_then(|count| i32::try_from(count).ok()) + .ok_or_else(|| anyhow!("too many realm interface objects"))?; + let objects = v8::Array::new(scope, count); + if objects.set_prototype(scope, v8::null(scope).into()) != Some(true) { + return Err(anyhow!("failed to initialize realm interface storage")); + } + let anchor = scope.get_current_context().get_extras_binding_object(scope); + let key = crate::util::private_key(scope, INTRINSIC_OBJECTS_SLOT) + .ok_or_else(|| anyhow!("failed to create realm interface storage key"))?; + if anchor.set_private(scope, key, objects.into()) != Some(true) { + return Err(anyhow!("failed to publish realm interface storage")); + } + objects + }; + let first = u32::try_from(id.index() * OBJECTS_PER_INTERFACE) + .map_err(|_| anyhow!("realm interface object index is out of range"))?; + for (offset, value) in [constructor, prototype, public_interface] + .into_iter() + .enumerate() + { + if objects.set_index(scope, first + offset as u32, value.into()) != Some(true) { + return Err(anyhow!("failed to publish realm interface object")); + } + } + self.entries.borrow_mut()[id.index()] = RealmInterfaceEntry::Ready; Ok(()) } + fn object<'s>( + &self, + scope: &mut v8::PinScope<'s, '_>, + id: InterfaceId, + offset: usize, + ) -> Option> { + if !matches!( + self.entries.borrow().get(id.index())?, + RealmInterfaceEntry::Ready + ) { + return None; + } + let objects = intrinsic_objects(scope)?; + let index = u32::try_from(id.index() * OBJECTS_PER_INTERFACE + offset).ok()?; + objects + .get_index(scope, index) + .and_then(|value| v8::Local::try_from(value).ok()) + } + pub(super) fn constructor<'s>( &self, scope: &mut v8::PinScope<'s, '_>, id: InterfaceId, ) -> Option> { - let entries = self.entries.borrow(); - let RealmInterfaceEntry::Ready(objects) = entries.get(id.index())? else { - return None; - }; - objects.constructor.to_local(scope) + self.object(scope, id, 0) } pub(super) fn prototype<'s>( @@ -180,11 +228,7 @@ impl IntrinsicInterfaceRegistry { scope: &mut v8::PinScope<'s, '_>, id: InterfaceId, ) -> Option> { - let entries = self.entries.borrow(); - let RealmInterfaceEntry::Ready(objects) = entries.get(id.index())? else { - return None; - }; - objects.prototype.to_local(scope) + self.object(scope, id, 1) } pub(super) fn public_interface<'s>( @@ -192,35 +236,92 @@ impl IntrinsicInterfaceRegistry { scope: &mut v8::PinScope<'s, '_>, id: InterfaceId, ) -> Option> { - let entries = self.entries.borrow(); - let RealmInterfaceEntry::Ready(objects) = entries.get(id.index())? else { - return None; - }; - objects.public_interface.to_local(scope) + self.object(scope, id, 2) } } -pub(crate) fn retain_intrinsic_interfaces_in_realm(scope: &mut v8::PinScope<'_, '_>) { - let context = scope.get_current_context(); - let Some(registry) = context.get_slot::() else { - return; - }; - for entry in registry.entries.borrow_mut().iter_mut() { - if let RealmInterfaceEntry::Ready(objects) = entry { - objects.constructor.retain_in_realm(scope); - objects.prototype.retain_in_realm(scope); - objects.public_interface.retain_in_realm(scope); - } - } - crate::util::retain_context_v8_handle_state_for_safe_release(context, registry); -} - #[cfg(test)] mod tests { use std::pin::pin; use super::*; + #[test] + fn intrinsic_identities_live_with_the_context_not_the_rust_registry() { + crate::ensure_v8_for_test(); + let mut isolate = v8::Isolate::new(Default::default()); + let id = InterfaceId::from_callback_data(0); + let (registry, context_root, context_weak, identities) = { + let scope = pin!(v8::HandleScope::new(&mut isolate)); + let scope = &mut scope.init(); + let context = v8::Context::new(scope, Default::default()); + let scope = &mut v8::ContextScope::new(scope, context); + let registry = IntrinsicInterfaceRegistry::initialize_for_current_context( + scope, + 1, + RealmKind::Window, + ) + .unwrap(); + let constructor = v8::Object::new(scope); + let prototype = v8::Object::new(scope); + let public_interface = v8::Object::new(scope); + registry + .publish_ready(scope, id, constructor, prototype, public_interface) + .unwrap(); + let identities = [constructor, prototype, public_interface] + .map(|object| v8::Weak::new(scope, object)); + context.detach_global(); + ( + registry, + v8::Global::new(scope, context), + v8::Weak::new(scope, context), + identities, + ) + }; + + for _ in 0..2 { + isolate.low_memory_notification(); + } + { + let scope = pin!(v8::HandleScope::new(&mut isolate)); + let scope = &mut scope.init(); + let context = v8::Local::new(scope, &context_root); + let scope = &mut v8::ContextScope::new(scope, context); + let values = [ + registry.constructor(scope, id), + registry.prototype(scope, id), + registry.public_interface(scope, id), + ]; + for (value, identity) in values.into_iter().zip(&identities) { + let identity = identity + .to_local(scope) + .expect("Context must retain each intrinsic"); + assert!(value.unwrap().strict_equals(identity.into())); + } + } + + drop(context_root); + for _ in 0..2 { + isolate.low_memory_notification(); + } + let scope = pin!(v8::HandleScope::new(&mut isolate)); + let scope = &mut scope.init(); + assert!( + context_weak.to_local(scope).is_none(), + "Rust metadata must not root the Context" + ); + for identity in identities { + assert!( + identity.to_local(scope).is_none(), + "unreachable intrinsic must be collected" + ); + } + assert!(matches!( + &*registry.entry(id).unwrap(), + RealmInterfaceEntry::Ready + )); + } + #[test] fn materialization_rejects_an_out_of_range_interface() { let registry = IntrinsicInterfaceRegistry::new(1, RealmKind::Window); diff --git a/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/tests.rs b/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/tests.rs index 4a8a09fbfe..52fff725a5 100644 --- a/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/tests.rs +++ b/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/tests.rs @@ -350,7 +350,7 @@ fn intrinsic_dom_string_map_materializes_without_reading_the_public_binding() { assert!(intrinsic.strict_equals(prototype.into())); assert!(matches!( &*realm.entry(id).unwrap(), - super::realm_registry::RealmInterfaceEntry::Ready(_) + super::realm_registry::RealmInterfaceEntry::Ready )); assert!( super::ensure_intrinsic_interface_constructor(scope, "DOMStringMap") diff --git a/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/tests/finalization.rs b/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/tests/finalization.rs index 3478ac7c73..f14b628398 100644 --- a/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/tests/finalization.rs +++ b/moli-renderer-v8/src/context_bootstrap/exposed_interfaces/tests/finalization.rs @@ -287,7 +287,7 @@ fn materialized_xhr_event_target_initializes_its_private_state() { } assert!(matches!( &*interface.realm.entry(interface.id).unwrap(), - RealmInterfaceEntry::Ready(_) + RealmInterfaceEntry::Ready )); assert_eq!(lazy_getter_calls(), 0); }); @@ -341,7 +341,7 @@ fn assert_crypto_materialization(already_finalized: bool) { assert!(intrinsic.strict_equals(prototype.into())); assert!(matches!( &*realm.entry(id).unwrap(), - RealmInterfaceEntry::Ready(_) + RealmInterfaceEntry::Ready )); assert!( ensure_intrinsic_interface_constructor(scope, "Crypto") diff --git a/moli-renderer-v8/src/native_bridge/identity.rs b/moli-renderer-v8/src/native_bridge/identity.rs index d875283802..cd18509b0a 100644 --- a/moli-renderer-v8/src/native_bridge/identity.rs +++ b/moli-renderer-v8/src/native_bridge/identity.rs @@ -588,7 +588,6 @@ pub(crate) fn clear_context_wrapper_cache_for_teardown( wrapper.wrapper.borrow_mut().retain_in_realm(scope); crate::util::retain_context_v8_handle_state_for_safe_release(context, wrapper); } - crate::context_bootstrap::exposed_interfaces::retain_intrinsic_interfaces_in_realm(scope); crate::context_bootstrap::retain_indexed_db_state_in_retired_realm(scope); crate::network_host::retain_pending_network_body_state_in_retired_realm( scope,