Skip to content
Closed
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions src/persist/io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,34 @@ fn save_json_to_path<T: serde::Serialize>(path: &Path, snapshot: &T) -> std::io:
let tmp_path = target.with_extension("json.tmp");
std::fs::write(&tmp_path, &json)?;
if let Err(err) = std::fs::rename(&tmp_path, &target) {
if is_cross_filesystem_rename_error(err.kind()) {
return write_fallback(&target, &tmp_path, &json);
}
let _ = std::fs::remove_file(&tmp_path);
return Err(err);
}
Ok(())
}

// Writes directly to `target` when `rename` cannot be used. The temp file at
// `tmp_path` still holds a complete copy of `json`, so it is only removed
// once the direct write has actually landed; if the direct write fails
// partway through, the temp file remains as a recovery copy.
fn write_fallback(target: &Path, tmp_path: &Path, json: &str) -> std::io::Result<()> {
let result = std::fs::write(target, json);
if result.is_ok() {
let _ = std::fs::remove_file(tmp_path);
}
Comment thread
greptile-apps[bot] marked this conversation as resolved.
result
}

fn is_cross_filesystem_rename_error(kind: std::io::ErrorKind) -> bool {
matches!(
kind,
std::io::ErrorKind::ResourceBusy | std::io::ErrorKind::CrossesDevices
)
}

pub(super) fn save_to_paths(
session_path: &Path,
history_path: &Path,
Expand Down Expand Up @@ -337,4 +359,59 @@ mod tests {
.is_symlink());
assert!(target.exists());
}

#[test]
fn cross_filesystem_rename_error_detection() {
assert!(is_cross_filesystem_rename_error(
std::io::ErrorKind::ResourceBusy
));
assert!(is_cross_filesystem_rename_error(
std::io::ErrorKind::CrossesDevices
));
assert!(!is_cross_filesystem_rename_error(
std::io::ErrorKind::PermissionDenied
));
}

#[test]
fn save_to_path_propagates_unrelated_rename_errors() {
// Renaming the temp file onto an existing directory fails with an
// unrelated rename error (the exact kind varies by platform). Herdr
// must still report the error and clean up the temp file.
let target = temp_session_path("rename-onto-directory");
std::fs::create_dir_all(&target).unwrap();

let err = save_to_path(&target, &empty_snapshot()).unwrap_err();

assert!(!is_cross_filesystem_rename_error(err.kind()));
assert!(!target.with_extension("json.tmp").exists());
}

#[test]
fn write_fallback_removes_tmp_file_on_success() {
let target = temp_session_path("fallback-success");
std::fs::create_dir_all(target.parent().unwrap()).unwrap();
let tmp_path = target.with_extension("json.tmp");
std::fs::write(&tmp_path, "{}").unwrap();

write_fallback(&target, &tmp_path, "{}").unwrap();

assert!(target.exists());
assert!(!tmp_path.exists());
}

#[test]
fn write_fallback_keeps_tmp_file_as_recovery_copy_on_failure() {
// `target`'s parent directory does not exist, so the direct write
// fails. The temp file must survive so the last known-good snapshot
// is not lost.
let target = temp_session_path("fallback-failure");
let tmp_path = temp_session_path("fallback-failure-tmp");
std::fs::create_dir_all(tmp_path.parent().unwrap()).unwrap();
std::fs::write(&tmp_path, "{}").unwrap();

write_fallback(&target, &tmp_path, "{}").unwrap_err();

assert!(tmp_path.exists());
}
}
Loading