From 88b304930c5fc2ffb9be5fcecf6ebbf0bfdbb117 Mon Sep 17 00:00:00 2001 From: ldm0 Date: Fri, 11 Sep 2026 03:47:55 +0800 Subject: [PATCH] fix(blob): scope realm URL cleanup to its resource owner Realm tokens are allocated per JsContextHost while the BlobStore is shared. Match both resource owner and realm token when retiring ordinary or isolated window contexts, including failed bootstrap cleanup. Extend the store and renderer regressions for colliding lifetime identifiers across two owners and two live VMs. --- moli-file-api/src/blob_store.rs | 19 +++++++++++---- moli-renderer-v8/src/blob.rs | 3 ++- .../child_frame_runtime/isolated_world.rs | 1 + .../context_host/runtime_observable.rs | 6 +++-- moli-renderer-v8/src/script_vm.rs | 8 ++++++- moli-renderer-v8/src/script_vm/post_parse.rs | 10 ++++++-- .../misc/extracted/blob_file_reader.rs | 23 +++++++++++++++---- 7 files changed, 54 insertions(+), 16 deletions(-) diff --git a/moli-file-api/src/blob_store.rs b/moli-file-api/src/blob_store.rs index d149bcef6..b39933576 100644 --- a/moli-file-api/src/blob_store.rs +++ b/moli-file-api/src/blob_store.rs @@ -270,13 +270,14 @@ where Some((String::from_utf8_lossy(&bytes).into_owned(), mime_type)) } - /// Revoke every object URL created by one execution-context lifetime. - pub fn cleanup_object_url_lifetime(&self, lifetime_id: u64) -> usize { + /// Revoke one owner's URLs for an execution-context lifetime. + /// Lifetime identifiers are local to each resource owner. + pub fn cleanup_object_url_lifetime(&self, owner_id: OwnerId, lifetime_id: u64) -> usize { let removed_blob_ids = { let mut object_urls = self.object_urls.lock(); let mut removed_blob_ids = Vec::new(); object_urls.retain(|_, state| { - if state.lifetime_id == Some(lifetime_id) { + if state.owner_id == Some(owner_id) && state.lifetime_id == Some(lifetime_id) { removed_blob_ids.push(state.blob_id); false } else { @@ -610,8 +611,11 @@ mod tests { let second_url = store .create_object_url_with_lifetime(Some(1), Some(202), blob, "https://example.test") .expect("second object URL"); + let other_owner_url = store + .create_object_url_with_lifetime(Some(2), Some(101), blob, "https://example.test") + .expect("another owner's URL with the same lifetime identifier"); - assert_eq!(store.cleanup_object_url_lifetime(101), 1); + assert_eq!(store.cleanup_object_url_lifetime(1, 101), 1); assert!(store.object_url_bytes_and_type(&first_url).is_none()); assert_eq!( store.object_url_bytes_and_type(&second_url), @@ -619,7 +623,12 @@ mod tests { ); store.release_blob_wrapper_ref(blob); - assert_eq!(store.cleanup_object_url_lifetime(202), 1); + assert_eq!(store.cleanup_object_url_lifetime(1, 202), 1); + assert_eq!( + store.object_url_bytes_and_type(&other_owner_url), + Some((b"shared".to_vec(), "text/plain".to_owned())) + ); + assert_eq!(store.cleanup_object_url_lifetime(2, 101), 1); assert!(store.blob_bytes(blob).is_none()); } } diff --git a/moli-renderer-v8/src/blob.rs b/moli-renderer-v8/src/blob.rs index ea3f0f828..ce42c88c8 100644 --- a/moli-renderer-v8/src/blob.rs +++ b/moli-renderer-v8/src/blob.rs @@ -519,9 +519,10 @@ pub(crate) fn cleanup_owner_resources(owner_id: ResourceOwnerId) { } pub(crate) fn cleanup_object_urls_for_context( + owner_id: ResourceOwnerId, context_token: native_bridge::RuntimeObservableContextToken, ) -> usize { - blob_store().cleanup_object_url_lifetime(context_token.as_u64()) + blob_store().cleanup_object_url_lifetime(owner_id, context_token.as_u64()) } fn release_blob_wrapper_ref(blob_id: BlobId) { diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/isolated_world.rs b/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/isolated_world.rs index 80c19866f..f19413971 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/isolated_world.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/isolated_world.rs @@ -139,6 +139,7 @@ impl JsContextHost { .retire_owner(WindowExecutionContextOwner::Frame(stale.local_window_id)); self.retire_window_execution_contexts_for_context_token( stale.runtime_observable_context_token, + config.resource_owner_id, ); let stale_context = v8::Local::new(scope, &stale.context); stale_context.detach_global(); diff --git a/moli-renderer-v8/src/native_bridge/context_host/runtime_observable.rs b/moli-renderer-v8/src/native_bridge/context_host/runtime_observable.rs index 884e407f5..27dcfaea4 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/runtime_observable.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/runtime_observable.rs @@ -421,9 +421,10 @@ impl JsContextHost { pub(crate) fn retire_window_execution_contexts_for_context_token( &mut self, context_token: RuntimeObservableContextToken, + resource_owner_id: crate::resource_owner::ResourceOwnerId, ) -> usize { let revoked_blob_object_url_count = - crate::blob::cleanup_object_urls_for_context(context_token); + crate::blob::cleanup_object_urls_for_context(resource_owner_id, context_token); crate::observer_runtime::retire_context_token(self, context_token); let indexed_db_retirement = self.retire_indexed_db_context(context_token); let retired_indexed_db_connections = indexed_db_retirement.retired_connections.len(); @@ -476,9 +477,10 @@ impl JsContextHost { pub(crate) fn retire_isolated_window_execution_context( &mut self, context_token: RuntimeObservableContextToken, + resource_owner_id: crate::resource_owner::ResourceOwnerId, ) -> usize { let revoked_blob_object_url_count = - crate::blob::cleanup_object_urls_for_context(context_token); + crate::blob::cleanup_object_urls_for_context(resource_owner_id, context_token); crate::observer_runtime::retire_context_token(self, context_token); let retired_realm_count = self .window_execution_context_realms diff --git a/moli-renderer-v8/src/script_vm.rs b/moli-renderer-v8/src/script_vm.rs index f22bb9119..1713547a3 100644 --- a/moli-renderer-v8/src/script_vm.rs +++ b/moli-renderer-v8/src/script_vm.rs @@ -3637,6 +3637,7 @@ impl ScriptVm { .borrow_mut() .retire_window_execution_contexts_for_context_token( context.runtime_observable_context_token, + self.resource_owner_id, ); let context_ptr = &context.context as *const v8::Global; self.renderer_document_isolate @@ -3782,6 +3783,7 @@ impl ScriptVm { for context in &stale_prebootstrapped_contexts { host.retire_window_execution_contexts_for_context_token( context.runtime_observable_context_token, + self.resource_owner_id, ); } } @@ -3876,6 +3878,7 @@ impl ScriptVm { let retired_window_execution_context_count = host .retire_window_execution_contexts_for_context_token( context.runtime_observable_context_token, + self.resource_owner_id, ); ( runtime_binding_retirement, @@ -4024,7 +4027,10 @@ impl ScriptVm { let retired_window_execution_context_realm_count = self ._context_host .borrow_mut() - .retire_isolated_window_execution_context(context.runtime_observable_context_token); + .retire_isolated_window_execution_context( + context.runtime_observable_context_token, + self.resource_owner_id, + ); tracing::debug!( execution_context_id, context_token = ?context.runtime_observable_context_token, diff --git a/moli-renderer-v8/src/script_vm/post_parse.rs b/moli-renderer-v8/src/script_vm/post_parse.rs index c09aab2f6..b57e8c40e 100644 --- a/moli-renderer-v8/src/script_vm/post_parse.rs +++ b/moli-renderer-v8/src/script_vm/post_parse.rs @@ -283,6 +283,7 @@ impl ScriptVmContextBootstrap { host_ptr, mode, runtime_observable_context_token, + resource_owner_id, )?; // SecureContext is origin-based, not document-URL-based. Initial // about:blank/srcdoc child contexts can keep about:* document URLs while @@ -432,6 +433,7 @@ enum WindowContextBootstrapMode { struct PendingWindowRealmBootstrapRegistration { host: *mut JsContextHost, realm_token: crate::native_bridge::RuntimeObservableContextToken, + resource_owner_id: ResourceOwnerId, committed: bool, } @@ -440,6 +442,7 @@ impl PendingWindowRealmBootstrapRegistration { host: *mut JsContextHost, mode: WindowContextBootstrapMode, realm_token: crate::native_bridge::RuntimeObservableContextToken, + resource_owner_id: ResourceOwnerId, ) -> Result> { let Some((owner, dispatch_scope, access_policy)) = mode.registration() else { return Ok(None); @@ -455,6 +458,7 @@ impl PendingWindowRealmBootstrapRegistration { Ok(Some(Self { host, realm_token, + resource_owner_id, committed: false, })) } @@ -467,8 +471,10 @@ impl PendingWindowRealmBootstrapRegistration { impl Drop for PendingWindowRealmBootstrapRegistration { fn drop(&mut self) { if !self.committed { - unsafe { &mut *self.host } - .retire_window_execution_contexts_for_context_token(self.realm_token); + unsafe { &mut *self.host }.retire_window_execution_contexts_for_context_token( + self.realm_token, + self.resource_owner_id, + ); } } } diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/blob_file_reader.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/blob_file_reader.rs index 663a1bf53..8b75cb9ed 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/blob_file_reader.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/blob_file_reader.rs @@ -1076,9 +1076,7 @@ fn removing_child_frame_revokes_only_its_blob_object_urls() { "", ); - let urls = vm - .eval( - r#" + let setup = r#" (() => { const parentUrl = URL.createObjectURL(new Blob(["parent"])); const frame = document.createElement("iframe"); @@ -1089,13 +1087,23 @@ fn removing_child_frame_revokes_only_its_blob_object_urls() { globalThis.__blobUrlLifetimeFrame = frame; return `${parentUrl}|${childUrl}`; })() -"#, - ) +"#; + let urls = vm + .eval(setup) .expect("child Blob object URL setup should evaluate"); let (parent_url, child_url) = urls .split_once('|') .expect("setup should return both object URLs"); + // A separate runtime starts its realm counter at the same value. Its + // object URLs must survive retirement of the first runtime's child. + let mut other_vm = new_parsed_test_vm( + "https://blob-url-child-lifetime.test/other", + "", + ); + let other_urls = other_vm.eval(setup).expect("other runtime's object URLs"); + let (_, other_child_url) = other_urls.split_once('|').expect("other child URL"); + assert_eq!( crate::blob::object_url_body_and_type(parent_url), Some(("parent".to_owned(), String::new())) @@ -1118,6 +1126,11 @@ fn removing_child_frame_revokes_only_its_blob_object_urls() { crate::blob::object_url_body_and_type(child_url).is_none(), "removing a child frame must revoke object URLs created by its realm" ); + assert_eq!( + crate::blob::object_url_body_and_type(other_child_url), + Some(("child".to_owned(), String::new())), + "realm token reuse in another runtime must not revoke that runtime's URLs" + ); } #[test]