refactor: asynchronous blob backing store (#10969)

Co-authored-by: Luca Casonato <hello@lcas.dev>

cli/file_fetcher.rs

@@ -19,7 +19,7 @@ use deno_core::futures;
 use deno_core::futures::future::FutureExt;
 use deno_core::ModuleSpecifier;
 use deno_runtime::deno_fetch::reqwest;
-use deno_runtime::deno_web::BlobUrlStore;
+use deno_runtime::deno_web::BlobStore;
 use deno_runtime::permissions::Permissions;
 use log::debug;
 use log::info;
@@ -212,7 +212,7 @@ pub struct FileFetcher {
   cache_setting: CacheSetting,
   http_cache: HttpCache,
   http_client: reqwest::Client,
-  blob_url_store: BlobUrlStore,
+  blob_store: BlobStore,
 }
 
 impl FileFetcher {
@@ -221,7 +221,7 @@ impl FileFetcher {
     cache_setting: CacheSetting,
     allow_remote: bool,
     ca_data: Option<Vec<u8>>,
-    blob_url_store: BlobUrlStore,
+    blob_store: BlobStore,
   ) -> Result<Self, AnyError> {
     Ok(Self {
       auth_tokens: AuthTokens::new(env::var(DENO_AUTH_TOKENS).ok()),
@@ -230,7 +230,7 @@ impl FileFetcher {
       cache_setting,
       http_cache,
       http_client: create_http_client(get_user_agent(), ca_data)?,
-      blob_url_store,
+      blob_store,
     })
   }
 
@@ -360,7 +360,7 @@ impl FileFetcher {
   }
 
   /// Get a blob URL.
-  fn fetch_blob_url(
+  async fn fetch_blob_url(
     &self,
     specifier: &ModuleSpecifier,
   ) -> Result<File, AnyError> {
@@ -381,20 +381,24 @@ impl FileFetcher {
       ));
     }
 
-    let blob_url_storage = self.blob_url_store.borrow();
-    let blob = blob_url_storage.get(specifier.clone())?.ok_or_else(|| {
-      custom_error(
-        "NotFound",
-        format!("Blob URL not found: \"{}\".", specifier),
-      )
-    })?;
+    let blob = {
+      let blob_store = self.blob_store.borrow();
+      blob_store
+        .get_object_url(specifier.clone())?
+        .ok_or_else(|| {
+          custom_error(
+            "NotFound",
+            format!("Blob URL not found: \"{}\".", specifier),
+          )
+        })?
+    };
 
-    let content_type = blob.media_type;
+    let content_type = blob.media_type.clone();
+    let bytes = blob.read_all().await?;
 
     let (media_type, maybe_charset) =
       map_content_type(specifier, Some(content_type.clone()));
-    let source =
-      strip_shebang(get_source_from_bytes(blob.data, maybe_charset)?);
+    let source = strip_shebang(get_source_from_bytes(bytes, maybe_charset)?);
 
     let local =
       self
@@ -525,7 +529,7 @@ impl FileFetcher {
       }
       result
     } else if scheme == "blob" {
-      let result = self.fetch_blob_url(specifier);
+      let result = self.fetch_blob_url(specifier).await;
       if let Ok(file) = &result {
         self.cache.insert(specifier.clone(), file.clone());
       }
@@ -580,6 +584,7 @@ mod tests {
   use deno_core::resolve_url;
   use deno_core::resolve_url_or_path;
   use deno_runtime::deno_web::Blob;
+  use deno_runtime::deno_web::InMemoryBlobPart;
   use std::rc::Rc;
   use tempfile::TempDir;
 
@@ -588,28 +593,28 @@ mod tests {
     maybe_temp_dir: Option<Rc<TempDir>>,
   ) -> (FileFetcher, Rc<TempDir>) {
     let (file_fetcher, temp_dir, _) =
-      setup_with_blob_url_store(cache_setting, maybe_temp_dir);
+      setup_with_blob_store(cache_setting, maybe_temp_dir);
     (file_fetcher, temp_dir)
   }
 
-  fn setup_with_blob_url_store(
+  fn setup_with_blob_store(
     cache_setting: CacheSetting,
     maybe_temp_dir: Option<Rc<TempDir>>,
-  ) -> (FileFetcher, Rc<TempDir>, BlobUrlStore) {
+  ) -> (FileFetcher, Rc<TempDir>, BlobStore) {
     let temp_dir = maybe_temp_dir.unwrap_or_else(|| {
       Rc::new(TempDir::new().expect("failed to create temp directory"))
     });
     let location = temp_dir.path().join("deps");
-    let blob_url_store = BlobUrlStore::default();
+    let blob_store = BlobStore::default();
     let file_fetcher = FileFetcher::new(
       HttpCache::new(&location),
       cache_setting,
       true,
       None,
-      blob_url_store.clone(),
+      blob_store.clone(),
     )
     .expect("setup failed");
-    (file_fetcher, temp_dir, blob_url_store)
+    (file_fetcher, temp_dir, blob_store)
   }
 
   macro_rules! file_url {
@@ -948,16 +953,18 @@ mod tests {
 
   #[tokio::test]
   async fn test_fetch_blob_url() {
-    let (file_fetcher, _, blob_url_store) =
-      setup_with_blob_url_store(CacheSetting::Use, None);
+    let (file_fetcher, _, blob_store) =
+      setup_with_blob_store(CacheSetting::Use, None);
+
+    let bytes =
+      "export const a = \"a\";\n\nexport enum A {\n  A,\n  B,\n  C,\n}\n"
+        .as_bytes()
+        .to_vec();
 
-    let specifier = blob_url_store.insert(
+    let specifier = blob_store.insert_object_url(
       Blob {
-        data:
-          "export const a = \"a\";\n\nexport enum A {\n  A,\n  B,\n  C,\n}\n"
-            .as_bytes()
-            .to_vec(),
         media_type: "application/typescript".to_string(),
+        parts: vec![Arc::new(Box::new(InMemoryBlobPart::from(bytes)))],
       },
       None,
     );
@@ -1049,7 +1056,7 @@ mod tests {
       CacheSetting::ReloadAll,
       true,
       None,
-      BlobUrlStore::default(),
+      BlobStore::default(),
     )
     .expect("setup failed");
     let result = file_fetcher
@@ -1076,7 +1083,7 @@ mod tests {
       CacheSetting::Use,
       true,
       None,
-      BlobUrlStore::default(),
+      BlobStore::default(),
     )
     .expect("could not create file fetcher");
     let specifier =
@@ -1104,7 +1111,7 @@ mod tests {
       CacheSetting::Use,
       true,
       None,
-      BlobUrlStore::default(),
+      BlobStore::default(),
     )
     .expect("could not create file fetcher");
     let result = file_fetcher_02
@@ -1265,7 +1272,7 @@ mod tests {
       CacheSetting::Use,
       true,
       None,
-      BlobUrlStore::default(),
+      BlobStore::default(),
     )
     .expect("could not create file fetcher");
     let specifier =
@@ -1296,7 +1303,7 @@ mod tests {
       CacheSetting::Use,
       true,
       None,
-      BlobUrlStore::default(),
+      BlobStore::default(),
     )
     .expect("could not create file fetcher");
     let result = file_fetcher_02
@@ -1406,7 +1413,7 @@ mod tests {
       CacheSetting::Use,
       false,
       None,
-      BlobUrlStore::default(),
+      BlobStore::default(),
     )
     .expect("could not create file fetcher");
     let specifier =
@@ -1433,15 +1440,15 @@ mod tests {
       CacheSetting::Only,
       true,
       None,
-      BlobUrlStore::default(),
+      BlobStore::default(),
     )
     .expect("could not create file fetcher");
     let file_fetcher_02 = FileFetcher::new(
       HttpCache::new(&location),
       CacheSetting::Use,
       true,
       None,
-      BlobUrlStore::default(),
+      BlobStore::default(),
     )
     .expect("could not create file fetcher");
     let specifier =

cli/lsp/registries.rs

@@ -26,7 +26,7 @@ use deno_core::serde_json::json;
 use deno_core::url::Position;
 use deno_core::url::Url;
 use deno_core::ModuleSpecifier;
-use deno_runtime::deno_web::BlobUrlStore;
+use deno_runtime::deno_web::BlobStore;
 use deno_runtime::permissions::Permissions;
 use log::error;
 use lspower::lsp;
@@ -264,7 +264,7 @@ impl Default for ModuleRegistry {
       cache_setting,
       true,
       None,
-      BlobUrlStore::default(),
+      BlobStore::default(),
     )
     .unwrap();
 
@@ -283,7 +283,7 @@ impl ModuleRegistry {
       CacheSetting::Use,
       true,
       None,
-      BlobUrlStore::default(),
+      BlobStore::default(),
     )
     .context("Error creating file fetcher in module registry.")
     .unwrap();

cli/main.rs

@@ -121,7 +121,7 @@ fn create_web_worker_callback(
       ts_version: version::TYPESCRIPT.to_string(),
       no_color: !colors::use_color(),
       get_error_class_fn: Some(&crate::errors::get_error_class_name),
-      blob_url_store: program_state.blob_url_store.clone(),
+      blob_store: program_state.blob_store.clone(),
       broadcast_channel: program_state.broadcast_channel.clone(),
     };
 
@@ -207,7 +207,7 @@ pub fn create_main_worker(
         .join("location_data")
         .join(checksum::gen(&[loc.to_string().as_bytes()]))
     }),
-    blob_url_store: program_state.blob_url_store.clone(),
+    blob_store: program_state.blob_store.clone(),
     broadcast_channel: program_state.broadcast_channel.clone(),
   };
 

cli/program_state.rs

@@ -16,7 +16,7 @@ use crate::source_maps::SourceMapGetter;
 use crate::specifier_handler::FetchHandler;
 use crate::version;
 use deno_runtime::deno_broadcast_channel::InMemoryBroadcastChannel;
-use deno_runtime::deno_web::BlobUrlStore;
+use deno_runtime::deno_web::BlobStore;
 use deno_runtime::inspector_server::InspectorServer;
 use deno_runtime::permissions::Permissions;
 
@@ -53,7 +53,7 @@ pub struct ProgramState {
   pub maybe_import_map: Option<ImportMap>,
   pub maybe_inspector_server: Option<Arc<InspectorServer>>,
   pub ca_data: Option<Vec<u8>>,
-  pub blob_url_store: BlobUrlStore,
+  pub blob_store: BlobStore,
   pub broadcast_channel: InMemoryBroadcastChannel,
 }
 
@@ -79,15 +79,15 @@ impl ProgramState {
       CacheSetting::Use
     };
 
-    let blob_url_store = BlobUrlStore::default();
+    let blob_store = BlobStore::default();
     let broadcast_channel = InMemoryBroadcastChannel::default();
 
     let file_fetcher = FileFetcher::new(
       http_cache,
       cache_usage,
       !flags.no_remote,
       ca_data.clone(),
-      blob_url_store.clone(),
+      blob_store.clone(),
     )?;
 
     let lockfile = if let Some(filename) = &flags.lock {
@@ -146,7 +146,7 @@ impl ProgramState {
       maybe_import_map,
       maybe_inspector_server,
       ca_data,
-      blob_url_store,
+      blob_store,
       broadcast_channel,
     };
     Ok(Arc::new(program_state))

cli/specifier_handler.rs

@@ -574,7 +574,7 @@ pub mod tests {
   use crate::file_fetcher::CacheSetting;
   use crate::http_cache::HttpCache;
   use deno_core::resolve_url_or_path;
-  use deno_runtime::deno_web::BlobUrlStore;
+  use deno_runtime::deno_web::BlobStore;
   use tempfile::TempDir;
 
   macro_rules! map (
@@ -599,7 +599,7 @@ pub mod tests {
       CacheSetting::Use,
       true,
       None,
-      BlobUrlStore::default(),
+      BlobStore::default(),
     )
     .expect("could not setup");
     let disk_cache = deno_dir.gen_cache;

cli/standalone.rs

@@ -24,7 +24,7 @@ use deno_core::ModuleLoader;
 use deno_core::ModuleSpecifier;
 use deno_core::OpState;
 use deno_runtime::deno_broadcast_channel::InMemoryBroadcastChannel;
-use deno_runtime::deno_web::BlobUrlStore;
+use deno_runtime::deno_web::BlobStore;
 use deno_runtime::permissions::Permissions;
 use deno_runtime::permissions::PermissionsOptions;
 use deno_runtime::worker::MainWorker;
@@ -213,7 +213,7 @@ pub async fn run(
   let main_module = resolve_url(SPECIFIER)?;
   let program_state = ProgramState::build(flags).await?;
   let permissions = Permissions::from_options(&metadata.permissions);
-  let blob_url_store = BlobUrlStore::default();
+  let blob_store = BlobStore::default();
   let broadcast_channel = InMemoryBroadcastChannel::default();
   let module_loader = Rc::new(EmbeddedModuleLoader(source_code));
   let create_web_worker_cb = Arc::new(|_| {
@@ -246,7 +246,7 @@ pub async fn run(
     get_error_class_fn: Some(&get_error_class_name),
     location: metadata.location,
     origin_storage_dir: None,
-    blob_url_store,
+    blob_store,
     broadcast_channel,
   };
   let mut worker =

cli/tests/blob_gc_finalization.js

@@ -0,0 +1,11 @@
+// This test creates 1024 blobs of 128 MB each. This will only work if the blobs
+// and their backing data is GCed as expected.
+for (let i = 0; i < 1024; i++) {
+  // Create a 128MB byte array, and then a blob from it.
+  const buf = new Uint8Array(128 * 1024 * 1024);
+  new Blob([buf]);
+  // It is very important that there is a yield here, otherwise the finalizer
+  // for the blob is not called and the memory is not freed.
+  await new Promise((resolve) => setTimeout(resolve, 0));
+}
+console.log("GCed all blobs");

cli/tests/blob_gc_finalization.js.out

@@ -0,0 +1 @@
+GCed all blobs

cli/tests/integration/run_tests.rs

@@ -366,6 +366,12 @@ itest!(js_import_detect {
   exit_code: 0,
 });
 
+itest!(blob_gc_finalization {
+  args: "run blob_gc_finalization.js",
+  output: "blob_gc_finalization.js.out",
+  exit_code: 0,
+});
+
 itest!(lock_write_requires_lock {
   args: "run --lock-write some_file.ts",
   output: "lock_write_requires_lock.out",