From 96d37aed18eeb9dc50953621aa395f32ef8fb81d Mon Sep 17 00:00:00 2001 From: PathGao Date: Thu, 27 Aug 2026 10:24:08 +0800 Subject: [PATCH 1/3] fix(save): delete the temp file the rename left behind, and the ones earlier runs left MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A save writes a sibling temp file, fsyncs it and renames it over the document. On the machine in #722 a 0-byte `.doc.md.markpad-tmp-…` is left beside the document by every save, and the save itself SUCCEEDS: no error reaches the app, the tab's modified dot clears, and the document holds the new text. So the rename returned success and the source name is still in the directory afterwards — which is not a thing the code can cause, and not a thing that happens on a clean machine or in the reporter's VM. What is different there is an endpoint agent (亚信安全 TrustOne) sitting in the filesystem; auto-save is not involved, since turning it off changes nothing. Three changes, none of which needs to know what the agent is doing: The rename is followed by a check for the file it should have consumed, and removes it if it is there. On every other machine this is one `exists()` on a path that is gone, and it is the only place that knows the name at the moment it appears. The temp file's handle is now closed before the rename and before any cleanup, rather than at the end of the function. `MoveFileExW` and `DeleteFileW` fail with a sharing violation while a handle without `FILE_SHARE_DELETE` is open, and ours joined whatever a scanner was already holding on a file created an instruction ago; a delete that does get through with a handle open only marks the file delete-pending, so it stays in the directory meanwhile. The next save of a document also sweeps leftovers for that document that no live write could own: nothing under a minute old, then anything empty, and anything older than an hour. Empty and not-empty are separated because a temp file with contents can be the only copy of a document, from a process that died between the fsync and the rename, while an empty one can never be anything but garbage. Cleanup failures are also folded into the error the save returns instead of being dropped by `let _ =`, so the next report of this comes with the reason attached. ref #722 --- src-tauri/src/fs_safety.rs | 219 ++++++++++++++++++++++++++++++++++++- 1 file changed, 215 insertions(+), 4 deletions(-) diff --git a/src-tauri/src/fs_safety.rs b/src-tauri/src/fs_safety.rs index 5b55e2ad..3caf1bcc 100644 --- a/src-tauri/src/fs_safety.rs +++ b/src-tauri/src/fs_safety.rs @@ -3,17 +3,102 @@ //! //! Split out of `lib.rs`; the code and its tests are unchanged. +use std::collections::HashSet; use std::fs; use std::io::Write; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicU64, Ordering}; -use std::time::{SystemTime, UNIX_EPOCH}; +use std::sync::{LazyLock, Mutex}; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; /// Distinguishes the temp files of `atomic_write` calls that share a process. /// Never reset, and read with `fetch_add` so no two callers can be handed the /// same value — the property the wall clock could not supply. static TEMP_FILE_SEQ: AtomicU64 = AtomicU64::new(0); +/// Temp-name prefixes already swept this run — see `sweep_abandoned_temps`. +static SWEPT_TEMP_PREFIXES: LazyLock>> = + LazyLock::new(|| Mutex::new(HashSet::new())); + +/// Below this, a temp file could belong to a write that is still running — +/// this process's own or another Markpad's — and nothing may touch it. +const IN_FLIGHT_GRACE: Duration = Duration::from_secs(60); + +/// How old a temp file that still has contents has to be before it counts as +/// abandoned. Long, because such a file can be the only copy of a document a +/// crash caught between the `fsync` and the rename. +const ABANDONED_TEMP_AGE: Duration = Duration::from_secs(60 * 60); + +/// Remove the temp file this call created, and if that fails, say so in the +/// error being returned. +/// +/// The two are one thing on purpose. A write or rename that fails leaves the +/// temp file behind, and the reason the CLEANUP failed is the interesting half +/// — the failure that reaches the user is "could not save", while the file +/// they then find in their folder is explained by an error nobody kept +/// (#722). `let _ =` discarded exactly that. +fn remove_temp_or_say_why(error: std::io::Error, temp_path: &Path) -> std::io::Error { + match fs::remove_file(temp_path) { + Ok(()) => error, + // Already gone is the outcome this wanted. + Err(cleanup) if cleanup.kind() == std::io::ErrorKind::NotFound => error, + Err(cleanup) => std::io::Error::new( + error.kind(), + format!( + "{error} — and {} could not be cleaned up either: {cleanup}", + temp_path.display() + ), + ), + } +} + +/// Delete temp files for `file_name` that an earlier run left in `parent`. +/// +/// Cleanup at the time of the failure is the first line and stays the main +/// one, but it cannot run at all when the process dies mid-write, and it can +/// be refused by whatever refused the write. Both leave a file the user has +/// to find and delete by hand, which is what #722 is: a portable Windows +/// build accumulating `.doc.md.markpad-tmp-…` siblings across sessions. +/// +/// `IN_FLIGHT_GRACE` is what keeps this off a write that is still happening — +/// a temp file lives for one `write_all` and one `fsync`, so a minute is four +/// orders of magnitude past any of them, on any volume. Past that, two kinds +/// of leftover with different stakes: +/// +/// - **Empty**: there is nothing in it to lose, so it goes. This is the #722 +/// shape, and waiting an hour to clear a file that can never be anything but +/// garbage only means the user finds it first. +/// - **Not empty**: it may be the only copy of a document, from a process that +/// died between the `fsync` and the rename. `ABANDONED_TEMP_AGE` gives that +/// a wide berth before Markpad decides it is garbage. +fn sweep_abandoned_temps(parent: &Path, file_name: &str) { + let prefix = format!(".{file_name}.markpad-tmp-"); + let Ok(entries) = fs::read_dir(parent) else { + return; + }; + for entry in entries.flatten() { + let name = entry.file_name(); + let Some(name) = name.to_str() else { continue }; + if !name.starts_with(&prefix) { + continue; + } + let Ok(meta) = entry.metadata() else { continue }; + let Some(age) = meta + .modified() + .ok() + .and_then(|modified| SystemTime::now().duration_since(modified).ok()) + else { + continue; + }; + if age < IN_FLIGHT_GRACE { + continue; + } + if meta.len() == 0 || age >= ABANDONED_TEMP_AGE { + let _ = fs::remove_file(entry.path()); + } + } +} + /// Write `bytes` to `target` durably and atomically: write to a sibling temp /// file, fsync it, then rename over the target. Atomic on both Unix and /// modern Windows — `std::fs::rename` calls `MoveFileExW` with @@ -81,6 +166,18 @@ pub(crate) fn atomic_write(target: &Path, bytes: &[u8]) -> std::io::Result<()> { .map(|n| n.to_string_lossy().into_owned()) .unwrap_or_else(|| "markpad".to_string()); + // Once per document per run. The scan is for garbage from earlier runs, so + // repeating it would find nothing new — and it would put a `read_dir` in + // front of every 1.5s auto-save, which on a network folder is the kind of + // cost that shows up as a stutter while typing. + let first_write_here = SWEPT_TEMP_PREFIXES + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .insert(parent_path.join(&file_name)); + if first_write_here { + sweep_abandoned_temps(&parent_path, &file_name); + } + // Claim a temp file nobody else holds. The name must be unique or two // concurrent writers of the same target collide, and a collision is not a // harmless retry: the loser used to delete `temp_path` on its way out, @@ -126,9 +223,18 @@ pub(crate) fn atomic_write(target: &Path, bytes: &[u8]) -> std::io::Result<()> { Ok(()) })(); + // Closed before the rename and before any cleanup, rather than at the end + // of the function. Both of those operations are ones an open handle takes + // part in on Windows: `MoveFileExW` and `DeleteFileW` fail with a sharing + // violation when a handle without `FILE_SHARE_DELETE` is open on the file, + // and our own handle joins whatever an antivirus scanner or indexer is + // already holding on a file created one instruction ago. A `DeleteFileW` + // that does get through with a handle still open only marks the file + // delete-pending, so it stays visible in the directory meanwhile. + drop(file); + if let Err(e) = write_result { - let _ = fs::remove_file(&temp_path); - return Err(e); + return Err(remove_temp_or_say_why(e, &temp_path)); } // Atomic on both Unix and modern Windows: std::fs::rename uses @@ -139,8 +245,24 @@ pub(crate) fn atomic_write(target: &Path, bytes: &[u8]) -> std::io::Result<()> { // we clean up the temp file and surface the original error without // touching the target. if let Err(e) = fs::rename(&temp_path, target) { + return Err(remove_temp_or_say_why(e, &temp_path)); + } + + // A rename that reported success has left nothing at the old name, and on + // every machine this has been run on that is what happens. #722 is a + // machine where it does not: an endpoint agent (亚信安全 TrustOne) sits in + // the filesystem, and the reporter gets one 0-byte `.doc.md.markpad-tmp-…` + // per save while the save itself succeeds — no error, the tab's modified + // dot clears, the document holds the new text. Whatever the agent is doing + // with the source name, this call is the moment we still know it, and one + // `remove_file` on a path that is normally already gone is a cheaper way + // to find out than any amount of reasoning about filter drivers. + // + // Best-effort, and deliberately not folded into the error above: the write + // has landed, the document is correct, and a save must not start failing + // over a file that should not be there in the first place. + if temp_path.exists() { let _ = fs::remove_file(&temp_path); - return Err(e); } // Best-effort restore of the original mode bits. If this fails (e.g. the @@ -759,6 +881,95 @@ pub(crate) mod tests { fs::remove_dir_all(dir).unwrap(); } + #[test] + fn a_temp_file_an_earlier_run_abandoned_is_swept_by_the_next_write() { + // #722: temp files accumulating beside a document across sessions. + // The next save of that document is the moment Markpad knows both the + // folder and the name, and the only moment it is guaranteed to be + // looking at them. + let dir = temp_path("atomic-sweep"); + fs::create_dir_all(&dir).unwrap(); + let path = dir.join("notes.md"); + fs::write(&path, b"body").unwrap(); + + // Old, and with contents — the case that has to wait out + // `ABANDONED_TEMP_AGE`, because a temp file with a document in it can + // be the only copy of that document. + let stale = dir.join(".notes.md.markpad-tmp-4242-1787728728439865400-4"); + fs::write(&stale, b"a whole document").unwrap(); + set_modified_ago(&stale, Duration::from_secs(3 * 60 * 60)); + + // Empty, and only minutes old. Nothing in it can be lost, and this is + // the shape #722 produces one of per save. + let empty = dir.join(".notes.md.markpad-tmp-4242-1787728728439865400-5"); + fs::write(&empty, b"").unwrap(); + set_modified_ago(&empty, Duration::from_secs(5 * 60)); + + // Another document's leftovers, equally stale. The sweep is scoped to + // the file being written, so a folder full of documents is cleaned by + // the saves that touch each of them rather than by the first save to + // reach the folder. + let neighbour = dir.join(".other.md.markpad-tmp-4242-1787728728439865400-6"); + fs::write(&neighbour, b"").unwrap(); + set_modified_ago(&neighbour, Duration::from_secs(3 * 60 * 60)); + + atomic_write(&path, b"edited").unwrap(); + + assert!( + !stale.exists(), + "an abandoned temp file must not survive a save" + ); + assert!( + !empty.exists(), + "an empty temp file has nothing in it to wait for" + ); + assert!( + neighbour.exists(), + "the sweep must not reach past the document being written", + ); + assert_eq!(fs::read(&path).unwrap(), b"edited"); + + fs::remove_dir_all(dir).unwrap(); + } + + #[test] + fn a_temp_file_that_could_still_be_in_flight_is_left_alone() { + // The dangerous half of a sweep: another Markpad, or another thread + // here, is between `create_new` and `rename` right now. Deleting its + // temp file fails its rename with ENOENT — the exact breakage + // `concurrent_atomic_writes_to_one_target_all_succeed` exists for. A + // write in that window is milliseconds old, and it is EMPTY for the + // stretch between `create_new` and `write_all`, so age has to be the + // first question the sweep asks and the empty-file rule the second. + let dir = temp_path("atomic-sweep-live"); + fs::create_dir_all(&dir).unwrap(); + let path = dir.join("notes.md"); + fs::write(&path, b"body").unwrap(); + + let just_created = dir.join(".notes.md.markpad-tmp-4242-1787728728439865400-4"); + fs::write(&just_created, b"").unwrap(); + let half_written = dir.join(".notes.md.markpad-tmp-4242-1787728728439865400-5"); + fs::write(&half_written, b"half a document").unwrap(); + + atomic_write(&path, b"edited").unwrap(); + + assert!( + just_created.exists(), + "a temp file young enough to be a live write must survive, empty or not", + ); + assert!(half_written.exists()); + + fs::remove_dir_all(dir).unwrap(); + } + + /// Backdate `path`'s mtime, which is what the sweep reads to tell an + /// abandoned temp file from a live one. + fn set_modified_ago(path: &Path, ago: Duration) { + let file = fs::OpenOptions::new().write(true).open(path).unwrap(); + file.set_times(fs::FileTimes::new().set_modified(SystemTime::now() - ago)) + .unwrap(); + } + #[test] fn concurrent_atomic_writes_to_one_target_all_succeed() { // Two threads used to derive the same temp name from the same clock From d709efd4316e4c2fb0207bcaef23fd001608960d Mon Sep 17 00:00:00 2001 From: PathGao Date: Thu, 27 Aug 2026 19:52:50 +0800 Subject: [PATCH 2/3] fix(save): clear leftovers beside the documents Markpad remembers, at startup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sweep a save performs reaches the folder of a document someone is still editing, and no other. A document that accumulated temp files and is then left alone keeps them for good — and the machine in #722 produces one per save, so a document put aside after an afternoon's work is exactly where a pile of them ends up. Startup asks for the documents Markpad already knows about: the recent list, and whatever the session restored. That is at most nine folders plus the open tabs, one `read_dir` each, off the main thread, with nothing on screen waiting for the result. The policy is the one the per-save sweep uses and it stays in one function, so the two cannot come to disagree about what a leftover is. Every window calls it and only the first pays: the sweep is once per document per run and Markpad's windows share a process. Not gated on the version upgrade. A marker file that says "cleaned once, at 2.7.7" is state to keep, to migrate and to get wrong, and what it would buy is skipping a `read_dir` on nine folders at startup. ref #722 --- src-tauri/src/app.rs | 1 + src-tauri/src/commands.rs | 26 +++++++++++- src-tauri/src/fs_safety.rs | 74 +++++++++++++++++++++++++++++------ src/lib/MarkdownViewer.svelte | 18 +++++++++ 4 files changed, 107 insertions(+), 12 deletions(-) diff --git a/src-tauri/src/app.rs b/src-tauri/src/app.rs index aa1229a8..eff358e9 100644 --- a/src-tauri/src/app.rs +++ b/src-tauri/src/app.rs @@ -235,6 +235,7 @@ pub fn run() { commands::canonicalize_path, commands::read_file_as_data_url, commands::save_file_content, + commands::sweep_temp_files, commands::export_pdf_windows, commands::print_pdf, commands::is_win11, diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index fe0c74a6..6f502c0c 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -5,7 +5,7 @@ use crate::fs_safety::{ atomic_write, canonical_identity, encode_text, ensure_path_within_root, read_to_string_lossy, - resolve_image_directory, safe_path_component, + resolve_image_directory, safe_path_component, sweep_document_temps, }; use crate::markdown::{build_markdown_preview, convert_markdown, heading_anchors, HeadingAnchor}; use crate::window_runtime::{self, WatcherState}; @@ -226,6 +226,30 @@ pub async fn save_file_content( .unwrap_or_else(|e| Err(e.to_string())) } +/// Clear temp files earlier runs left beside the documents this window knows +/// about — the recent-file list, and whatever the session restored. +/// +/// A save already sweeps the document it is writing, and that is the sweep +/// that matters while someone is working. It cannot reach a document nobody +/// opens again, whose leftovers would otherwise stay in the user's folder for +/// good, so startup asks for the documents Markpad remembers (#722). +/// +/// Every window calls this and only the first pays for it: the sweep is once +/// per document per run, and Markpad's windows share a process. +/// +/// Async and off the main thread: this is a `read_dir` per document on folders +/// that may live on a network volume, and nothing waits for the result. +#[tauri::command] +pub async fn sweep_temp_files(paths: Vec) -> Result<(), String> { + tauri::async_runtime::spawn_blocking(move || { + for path in paths { + sweep_document_temps(Path::new(&path)); + } + }) + .await + .map_err(|e| e.to_string()) +} + /// Resolve `path` to the identity the filesystem gives it — see /// `canonical_identity` for what that folds and why the filesystem, rather /// than a platform guess, is the thing being asked. diff --git a/src-tauri/src/fs_safety.rs b/src-tauri/src/fs_safety.rs index 3caf1bcc..1aa8def4 100644 --- a/src-tauri/src/fs_safety.rs +++ b/src-tauri/src/fs_safety.rs @@ -52,6 +52,40 @@ fn remove_temp_or_say_why(error: std::io::Error, temp_path: &Path) -> std::io::E } } +/// Clear temp files earlier runs left beside `document`, once per document +/// per run. +/// +/// Two callers, because one of them cannot reach everything. A save sweeps the +/// document it is writing, which clears a folder the next time the user edits +/// what is in it — and never clears the folder of a document nobody edits +/// again. Startup sweeps the documents Markpad already knows about (the recent +/// list, and whatever the session restored), which is where a leftover would +/// otherwise sit for good. +/// +/// Once per document per run because the scan only ever finds garbage from +/// earlier runs: repeating it finds nothing new, and it would put a `read_dir` +/// in front of every 1.5s auto-save, which on a network folder is the kind of +/// cost that shows up as a stutter while typing. +pub(crate) fn sweep_document_temps(document: &Path) { + let Some(file_name) = document + .file_name() + .map(|name| name.to_string_lossy().into_owned()) + else { + return; + }; + let parent = match document.parent() { + Some(parent) if !parent.as_os_str().is_empty() => parent.to_path_buf(), + _ => PathBuf::from("."), + }; + let first_time = SWEPT_TEMP_PREFIXES + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .insert(parent.join(&file_name)); + if first_time { + sweep_abandoned_temps(&parent, &file_name); + } +} + /// Delete temp files for `file_name` that an earlier run left in `parent`. /// /// Cleanup at the time of the failure is the first line and stays the main @@ -166,17 +200,7 @@ pub(crate) fn atomic_write(target: &Path, bytes: &[u8]) -> std::io::Result<()> { .map(|n| n.to_string_lossy().into_owned()) .unwrap_or_else(|| "markpad".to_string()); - // Once per document per run. The scan is for garbage from earlier runs, so - // repeating it would find nothing new — and it would put a `read_dir` in - // front of every 1.5s auto-save, which on a network folder is the kind of - // cost that shows up as a stutter while typing. - let first_write_here = SWEPT_TEMP_PREFIXES - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()) - .insert(parent_path.join(&file_name)); - if first_write_here { - sweep_abandoned_temps(&parent_path, &file_name); - } + sweep_document_temps(target); // Claim a temp file nobody else holds. The name must be unique or two // concurrent writers of the same target collide, and a collision is not a @@ -932,6 +956,34 @@ pub(crate) mod tests { fs::remove_dir_all(dir).unwrap(); } + #[test] + fn a_document_nobody_edits_again_is_still_swept() { + // The reason `sweep_document_temps` is reachable without a write: the + // per-save sweep only ever clears the folder of a document someone is + // still editing. Startup calls this for the recent list and the + // restored tabs, which is the only thing that reaches leftovers beside + // a document that is never saved again. + let dir = temp_path("atomic-sweep-startup"); + fs::create_dir_all(&dir).unwrap(); + let path = dir.join("archived.md"); + fs::write(&path, b"body").unwrap(); + + let leftover = dir.join(".archived.md.markpad-tmp-4242-1787728728439865400-4"); + fs::write(&leftover, b"").unwrap(); + set_modified_ago(&leftover, Duration::from_secs(5 * 60)); + + sweep_document_temps(&path); + + assert!(!leftover.exists()); + assert_eq!( + fs::read(&path).unwrap(), + b"body", + "sweeping must not touch the document itself", + ); + + fs::remove_dir_all(dir).unwrap(); + } + #[test] fn a_temp_file_that_could_still_be_in_flight_is_left_alone() { // The dangerous half of a sweep: another Markpad, or another thread diff --git a/src/lib/MarkdownViewer.svelte b/src/lib/MarkdownViewer.svelte index 6656cbce..e3b9a7cc 100644 --- a/src/lib/MarkdownViewer.svelte +++ b/src/lib/MarkdownViewer.svelte @@ -3244,6 +3244,24 @@ import { createDocumentSession, type LoadMarkdownOptions } from './sessions/docu await windowSession.claimTransferredTab(); if (isDisposed) return; + // Clear temp files an earlier run left beside the documents this + // window knows about. A save sweeps the document it writes, which + // covers everything the user is working on; this covers the one a + // save cannot reach — a document nobody edits again, whose + // leftovers would otherwise stay in the folder for good (#722). + // + // Not awaited, and its failure is not the user's problem: nothing + // on screen depends on it, and a folder that cannot be read is a + // folder with nothing to clean. + void invoke('sweep_temp_files', { + paths: [ + ...new Set([ + ...recentFiles, + ...tabManager.tabs.map((tab) => tab.path).filter(hasRealFilePath), + ]), + ], + }).catch((error) => console.error('Failed to sweep temp files', error)); + const urlParams = new URLSearchParams(window.location.search); const fileParam = urlParams.get('file'); From 452cd0d49662de7fdf6fbdfd05bb574af3aeb405 Mon Sep 17 00:00:00 2001 From: PathGao <42336971+PathGao@users.noreply.github.com> Date: Wed, 2 Sep 2026 09:14:29 +0800 Subject: [PATCH 3/3] diag: log temp-file removal around atomic_write for #722 (do not merge) Appends one line per rename, post-rename check, delayed re-check (200ms/2s/10s) and sweep removal to %TEMP%\markpad-diag.log, with the raw OS error code. Built for the reporter of #722 to run once. --- src-tauri/src/fs_safety.rs | 68 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 66 insertions(+), 2 deletions(-) diff --git a/src-tauri/src/fs_safety.rs b/src-tauri/src/fs_safety.rs index 1aa8def4..389ab522 100644 --- a/src-tauri/src/fs_safety.rs +++ b/src-tauri/src/fs_safety.rs @@ -11,6 +11,45 @@ use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{LazyLock, Mutex}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; +/// DIAGNOSTIC BUILD for #722 — never merge. Appends one line per event to +/// `%TEMP%\markpad-diag.log` (`std::env::temp_dir()`), so the reporter can +/// show what `remove_file` returns on their machine. +pub(crate) fn diag(msg: &str) { + let ms = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_millis()) + .unwrap_or(0); + if let Ok(mut f) = fs::OpenOptions::new() + .append(true) + .create(true) + .open(std::env::temp_dir().join("markpad-diag.log")) + { + let _ = writeln!(f, "{ms} pid={} {msg}", std::process::id()); + } +} + +fn diag_remove(what: &str, path: &Path) { + let exists = path.exists(); + let len = fs::metadata(path).map(|m| m.len()).ok(); + if !exists { + diag(&format!("{what}: {} exists=false", path.display())); + return; + } + match fs::remove_file(path) { + Ok(()) => diag(&format!( + "{what}: {} exists=true len={len:?} remove=Ok still_exists={}", + path.display(), + path.exists() + )), + Err(e) => diag(&format!( + "{what}: {} exists=true len={len:?} remove=Err({e}) os_error={:?} still_exists={}", + path.display(), + e.raw_os_error(), + path.exists() + )), + } +} + /// Distinguishes the temp files of `atomic_write` calls that share a process. /// Never reset, and read with `fetch_add` so no two callers can be handed the /// same value — the property the wall clock could not supply. @@ -81,6 +120,7 @@ pub(crate) fn sweep_document_temps(document: &Path) { .lock() .unwrap_or_else(|poisoned| poisoned.into_inner()) .insert(parent.join(&file_name)); + diag(&format!("sweep_document_temps: {} first_time={first_time}", document.display())); if first_time { sweep_abandoned_temps(&parent, &file_name); } @@ -124,11 +164,17 @@ fn sweep_abandoned_temps(parent: &Path, file_name: &str) { else { continue; }; + diag(&format!( + "sweep candidate: {} len={} age_s={}", + entry.path().display(), + meta.len(), + age.as_secs() + )); if age < IN_FLIGHT_GRACE { continue; } if meta.len() == 0 || age >= ABANDONED_TEMP_AGE { - let _ = fs::remove_file(entry.path()); + diag_remove("sweep", &entry.path()); } } } @@ -268,9 +314,27 @@ pub(crate) fn atomic_write(target: &Path, bytes: &[u8]) -> std::io::Result<()> { // rename fails (e.g. target locked by another process on Windows), // we clean up the temp file and surface the original error without // touching the target. - if let Err(e) = fs::rename(&temp_path, target) { + let rename_result = fs::rename(&temp_path, target); + diag(&format!( + "rename: {} -> {} result={:?} os_error={:?}", + temp_path.display(), + target.display(), + rename_result.as_ref().map(|_| ()).map_err(|e| e.to_string()), + rename_result.as_ref().err().and_then(|e| e.raw_os_error()) + )); + if let Err(e) = rename_result { return Err(remove_temp_or_say_why(e, &temp_path)); } + diag_remove("post-rename", &temp_path); + { + let later = temp_path.clone(); + std::thread::spawn(move || { + for ms in [200u64, 2_000, 10_000] { + std::thread::sleep(Duration::from_millis(ms)); + diag_remove(&format!("recheck+{ms}ms"), &later); + } + }); + } // A rename that reported success has left nothing at the old name, and on // every machine this has been run on that is what happens. #722 is a