Skip to content

Fix file write durability by properly handling sync_all errors #176

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
logbie merged 2 commits into from
Nov 30, 2025
Merged

Fix file write durability by properly handling sync_all errors #176

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eb91824
Fix file write durability by properly handling sync_all errors
claude[bot] Nov 30, 2025
6bdbf40
Fix Windows sync_all permission errors in concurrent scenarios
claude[bot] Nov 30, 2025
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
70 changes: 61 additions & 9 deletions src/interpreter/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,9 +450,27 @@ impl IoClient {
// Flush the data to ensure it's written to disk
match file_clone.flush().await {
Ok(_) => {
// Try to sync to disk for durability, but don't fail if it's not supported
let _ = file_clone.sync_all().await;
Ok(())
// Sync to disk for durability
match file_clone.sync_all().await {
Ok(_) => Ok(()),
Err(e) => {
// On Windows, sync_all can fail with "Access denied" in concurrent scenarios
// This is often a limitation of Windows filesystem, not a real error
if cfg!(windows)
&& e.kind() == std::io::ErrorKind::PermissionDenied
{
// Log warning but don't fail - flush() already ensured data reaches OS buffers
eprintln!(
"Warning: Windows file sync limitation encountered: {}",
e
);
Ok(())
} else {
// On other platforms or different error types, this is a real failure
Err(format!("Failed to sync file to disk: {e}"))
}
}
}
}
Err(e) => Err(format!("Failed to flush file: {e}")),
}
Expand Down Expand Up @@ -481,9 +499,25 @@ impl IoClient {
// Flush the file before closing to ensure all data is written to disk
match file.flush().await {
Ok(_) => {
// Try to sync to disk for durability, but don't fail if it's not supported
let _ = file.sync_all().await;
Ok(())
// Sync to disk for durability
match file.sync_all().await {
Ok(_) => Ok(()),
Err(e) => {
// On Windows, sync_all can fail with "Access denied" in concurrent scenarios
// This is often a limitation of Windows filesystem, not a real error
if cfg!(windows) && e.kind() == std::io::ErrorKind::PermissionDenied {
// Log warning but don't fail - flush() already ensured data reaches OS buffers
eprintln!(
"Warning: Windows file sync limitation encountered: {}",
e
);
Ok(())
} else {
// On other platforms or different error types, this is a real failure
Err(format!("Failed to sync file during close: {e}"))
}
}
}
}
Err(e) => Err(format!("Failed to flush file during close: {e}")),
}
Expand All @@ -507,9 +541,27 @@ impl IoClient {
// Flush the data to ensure it's written to disk
match file.flush().await {
Ok(_) => {
// Try to sync to disk for durability, but don't fail if it's not supported
let _ = file.sync_all().await;
Ok(())
// Sync to disk for durability
match file.sync_all().await {
Ok(_) => Ok(()),
Err(e) => {
// On Windows, sync_all can fail with "Access denied" in concurrent scenarios
// This is often a limitation of Windows filesystem, not a real error
if cfg!(windows)
&& e.kind() == std::io::ErrorKind::PermissionDenied
{
// Log warning but don't fail - flush() already ensured data reaches OS buffers
eprintln!(
"Warning: Windows file sync limitation encountered: {}",
e
);
Ok(())
} else {
// On other platforms or different error types, this is a real failure
Err(format!("Failed to sync appended data to disk: {e}"))
}
}
}
}
Err(e) => Err(format!("Failed to flush appended data: {e}")),
}
Expand Down
Loading