From d3b6633c33dca72daf5e6e8ccef5bd6f4cc57bcf Mon Sep 17 00:00:00 2001 From: ldm0 Date: Tue, 6 Oct 2026 18:30:23 +0800 Subject: [PATCH] fix(renderer): release module owners on context teardown Document teardown used the document.open reset, which preserves compiled module records and pending dynamic imports. Their strong V8 handles rooted retired Contexts and native Documents across shared-isolate navigations. Release module records, pending imports, graph fetches and owner events on context teardown. Preserve import-map metadata so externally retained module functions can still access native DOM values and import.meta.resolve aliases. Leave document.open module-map semantics unchanged. Add regressions for compiled modules, pending dynamic imports, and exported functions retained across teardown. All three collection regressions fail before the fix; retained functions remain usable until their last V8 reference is released. Validation: cargo fmt --all; cargo clippy --workspace --all-targets --all-features -- -D warnings; cargo nextest run --no-fail-fast (19,646 passed, 16 skipped). The initial run hit a 250ms CLI timeout stage assertion; a full run with unchanged sources passed. Release build with default Cargo parallelism. The CI real-site spider's 48-site peak RSS drops from 485.5 to 281.1 MiB, and last alive RSS drops from 398.9 to 171.6 MiB. No forced GC was added to production navigation. --- .../src/document_runtime/lifecycle.rs | 2 +- .../src/document_runtime/script_lifecycle.rs | 8 ++ moli-renderer-v8/src/host/scripts.rs | 4 + moli-renderer-v8/src/module_runtime.rs | 8 ++ .../tests/extracted/realms_and_teardown.rs | 121 ++++++++++++++++++ 5 files changed, 142 insertions(+), 1 deletion(-) diff --git a/moli-renderer-v8/src/document_runtime/lifecycle.rs b/moli-renderer-v8/src/document_runtime/lifecycle.rs index c8be855dae..af273340c3 100644 --- a/moli-renderer-v8/src/document_runtime/lifecycle.rs +++ b/moli-renderer-v8/src/document_runtime/lifecycle.rs @@ -40,7 +40,7 @@ impl DocumentRuntime { pub(crate) fn retire_v8_execution_state_for_context_teardown(&mut self) { self.timeouts = HostTimeoutScheduler::default(); self.events = HostEventTargetRegistry::default(); - self.script_lifecycle.clear_for_document_replacement(); + self.script_lifecycle.clear_for_context_teardown(); self.root_document_parser = None; self.document_write_script_preload_scanner = None; self.document_write_script_preloads.clear(); diff --git a/moli-renderer-v8/src/document_runtime/script_lifecycle.rs b/moli-renderer-v8/src/document_runtime/script_lifecycle.rs index 742928eceb..5140ac1456 100644 --- a/moli-renderer-v8/src/document_runtime/script_lifecycle.rs +++ b/moli-renderer-v8/src/document_runtime/script_lifecycle.rs @@ -114,6 +114,14 @@ impl DocumentScriptLifecycle { self.pending_main_parser_deferred_starts.clear(); } + pub(crate) fn clear_for_context_teardown(&mut self) { + self.clear_for_document_replacement(); + // document.open() preserves the current ScriptState's module map and + // dynamic imports. Context teardown must release those strong V8 + // handles before its native Document is retained by the retired realm. + self.scripts.clear_module_owner_for_context_teardown(); + } + pub(crate) fn scripts(&self) -> &HostScriptScheduler { &self.scripts } diff --git a/moli-renderer-v8/src/host/scripts.rs b/moli-renderer-v8/src/host/scripts.rs index c47bba1324..bcc612d4b5 100644 --- a/moli-renderer-v8/src/host/scripts.rs +++ b/moli-renderer-v8/src/host/scripts.rs @@ -316,6 +316,10 @@ impl HostScriptScheduler { &self.module_owner } + pub(crate) fn clear_module_owner_for_context_teardown(&mut self) { + self.module_owner.clear_for_context_teardown(); + } + fn default_page_task_execution_kind_for_source( source: ScriptHandleSource, ) -> ScriptPageTaskExecutionKind { diff --git a/moli-renderer-v8/src/module_runtime.rs b/moli-renderer-v8/src/module_runtime.rs index ca77ac351f..9eb10b94c1 100644 --- a/moli-renderer-v8/src/module_runtime.rs +++ b/moli-renderer-v8/src/module_runtime.rs @@ -357,6 +357,14 @@ pub(crate) struct ModuleOwnerState { } impl ModuleOwnerState { + pub(crate) fn clear_for_context_teardown(&mut self) { + // Retained import.meta.resolve functions still use the import map, but + // compiled records and pending imports must not root the retired realm. + self.document_modulator = NativeDocumentModulator::default(); + self.graph_fetches.clear(); + self.owner_event_tasks = DocumentPostedTaskSource::default(); + } + pub(crate) fn clear_for_document_replacement(&mut self) { // Import maps, module-map entries, compiled records, ID allocation and // dynamic-import resolver state belong to the live ScriptState. Only diff --git a/moli-renderer-v8/src/script_vm/tests/extracted/realms_and_teardown.rs b/moli-renderer-v8/src/script_vm/tests/extracted/realms_and_teardown.rs index 7f9dcf1f05..994db8ad5d 100644 --- a/moli-renderer-v8/src/script_vm/tests/extracted/realms_and_teardown.rs +++ b/moli-renderer-v8/src/script_vm/tests/extracted/realms_and_teardown.rs @@ -1,5 +1,126 @@ use super::*; +fn new_vm_with_evaluated_module_for_teardown_test() -> StandaloneScriptVmHarness { + let mut vm = new_storage_test_vm("https://module-teardown.test/"); + let url = vm.document_runtime.document_url().clone(); + vm.document_runtime + .register_import_map_source( + r#"{"imports":{"retained":"https://module-teardown.test/retained.mjs"}}"#, + ) + .expect("module import map should register"); + let source = crate::module_runtime::ModuleSource::text( + "export const node = document.createElement('p');\n\ + node.textContent = 'module';\n\ + export function readNative() { return node.textContent + ':' + import.meta.resolve('retained'); }\n\ + globalThis.readNative = readNative;" + .to_owned(), + ); + let mut job = crate::module_runtime::runtime_owned_loaded_module_script_graph_job( + &mut vm, + source, + &url, + &url, + &crate::planning::ScriptFetchMetadata::default(), + false, + ) + .expect("module graph should be accepted"); + let crate::module_runtime::NativeModuleGraphJobAdvance::Complete(graph) = job + .advance_module_script_owner_lane(&mut vm) + .expect("import-free module graph should complete") + else { + panic!("an import-free module must not fetch"); + }; + vm.instantiate_native_module_graph(&graph) + .expect("module graph should instantiate"); + vm.evaluate_native_module_graph(graph.root_entry) + .expect("module graph should evaluate"); + assert_eq!( + vm.eval("readNative()").unwrap(), + "module:https://module-teardown.test/retained.mjs" + ); + vm +} + +#[test] +fn compiled_module_releases_native_host_on_context_teardown() { + let vm = new_vm_with_evaluated_module_for_teardown_test(); + let weak_host = vm.context_host_weak_for_test(); + let isolate = vm.renderer_document_isolate.clone(); + drop(vm); + for _ in 0..2 { + isolate.with_renderer_document_isolate_mut(|isolate| isolate.low_memory_notification()); + } + assert!( + weak_host.upgrade().is_none(), + "compiled module records must not root their retired native Document" + ); +} + +#[test] +fn pending_dynamic_import_releases_native_host_on_context_teardown() { + let mut vm = new_storage_test_vm("https://pending-module-teardown.test/"); + vm.eval("void import('./never-ready.mjs')") + .expect("dynamic import should queue"); + assert!(vm.document_runtime.has_ready_native_dynamic_module_import()); + let weak_host = vm.context_host_weak_for_test(); + let isolate = vm.renderer_document_isolate.clone(); + drop(vm); + for _ in 0..2 { + isolate.with_renderer_document_isolate_mut(|isolate| isolate.low_memory_notification()); + } + assert!( + weak_host.upgrade().is_none(), + "a pending dynamic import must not root its retired native Document" + ); +} + +#[test] +fn retained_module_function_keeps_native_values_until_the_last_v8_reference() { + let vm = new_vm_with_evaluated_module_for_teardown_test(); + let isolate = vm.renderer_document_isolate.clone(); + let callback = isolate.with_renderer_document_isolate_mut(|isolate| { + let scope = std::pin::pin!(v8::HandleScope::new(isolate)); + let scope = &mut scope.init(); + let context = v8::Local::new(scope, &vm.page_default_context); + let scope = &mut v8::ContextScope::new(scope, context); + let value = context + .global(scope) + .get(scope, crate::util::v8str(scope, "readNative").into()) + .unwrap(); + let function = v8::Local::::try_from(value).unwrap(); + v8::Global::new(scope, function) + }); + let weak_host = vm.context_host_weak_for_test(); + drop(vm); + for _ in 0..2 { + isolate.with_renderer_document_isolate_mut(|isolate| isolate.low_memory_notification()); + } + assert!(weak_host.upgrade().is_some()); + let value = isolate.with_renderer_document_isolate_mut(|isolate| { + let scope = std::pin::pin!(v8::HandleScope::new(isolate)); + let scope = &mut scope.init(); + let caller_context = v8::Context::new(scope, Default::default()); + let scope = &mut v8::ContextScope::new(scope, caller_context); + let function = v8::Local::new(scope, &callback); + let context = function.get_creation_context(scope).unwrap(); + let scope = &mut v8::ContextScope::new(scope, context); + let receiver = v8::undefined(scope).into(); + crate::script_execution::call_function(scope, function, receiver, &[]) + .expect("retained module function should read its native DOM value") + .to_rust_string_lossy(scope) + }); + assert_eq!(value, "module:https://module-teardown.test/retained.mjs"); + isolate.with_renderer_document_isolate_mut(|isolate| { + drop(callback); + isolate.low_memory_notification(); + }); + isolate.with_renderer_document_isolate_mut(|isolate| isolate.low_memory_notification()); + assert!( + weak_host.upgrade().is_none(), + "releasing the exported function must release its retired native Document" + ); +} + #[test] fn unpromoted_child_realm_releases_native_host_on_document_teardown() { let vm = new_vm_with_unpromoted_child_realm_for_teardown_test();