From 2baad6720d803673d5edefe71f8a35c080228ef1 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Mon, 5 Oct 2026 02:52:51 +0200 Subject: [PATCH] fix(core): leave no temp copy when the cache adopts a blob it already holds (B187) --- crates/pdfs-core/src/cache.rs | 21 +++++++++++++++++++++ docs/BUGS.md | 17 +++++++++++++++++ docs/CHANGELOG.md | 2 ++ docs/MILESTONE-3.0.0-STATUS.md | 2 +- 4 files changed, 41 insertions(+), 1 deletion(-) diff --git a/crates/pdfs-core/src/cache.rs b/crates/pdfs-core/src/cache.rs index 18b00ff..7457d4e 100644 --- a/crates/pdfs-core/src/cache.rs +++ b/crates/pdfs-core/src/cache.rs @@ -845,6 +845,9 @@ impl ContentCache { } let bytes = std::fs::metadata(&tmp)?.len(); std::fs::rename(&tmp, &blob)?; + // Linked from the blob already cached, the temp name was a second name + // of it, and the rename did nothing (`docs/BUGS.md` B187). + let _ = std::fs::remove_file(&tmp); let meta = serde_json::to_vec(&Meta { mtime, size })?; std::fs::write(self.meta_path(uid), meta)?; self.db @@ -2197,6 +2200,24 @@ mod tests { assert_eq!(c.read_range(&u, 200, 5, 0, 5).unwrap(), b"newer"); } + /// A landed create and the revision after it can adopt the same staged + /// blob. The link to the temp name is then a second name of the cached + /// blob, and a rename between two names of one file does nothing: the + /// temp name stayed until the next start swept it (`docs/BUGS.md` B187). + #[test] + fn store_file_twice_from_the_same_blob_leaves_no_temp_file() { + let (c, d) = cache(); + let u = uid("a"); + let src = d.path().join("staged-blob"); + std::fs::write(&src, b"staged").unwrap(); + + c.store_file(&u, 100, 6, &src).unwrap(); + c.store_file(&u, 100, 6, &src).unwrap(); + + assert!(!c.blob_path(&u).with_extension("tmp").exists()); + assert_eq!(c.read_range(&u, 100, 6, 0, 6).unwrap(), b"staged"); + } + #[test] fn reconcile_rebuilds_index_from_disk() { let dir = TempDir::new(); diff --git a/docs/BUGS.md b/docs/BUGS.md index b650af6..786eb1c 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -12,6 +12,23 @@ Conventions: --- +## B187 — An uploaded file can leave a copy of itself in the cache until the next start + +**Status:** Fixed (unverified). +**Found:** 2026-10-05, writing the test for B186: after its file was deleted, the cache still +held a `.tmp` file with its bytes. + +**Where:** `ContentCache::store_file` in `crates/pdfs-core/src/cache.rs`. + +**Cause.** The cache adopts an uploaded file by linking its staged bytes to a temp name and +renaming that over the cached blob. A landed create and the revision after it can adopt the same +staged bytes. The temp name was then a second name of the cached blob, and a rename between two +names of one file does nothing. The temp file stayed, outside the cache's budget, until the next +start swept it, even after the file was evicted or deleted. + +**Fix.** The temp name is removed after the rename. Test +`store_file_twice_from_the_same_blob_leaves_no_temp_file` in `crates/pdfs-core/src/cache.rs`. + ## B186 — A file deleted offline while open fails a partial write with "Input/output error" **Status:** Fixed (unverified). diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index a1aa55a..0df8617 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -109,6 +109,8 @@ by its local id, in the queue, the stored tree and pins alike. it reaches Drive (B185). - **A file deleted while open can still be read and written until it is closed**, offline too. A write into it no longer failed with "Input/output error" (B186). +- **An uploaded file no longer leaves a copy of itself in the cache** until the next start + (B187). - **A file renamed just as its upload finished keeps its new name.** The mount no longer showed the old name, and a second rename no longer failed with "No such file or directory" (B152, B172). diff --git a/docs/MILESTONE-3.0.0-STATUS.md b/docs/MILESTONE-3.0.0-STATUS.md index 68edda5..fa4d91d 100644 --- a/docs/MILESTONE-3.0.0-STATUS.md +++ b/docs/MILESTONE-3.0.0-STATUS.md @@ -9,7 +9,7 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase - **Phase 2**: local ids, inode = local id, one tree for every mount (2.10.0). - **Phase 3, first part**: mkdir, create, rename, unlink and rmdir on the mount are recorded locally and sent from the queue; `"local_first": false` brings back the old path (5e0f30c). -- Fixes from the simulation and account runs since then: B125, B127, B130 and B141 to B186. +- Fixes from the simulation and account runs since then: B125, B127, B130 and B141 to B187. - **Sync issues, first cut** (§6.2): refusals sorted into issues, shown at once in the app, tray and CLI; Export button and `pdfs sync export`; `pdfs sync issues`; Discard button and `pdfs sync discard`; a warning badge on the node in the file browser.