fix(snapshot): narrow index lock scope and use std file locking

Author: suhasagg

Cargo.lock

@@ -21,7 +21,6 @@ bench = false # see https://bheisler.github.io/criterion.rs/book/faq.html#cargo-
 workspace = true
 
 [dependencies]
-fs2 = "0.4"
 gdbstub = { version = "0.7.10", optional = true }
 gdbstub_arch = { version = "0.3.3", optional = true }
 goblin = { version = "0.10", default-features = false, features = ["std", "elf32", "elf64", "endian_fd"] }

src/hyperlight_host/Cargo.toml

@@ -14,8 +14,6 @@ mod transport;
 use std::fs::OpenOptions;
 use std::path::{Path, PathBuf};
 
-use fs2::FileExt;
-
 use hyperlight_common::flatbuffer_wrappers::host_function_details::HostFunctionDetails;
 use hyperlight_common::vmem::PAGE_SIZE;
 use oci_spec::image::{
@@ -149,6 +147,22 @@ fn read_layout_marker(path: &Path) -> crate::Result<()> {
     Ok(())
 }
 
+
+/// Validate an existing OCI layout marker, if present.
+/// Returns whether the marker already exists.
+fn validate_existing_layout(path: &Path) -> crate::Result<bool> {
+    let marker = path.join("oci-layout");
+    let exists = marker
+        .try_exists()
+        .map_err(|e| crate::new_error!("save: failed to stat {:?}: {}", marker, e))?;
+
+    if exists {
+        read_layout_marker(path)?;
+    }
+
+    Ok(exists)
+}
+
 fn load_manifest(
     path: &Path,
     blobs_dir: &Path,
@@ -400,6 +414,14 @@ impl Snapshot {
             }
         }
 
+        // Reject unsupported layouts before writing snapshot data.
+        validate_existing_layout(path)?;
+        let layout_marker = path.join("oci-layout");
+
+        // Generate content-addressed snapshot blobs outside the index lock.
+        let new_desc = self.write_blobs_and_build_descriptor(path, tag, &cfg, &cfg_bytes)?;
+        let written_digest = OciDigest::from_oci_spec_digest(new_desc.digest());
+
         // Serialize layout validation and the index read-modify-write
         // transaction across cooperating processes.
         // Use a stable lock file, not index.json, which is atomically replaced.
@@ -412,40 +434,13 @@ impl Snapshot {
             .map_err(|e| {
                 crate::new_error!("save: failed to open index lock {:?}: {}", lock_path, e)
             })?;
-        lock_file.lock_exclusive().map_err(|e| {
+        lock_file.lock().map_err(|e| {
             crate::new_error!("save: failed to acquire index lock {:?}: {}", lock_path, e)
         })?;
         // File lock is released when lock_file is dropped on all exit paths.
 
-        // Validate the existing marker while holding the lock.
-        // Invalid layouts are rejected before writing snapshot data.
-        let layout_marker = path.join("oci-layout");
-        let marker_existed = layout_marker
-            .try_exists()
-            .map_err(|e| crate::new_error!("save: failed to stat {:?}: {}", layout_marker, e))?;
-        if marker_existed {
-            let bytes = read_bounded(&layout_marker, MAX_JSON_BLOB_SIZE).map_err(|e| {
-                crate::new_error!("save: failed to read existing oci-layout: {}", e)
-            })?;
-            let v: serde_json::Value = serde_json::from_slice(&bytes).map_err(|e| {
-                crate::new_error!("save: existing oci-layout is not valid JSON: {}", e)
-            })?;
-            match v.get("imageLayoutVersion").and_then(|s| s.as_str()) {
-                Some(s) if s == OCI_LAYOUT_VERSION => {}
-                Some(other) => {
-                    return Err(crate::new_error!(
-                        "save: existing imageLayoutVersion {:?} is unsupported (expected {:?})",
-                        other,
-                        OCI_LAYOUT_VERSION
-                    ));
-                }
-                None => {
-                    return Err(crate::new_error!(
-                        "save: existing oci-layout is missing imageLayoutVersion"
-                    ));
-                }
-            }
-        }
+        // Revalidate under the lock in case the layout changed.
+        let marker_existed = validate_existing_layout(path)?;
 
         let index_path = path.join("index.json");
         let index_existed = index_path
@@ -466,9 +461,6 @@ impl Snapshot {
             Vec::new()
         };
 
-        let new_desc = self.write_blobs_and_build_descriptor(path, tag, &cfg, &cfg_bytes)?;
-        let written_digest = OciDigest::from_oci_spec_digest(new_desc.digest());
-
         // Replacement is by tag, not by digest: a new snapshot may
         // hash to a different value but still claim the same logical
         // ref. Blobs from the replaced manifest become orphans.

src/hyperlight_host/src/sandbox/snapshot/file/mod.rs

@@ -1597,6 +1597,15 @@ fn save_rejects_unsupported_existing_layout_version() {
         br#"{"imageLayoutVersion":"99.0.0"}"#,
     )
     .unwrap();
+    // Capture every existing entry and its bytes before the failed save.
+    let before: Vec<_> = std::fs::read_dir(&path)
+        .unwrap()
+        .map(|entry| {
+            let entry = entry.unwrap();
+            (entry.file_name(), std::fs::read(entry.path()).unwrap())
+        })
+        .collect();
+
     let err = snap
         .save(&path, &OciTag::new("latest").unwrap())
         .unwrap_err();
@@ -1605,9 +1614,17 @@ fn save_rejects_unsupported_existing_layout_version() {
         msg.contains("imageLayoutVersion") || msg.contains("unsupported"),
         "expected unsupported-version error, got: {msg}"
     );
-    assert!(
-        !path.join("index.json").exists(),
-        "save must not have written index.json"
+    let after: Vec<_> = std::fs::read_dir(&path)
+        .unwrap()
+        .map(|entry| {
+            let entry = entry.unwrap();
+            (entry.file_name(), std::fs::read(entry.path()).unwrap())
+        })
+        .collect();
+
+    assert_eq!(
+        before, after,
+        "unsupported layout must leave directory contents unchanged"
     );
 }