From cd2cbc8541a43822c7087a991d408284677155bd Mon Sep 17 00:00:00 2001 From: ldm0 Date: Tue, 6 Oct 2026 19:41:28 +0800 Subject: [PATCH] fix(renderer): trace response body handles on realm teardown Pending Response body materializations and stored JavaScript rejection reasons held strong V8 handles in the native context host. Navigation could cancel the body fetch while leaving a resolver rooted, retaining the old Context and its native Document through the shared isolate. Move these handles into the owning realm's traced graph at teardown. Keep buffered bytes and error identity available to externally retained Responses, including pending body methods completed after the realm retires. Add native host lifetime regressions for pending bodies and stored errors, and verify retained body readers remain usable until their last V8 reference. Validation: cargo fmt --all; strict workspace/all-target/all-feature Clippy; cargo nextest run --no-fail-fast (19,650 passed, 16 skipped); release build. CI-equivalent fixture contract is clean. At 144 real-site navigations, post-blank GC leaves 1.76 MiB JS heap and two actual NativeContexts; benchmark RSS ends at 177 MiB versus 706 MiB before the fixes. --- .../src/native_bridge/identity.rs | 4 + moli-renderer-v8/src/network_host.rs | 2 +- .../src/network_host/body_source.rs | 80 +++++-- .../tests/extracted/realms_and_teardown.rs | 218 ++++++++++++++++++ 4 files changed, 287 insertions(+), 17 deletions(-) diff --git a/moli-renderer-v8/src/native_bridge/identity.rs b/moli-renderer-v8/src/native_bridge/identity.rs index 24adaa79a8..d875283802 100644 --- a/moli-renderer-v8/src/native_bridge/identity.rs +++ b/moli-renderer-v8/src/native_bridge/identity.rs @@ -590,6 +590,10 @@ pub(crate) fn clear_context_wrapper_cache_for_teardown( } 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, + include_shared_default_world, + ); } #[derive(Debug, Default)] diff --git a/moli-renderer-v8/src/network_host.rs b/moli-renderer-v8/src/network_host.rs index f2c85c1358..70344c9ff2 100644 --- a/moli-renderer-v8/src/network_host.rs +++ b/moli-renderer-v8/src/network_host.rs @@ -63,7 +63,7 @@ pub(crate) use self::body_source::{ PendingNetworkBodySourceState, close_pending_network_body_stream, enqueue_pending_network_body_chunk, error_pending_network_body_stream, error_pending_network_body_stream_with_reason, new_network_body_source_id, - pending_network_body_stream, + pending_network_body_stream, retain_pending_network_body_state_in_retired_realm, }; pub(in crate::network_host) use self::browser_response::http_status_text; pub(crate) use self::browser_response::{ diff --git a/moli-renderer-v8/src/network_host/body_source.rs b/moli-renderer-v8/src/network_host/body_source.rs index dd52105e8b..f28aec8b44 100644 --- a/moli-renderer-v8/src/network_host/body_source.rs +++ b/moli-renderer-v8/src/network_host/body_source.rs @@ -4,7 +4,7 @@ use crate::context_bootstrap::{ }; use crate::protocol_types::SubresourceResponseBody; use crate::types::NetworkBodySourceId; -use crate::util::{get_private_value, set_private_value}; +use crate::util::{RealmObjectHandle, RealmValueHandle, get_private_value, set_private_value}; use crate::web_api_interfaces; use crate::worker::get_worker_state; use std::collections::{HashMap, HashSet}; @@ -244,7 +244,7 @@ enum PendingBodyMaterializationKind { } pub(crate) struct PendingBodyMaterialization { - resolver: v8::Global, + resolver: RealmObjectHandle, kind: PendingBodyMaterializationKind, } @@ -261,10 +261,52 @@ pub(crate) struct PendingNetworkBodySourceState { pull_requested: bool, closed: bool, error: Option, - error_reason: Option>, + error_reason: Option, materializations: Vec, } +pub(crate) fn retain_pending_network_body_state_in_retired_realm( + scope: &mut v8::PinScope<'_, '_>, + include_shared_default_world: bool, +) { + let context = scope.get_current_context(); + let Some(host) = context_host_mut(scope) else { + return; + }; + // Retained Responses keep their buffered bytes and error identity. Their + // native body state must not in turn root the retired realm through a + // pending body-method resolver or stored JavaScript rejection reason. + for state in host.pending_network_body_sources.values_mut() { + for materialization in &mut state.materializations { + if include_shared_default_world + || materialization + .resolver + .to_local(scope) + .and_then(|resolver| resolver.get_creation_context(scope)) + == Some(context) + { + materialization.resolver.retain_in_realm(scope); + } + } + let stream_context = state + .stream + .to_local(scope) + .and_then(|stream| stream.get_creation_context(scope)); + if (include_shared_default_world || stream_context == Some(context)) + && let Some(reason) = &mut state.error_reason + { + // A shared native host can own body state from several realms. + // Anchor the reason in the stream's realm, even during main teardown. + if let Some(stream_context) = stream_context { + let scope = &mut v8::ContextScope::new(scope, stream_context); + reason.retain_in_realm(scope); + } else { + reason.retain_in_realm(scope); + } + } + } +} + enum PendingBodyRejection { Reason(v8::Global), Message(String), @@ -907,7 +949,7 @@ fn error_pending_network_body_stream_with_clone_ids<'s>( } state.closed = true; state.error = Some(error_text.to_owned()); - state.error_reason = Some(v8::Global::new(scope, reason)); + state.error_reason = Some(RealmValueHandle::new(scope, reason)); if let Some(stream) = state.stream.to_local(scope) { error_stream(scope, stream, reason); } @@ -926,7 +968,7 @@ fn error_pending_network_body_stream_with_clone_ids<'s>( } clone.closed = true; clone.error = Some(error_text.to_owned()); - clone.error_reason = Some(v8::Global::new(scope, reason)); + clone.error_reason = Some(RealmValueHandle::new(scope, reason)); if let Some(stream) = clone.stream.to_local(scope) { error_stream(scope, stream, reason); } @@ -1199,10 +1241,10 @@ fn clone_pending_network_body_source_in_maps( let snapshot = original.bytes.clone(); outcome.close_now = original.closed; outcome.error_now = original.error.clone(); - outcome.error_reason_now = original - .error_reason - .as_ref() - .map(|reason| v8::Global::new(scope, v8::Local::new(scope, reason))); + outcome.error_reason_now = original.error_reason.as_ref().map(|reason| { + let reason = reason.to_local(scope); + v8::Global::new(scope, reason) + }); if let Some(clone) = sources.get_mut(&clone_id) { append_pending_body_state_bytes(scope, clone, &snapshot); } @@ -1513,10 +1555,8 @@ fn inspect_pending_body_source_for_materialization<'s>( .error_reason .as_ref() .map(|reason| { - PendingBodyRejection::Reason(v8::Global::new( - scope, - v8::Local::new(scope, reason), - )) + let reason = reason.to_local(scope); + PendingBodyRejection::Reason(v8::Global::new(scope, reason)) }) .unwrap_or(PendingBodyRejection::Message(error_text)), ); @@ -1526,7 +1566,7 @@ fn inspect_pending_body_source_for_materialization<'s>( sources.remove(&id); } else { state.materializations.push(PendingBodyMaterialization { - resolver: v8::Global::new(scope, resolver), + resolver: RealmObjectHandle::new(scope, resolver.into()), kind, }); } @@ -1770,7 +1810,11 @@ fn resolve_pending_body_materializations( let mut bytes: Vec = take_pending_network_body_source_bytes(scope, id).unwrap_or_default(); let materialization_count = materializations.len(); for (index, materialization) in materializations.into_iter().enumerate() { - let resolver = v8::Local::new(scope, &materialization.resolver); + let Some(resolver) = materialization.resolver.to_local(scope) else { + continue; + }; + // The private handle is populated only from a PromiseResolver. + let resolver = unsafe { v8::Local::::cast_unchecked(resolver) }; // Most body sources have exactly one pending materialization. Move the // completed buffer into that consumer and clone only when fan-out is // actually needed. @@ -1808,7 +1852,11 @@ fn reject_pending_body_materializations_with_reason<'s>( reason: v8::Local<'s, v8::Value>, ) { for materialization in materializations { - let resolver = v8::Local::new(scope, &materialization.resolver); + let Some(resolver) = materialization.resolver.to_local(scope) else { + continue; + }; + // The private handle is populated only from a PromiseResolver. + let resolver = unsafe { v8::Local::::cast_unchecked(resolver) }; let _ = resolver.reject(scope, reason); } } 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 994db8ad5d..c8f410e256 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,223 @@ use super::*; +fn new_vm_with_pending_response_for_teardown_test() +-> (StandaloneScriptVmHarness, crate::types::NetworkBodySourceId) { + let mut vm = new_storage_test_vm("https://body-teardown.test/"); + let body_source_id = crate::network_host::new_network_body_source_id(); + let document_url = vm.document_runtime.document_url().clone(); + vm.with_default_context_scope_and_checkpoint_for_test(|scope, _| { + let response = + crate::network_host::build_fetch_response_object_from_stream_for_request_mode( + scope, + &document_url, + moli_fetch::RequestMode::Cors, + moli_fetch::ResponseHead { + final_url: document_url.join("data.json").unwrap(), + status: 200, + headers: vec![("content-type".to_owned(), b"application/json".to_vec())], + request_cookie_report: None, + cookie_set_reports: Vec::new(), + redirected: false, + redirect_chain: Vec::new(), + from_cache: false, + negotiated_http_version: None, + }, + body_source_id, + ); + let global = scope.get_current_context().global(scope); + let _ = global.set( + scope, + crate::util::v8str(scope, "response").into(), + response.into(), + ); + Ok(()) + }) + .expect("pending Response should be installed"); + (vm, body_source_id) +} + +fn error_response_for_teardown_test( + vm: &mut StandaloneScriptVmHarness, + body_source_id: crate::types::NetworkBodySourceId, +) { + vm.eval("globalThis.bodyFailure = new Error('body failed')") + .unwrap(); + vm.with_default_context_scope_and_checkpoint_for_test(|scope, _| { + let global = scope.get_current_context().global(scope); + let reason = global + .get(scope, crate::util::v8str(scope, "bodyFailure").into()) + .unwrap(); + crate::network_host::error_pending_network_body_stream_with_reason( + scope, + body_source_id, + "body failed".to_owned(), + reason, + ); + Ok(()) + }) + .expect("pending Response should retain its rejection reason"); +} + +#[test] +fn pending_response_body_releases_native_host_on_context_teardown() { + let (mut vm, _) = new_vm_with_pending_response_for_teardown_test(); + vm.eval("globalThis.bodyPromise = response.json(); 1") + .expect("body materialization should wait for network completion"); + 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 body resolver must not root its retired native Document" + ); +} + +#[test] +fn errored_response_body_releases_native_host_on_context_teardown() { + let (mut vm, body_source_id) = new_vm_with_pending_response_for_teardown_test(); + error_response_for_teardown_test(&mut vm, body_source_id); + 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 stored body error must not root its retired native Document" + ); +} + +#[test] +fn retained_response_body_keeps_bytes_until_the_last_v8_reference() { + for pending in [false, true] { + let (mut vm, body_source_id) = new_vm_with_pending_response_for_teardown_test(); + vm.with_default_context_scope_and_checkpoint_for_test(|scope, _| { + crate::network_host::enqueue_pending_network_body_chunk( + scope, + body_source_id, + br#"{"ok":true}"#.to_vec(), + ); + if !pending { + crate::network_host::close_pending_network_body_stream(scope, body_source_id); + } + Ok(()) + }) + .unwrap(); + vm.eval(if pending { + "globalThis.bodyPromise = response.text(); globalThis.readBody = () => bodyPromise" + } else { + "globalThis.readBody = () => response.text()" + }) + .unwrap(); + 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, "readBody").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); + if pending { + crate::network_host::close_pending_network_body_stream(scope, body_source_id); + } + let receiver = v8::undefined(scope).into(); + let value = crate::script_execution::call_function(scope, function, receiver, &[]) + .expect("retained Response body should remain readable"); + let promise = v8::Local::::try_from(value).unwrap(); + assert_eq!(promise.state(), v8::PromiseState::Fulfilled); + promise.result(scope).to_rust_string_lossy(scope) + }); + assert_eq!(value, r#"{"ok":true}"#); + 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 body reader must release its retired native Document" + ); + } +} + +#[test] +fn retained_response_body_keeps_error_reason_until_the_last_v8_reference() { + let (mut vm, body_source_id) = new_vm_with_pending_response_for_teardown_test(); + error_response_for_teardown_test(&mut vm, body_source_id); + vm.eval("globalThis.readBody = () => response.text()") + .unwrap(); + 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, "readBody").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()); + 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(); + let value = crate::script_execution::call_function(scope, function, receiver, &[]) + .expect("retained Response should preserve its body error"); + let promise = v8::Local::::try_from(value).unwrap(); + assert_eq!(promise.state(), v8::PromiseState::Rejected); + let reason = context + .global(scope) + .get(scope, crate::util::v8str(scope, "bodyFailure").into()) + .unwrap(); + assert!(promise.result(scope).strict_equals(reason)); + }); + 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 body reader must release its retired native Document" + ); +} + 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();