From 62a38e996c36cb332871a0b9de110f74d4aad9a5 Mon Sep 17 00:00:00 2001 From: Johnathan Sharratt Date: Thu, 30 Nov 2023 12:53:40 +1100 Subject: [PATCH 1/2] Fixed an issue where the package loader was blocking the tokio runtime --- .../runtime/package_loader/builtin_loader.rs | 66 +++++++++++-------- 1 file changed, 40 insertions(+), 26 deletions(-) diff --git a/lib/wasix/src/runtime/package_loader/builtin_loader.rs b/lib/wasix/src/runtime/package_loader/builtin_loader.rs index 34ffd58e210..4a8d0d8bf07 100644 --- a/lib/wasix/src/runtime/package_loader/builtin_loader.rs +++ b/lib/wasix/src/runtime/package_loader/builtin_loader.rs @@ -115,9 +115,12 @@ impl BuiltinPackageLoader { if dist.webc.scheme() == "file" { match crate::runtime::resolver::utils::file_path_from_url(&dist.webc) { Ok(path) => { - // FIXME: This will block the thread - let bytes = std::fs::read(&path) - .with_context(|| format!("Unable to read \"{}\"", path.display()))?; + let bytes = tokio::task::spawn_blocking({ + let path = path.clone(); + move || std::fs::read(&path) + }) + .await? + .with_context(|| format!("Unable to read \"{}\"", path.display()))?; return Ok(bytes.into()); } Err(e) => { @@ -222,7 +225,10 @@ impl PackageLoader for BuiltinPackageLoader { // in a smart way to keep memory usage down. if let Some(cache) = &self.cache { - match cache.save_and_load_as_mmapped(&bytes, &summary.dist).await { + match cache + .save_and_load_as_mmapped(bytes.clone(), &summary.dist) + .await + { Ok(container) => { tracing::debug!("Cached to disk"); self.in_memory.save(&container, summary.dist.webc_sha256); @@ -274,7 +280,10 @@ impl FileSystemCache { #[cfg(target_arch = "wasm32")] let container = Container::from_disk(&path); #[cfg(not(target_arch = "wasm32"))] - let container = tokio::task::block_in_place(|| Container::from_disk(&path)); + let container = { + let path = path.clone(); + tokio::task::spawn_blocking(move || Container::from_disk(&path)).await? + }; match container { Ok(c) => Ok(Some(c)), Err(ContainerError::Open { error, .. }) @@ -291,27 +300,32 @@ impl FileSystemCache { } } - async fn save(&self, webc: &[u8], dist: &DistributionInfo) -> Result<(), Error> { + async fn save(&self, webc: Bytes, dist: &DistributionInfo) -> Result<(), Error> { let path = self.path(&dist.webc_sha256); - - let parent = path.parent().expect("Always within cache_dir"); - - std::fs::create_dir_all(parent) - .with_context(|| format!("Unable to create \"{}\"", parent.display()))?; - - let mut temp = NamedTempFile::new_in(parent)?; - temp.write_all(webc)?; - temp.flush()?; - temp.as_file_mut().sync_all()?; - temp.persist(&path)?; - - tracing::debug!( - pkg.hash=%dist.webc_sha256, - pkg.url=%dist.webc, - path=%path.display(), - num_bytes=webc.len(), - "Saved to disk", - ); + let dist = dist.clone(); + + tokio::task::spawn_blocking(move || { + let parent = path.parent().expect("Always within cache_dir"); + + std::fs::create_dir_all(parent) + .with_context(|| format!("Unable to create \"{}\"", parent.display()))?; + + let mut temp = NamedTempFile::new_in(parent)?; + temp.write_all(&webc)?; + temp.flush()?; + temp.as_file_mut().sync_all()?; + temp.persist(&path)?; + + tracing::debug!( + pkg.hash=%dist.webc_sha256, + pkg.url=%dist.webc, + path=%path.display(), + num_bytes=webc.len(), + "Saved to disk", + ); + Result::<_, Error>::Ok(()) + }) + .await??; Ok(()) } @@ -319,7 +333,7 @@ impl FileSystemCache { #[tracing::instrument(level = "debug", skip_all)] async fn save_and_load_as_mmapped( &self, - webc: &[u8], + webc: Bytes, dist: &DistributionInfo, ) -> Result { // First, save it to disk From daab4aa0589f473de0de8db338703bab8b440527 Mon Sep 17 00:00:00 2001 From: Johnathan Sharratt Date: Mon, 11 Mar 2024 18:40:36 +1100 Subject: [PATCH 2/2] Lint fixes --- lib/wasix/src/runtime/package_loader/builtin_loader.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/wasix/src/runtime/package_loader/builtin_loader.rs b/lib/wasix/src/runtime/package_loader/builtin_loader.rs index 2771e372a41..9fa90960f08 100644 --- a/lib/wasix/src/runtime/package_loader/builtin_loader.rs +++ b/lib/wasix/src/runtime/package_loader/builtin_loader.rs @@ -117,7 +117,7 @@ impl BuiltinPackageLoader { Ok(path) => { let bytes = crate::spawn_blocking({ let path = path.clone(); - move || std::fs::read(&path) + move || std::fs::read(path) }) .await? .with_context(|| format!("Unable to read \"{}\"", path.display()))?; @@ -279,7 +279,7 @@ impl FileSystemCache { let container = crate::spawn_blocking({ let path = path.clone(); - move || Container::from_disk(&path) + move || Container::from_disk(path) }) .await?; match container {