Fix static file paths and race conditions (#473)

* fix: Use entrypoint path as static file root when available. * fix(graph): don't spawn tasks in the current runtime * fix: fmt * stamp: typing * stamp(graph): use semaphore to ensure strong count is exactly one --------- Co-authored-by: Nyannyacha <meow@nnc.gg>
supabase · Jan 15, 2025 · 085c61e · 085c61e
1 parent 8989dd3
commit 085c61e
Show file tree

Hide file tree

Showing 9 changed files with 65 additions and 32 deletions.
diff --git a/crates/base/src/deno_runtime.rs b/crates/base/src/deno_runtime.rs
@@ -479,7 +479,7 @@ where
             .unwrap_or_default();
 
         if is_some_entry_point {
-            main_module_url = Url::parse(&maybe_entrypoint.unwrap())?;
+            main_module_url = Url::parse(&maybe_entrypoint.clone().unwrap())?;
         }
 
         let mut net_access_disabled = false;
@@ -620,9 +620,18 @@ where
             .and_then(serde_json::Value::as_bool)
             .unwrap_or_default();
 
+        let static_root_path = if is_some_entry_point {
+            match Url::parse(&maybe_entrypoint.clone().unwrap())?.to_file_path() {
+                Ok(path) => path.parent().unwrap().to_path_buf(),
+                Err(_) => base_dir_path,
+            }
+        } else {
+            base_dir_path
+        };
+
         let rt_provider = create_module_loader_for_standalone_from_eszip_kind(
             eszip,
-            base_dir_path.clone(),
+            static_root_path.clone(),
             maybe_import_map,
             import_map_path,
             has_inspector || need_source_map,
@@ -658,7 +667,7 @@ where
         let file_system = build_file_system_fn(if is_user_worker {
             Arc::new(StaticFs::new(
                 static_files,
-                base_dir_path,
+                static_root_path,
                 vfs_path,
                 vfs,
                 npm_snapshot,

diff --git a/crates/fs/impl/deno_compile_fs.rs b/crates/fs/impl/deno_compile_fs.rs
@@ -7,7 +7,7 @@ use std::path::PathBuf;
 use std::rc::Rc;
 use std::sync::Arc;
 
-use crate::rt::SYNC_IO_RT;
+use crate::rt::IO_RT;
 
 use super::virtual_fs::FileBackedVfs;
 
@@ -159,9 +159,9 @@ impl FileSystem for DenoCompileFileSystem {
                 let this = self.clone();
 
                 s.spawn(move || {
-                    SYNC_IO_RT.block_on(async move {
-                        this.copy_to_real_path_async(oldpath, newpath).await
-                    })
+                    IO_RT.block_on(
+                        async move { this.copy_to_real_path_async(oldpath, newpath).await },
+                    )
                 })
                 .join()
                 .unwrap()

diff --git a/crates/fs/impl/static_fs.rs b/crates/fs/impl/static_fs.rs
@@ -1,4 +1,4 @@
-use crate::rt::SYNC_IO_RT;
+use crate::rt::IO_RT;
 use crate::{EszipStaticFiles, FileBackedVfs};
 use deno_core::normalize_path;
 use deno_fs::{AccessCheckCb, FsDirEntry, FsFileType, OpenOptions};
@@ -13,7 +13,7 @@ use std::sync::Arc;
 #[derive(Debug, Clone)]
 pub struct StaticFs {
     static_files: EszipStaticFiles,
-    base_dir_path: PathBuf,
+    root_path: PathBuf,
     vfs_path: PathBuf,
     snapshot: Option<ValidSerializedNpmResolutionSnapshot>,
     vfs: Arc<FileBackedVfs>,
@@ -22,15 +22,15 @@ pub struct StaticFs {
 impl StaticFs {
     pub fn new(
         static_files: EszipStaticFiles,
-        base_dir_path: PathBuf,
+        root_path: PathBuf,
         vfs_path: PathBuf,
         vfs: Arc<FileBackedVfs>,
         snapshot: Option<ValidSerializedNpmResolutionSnapshot>,
     ) -> Self {
         Self {
             vfs,
             static_files,
-            base_dir_path,
+            root_path,
             vfs_path,
             snapshot,
         }
@@ -336,7 +336,7 @@ impl deno_fs::FileSystem for StaticFs {
         } else {
             let eszip = self.vfs.eszip.as_ref();
             let path = if path.is_relative() {
-                self.base_dir_path.join(path)
+                self.root_path.join(path)
             } else {
                 path.to_path_buf()
             };
@@ -349,7 +349,7 @@ impl deno_fs::FileSystem for StaticFs {
                 .and_then(|it| eszip.ensure_module(it))
             {
                 let Some(res) = std::thread::scope(|s| {
-                    s.spawn(move || SYNC_IO_RT.block_on(async move { file.source().await }))
+                    s.spawn(move || IO_RT.block_on(async move { file.source().await }))
                         .join()
                         .unwrap()
                 }) else {

diff --git a/crates/fs/impl/virtual_fs.rs b/crates/fs/impl/virtual_fs.rs
@@ -30,7 +30,7 @@ use sb_core::util::checksum;
 use sb_core::util::fs::canonicalize_path;
 use thiserror::Error;
 
-use crate::rt::SYNC_IO_RT;
+use crate::rt::IO_RT;
 
 #[derive(Error, Debug)]
 #[error(
@@ -652,7 +652,7 @@ impl deno_io::fs::File for FileBackedVfsFile {
         std::thread::scope(|s| {
             let inner = (*self).clone();
 
-            s.spawn(move || SYNC_IO_RT.block_on(inner.read_to_buf(buf)))
+            s.spawn(move || IO_RT.block_on(inner.read_to_buf(buf)))
                 .join()
                 .unwrap()
         })
@@ -682,7 +682,7 @@ impl deno_io::fs::File for FileBackedVfsFile {
         std::thread::scope(|s| {
             let inner = (*self).clone();
 
-            s.spawn(move || SYNC_IO_RT.block_on(inner.read_to_end()))
+            s.spawn(move || IO_RT.block_on(inner.read_to_end()))
                 .join()
                 .unwrap()
         })

diff --git a/crates/fs/lib.rs b/crates/fs/lib.rs
@@ -22,6 +22,7 @@ pub use r#impl::s3_fs;
 pub use r#impl::static_fs;
 pub use r#impl::tmp_fs;
 pub use r#impl::virtual_fs;
+pub use rt::IO_RT;
 
 pub struct VfsOpts {
     pub npm_resolver: Arc<dyn CliNpmResolver>,
@@ -35,7 +36,7 @@ impl<T> LazyEszipV2 for T where T: std::ops::Deref<Target = EszipV2> + AsyncEszi
 
 pub async fn extract_static_files_from_eszip<P>(
     eszip: &dyn LazyEszipV2,
-    mapped_base_dir_path: P,
+    mapped_static_root_path: P,
 ) -> EszipStaticFiles
 where
     P: AsRef<Path>,
@@ -66,7 +67,7 @@ where
         };
 
         files.insert(
-            normalize_path(mapped_base_dir_path.as_ref().join(path)),
+            normalize_path(mapped_static_root_path.as_ref().join(path)),
             specifier.to_string(),
         );
     }

diff --git a/crates/fs/rt.rs b/crates/fs/rt.rs
@@ -1,9 +1,9 @@
 use once_cell::sync::Lazy;
 
-pub(crate) static SYNC_IO_RT: Lazy<tokio::runtime::Runtime> = Lazy::new(|| {
+pub static IO_RT: Lazy<tokio::runtime::Runtime> = Lazy::new(|| {
     tokio::runtime::Builder::new_multi_thread()
         .enable_all()
-        .thread_name("sb-virtualfs-io")
+        .thread_name("io")
         .build()
         .unwrap()
 });
diff --git a/crates/graph/lib.rs b/crates/graph/lib.rs
@@ -28,7 +28,7 @@ use std::fs::{create_dir_all, File};
 use std::io::{Cursor, SeekFrom, Write};
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
-use tokio::sync::Mutex;
+use tokio::sync::{Mutex, Semaphore};
 
 mod eszip_parse;
 
@@ -44,6 +44,8 @@ pub mod resolver;
 
 pub use eszip::v2::Checksum;
 
+const READ_ALL_BARRIER_MAX_PERMITS: usize = 10;
+
 #[derive(Debug, Clone, Copy, Serialize, Deserialize)]
 #[serde(rename_all = "snake_case")]
 pub enum DecoratorType {
@@ -158,14 +160,20 @@ impl LazyLoadableEszip {
 
         if let Some(section) = self.maybe_data_section.clone() {
             let specifier = module.specifier.clone();
+            let sem = section.read_all_barrier.clone();
+
+            drop(fs::IO_RT.spawn(async move {
+                let permit = sem.acquire_owned().await.unwrap();
 
-            drop(tokio::spawn(async move {
                 match section.read_data_section_by_specifier(&specifier).await {
                     Ok(_) => {}
                     Err(err) => {
                         error!("failed to read module data from the data section: {}", err);
                     }
                 }
+
+                drop(section);
+                drop(permit);
             }));
         }
 
@@ -222,6 +230,7 @@ pub struct EszipDataSection {
     sources_len: Arc<Mutex<Option<u64>>>,
     locs_by_specifier: Arc<Mutex<Option<HashMap<String, EszipDataSectionMetadata>>>>,
     loaded_locs_by_specifier: Arc<Mutex<HashMap<String, EszipDataLoc>>>,
+    read_all_barrier: Arc<Semaphore>,
 }
 
 impl EszipDataSection {
@@ -239,6 +248,7 @@ impl EszipDataSection {
             sources_len: Arc::default(),
             locs_by_specifier: Arc::default(),
             loaded_locs_by_specifier: Arc::default(),
+            read_all_barrier: Arc::new(Semaphore::new(READ_ALL_BARRIER_MAX_PERMITS)),
         }
     }
 
@@ -408,7 +418,20 @@ impl EszipDataSection {
     pub async fn read_data_section_all(self: Arc<Self>) -> Result<(), ParseError> {
         // NOTE: Below codes is roughly originated from eszip@0.72.2/src/v2.rs
 
-        let this = Arc::into_inner(self).unwrap();
+        let this = {
+            let sem = self.read_all_barrier.clone();
+            let permit = sem
+                .acquire_many(READ_ALL_BARRIER_MAX_PERMITS as u32)
+                .await
+                .unwrap();
+
+            let this = Arc::into_inner(self).unwrap();
+
+            sem.close();
+            drop(permit);
+            this
+        };
+
         let modules = this.modules;
         let checksum_size = this
             .options

diff --git a/crates/module_loader/standalone/mod.rs b/crates/module_loader/standalone/mod.rs
@@ -63,7 +63,7 @@ impl RootCertStoreProvider for StandaloneRootCertStoreProvider {
 
 pub async fn create_module_loader_for_eszip<P>(
     mut eszip: LazyLoadableEszip,
-    base_dir_path: P,
+    static_root_path: P,
     metadata: Metadata,
     maybe_import_map: Option<ImportMap>,
     include_source_map: bool,
@@ -143,7 +143,7 @@ where
     .map(FastString::from);
 
     let snapshot = eszip.take_npm_snapshot();
-    let static_files = extract_static_files_from_eszip(&eszip, base_dir_path).await;
+    let static_files = extract_static_files_from_eszip(&eszip, static_root_path).await;
     let vfs_root_dir_path = npm_cache_dir.root_dir().to_owned();
 
     let (fs, vfs) = {
@@ -251,7 +251,7 @@ where
 
 pub async fn create_module_loader_for_standalone_from_eszip_kind<P>(
     eszip_payload_kind: EszipPayloadKind,
-    base_dir_path: P,
+    static_root_path: P,
     maybe_import_map: Option<ImportMap>,
     maybe_import_map_path: Option<String>,
     include_source_map: bool,
@@ -289,7 +289,7 @@ where
 
     create_module_loader_for_eszip(
         eszip,
-        base_dir_path,
+        static_root_path,
         Metadata {
             ca_stores: None,
             ca_data: None,

diff --git a/examples/serve-html/index.ts b/examples/serve-html/index.ts
@@ -1,9 +1,9 @@
 import { join } from 'https://deno.land/std/path/mod.ts';
 
 Deno.serve(async (req) => {
-	if (req.url.endsWith('/foo')) {
-		return new Response(await Deno.readTextFile(join(import.meta.dirname, 'foo.html')));
-	} else {
-		return new Response(await Deno.readTextFile(join(import.meta.dirname, 'bar.html')));
-	}
+    if (req.url.endsWith('/foo')) {
+        return new Response(await Deno.readTextFile(new URL('./foo.html', import.meta.url)));
+    } else {
+        return new Response(await Deno.readTextFile(new URL('./bar.html', import.meta.url)));
+    }
 });