refactor(pkg/utils): improve WriteFileAtomic with stronger durability guarantees
This commit is contained in:
parent
c56fcedcb1
commit
4aed3591e7
2 changed files with 44 additions and 16 deletions
|
|
@ -339,9 +339,9 @@ func (r *sandboxFs) WriteFile(path string, data []byte) error {
|
||||||
|
|
||||||
// Use atomic write pattern with explicit sync for flash storage reliability.
|
// Use atomic write pattern with explicit sync for flash storage reliability.
|
||||||
// Using 0o600 (owner read/write only) for secure default permissions.
|
// Using 0o600 (owner read/write only) for secure default permissions.
|
||||||
tmpRelPath := fmt.Sprintf(".tmp-%d.tmp", time.Now().UnixNano())
|
tmpRelPath := fmt.Sprintf(".tmp-%d-%d", os.Getpid(), time.Now().UnixNano())
|
||||||
|
|
||||||
tmpFile, err := root.OpenFile(tmpRelPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600)
|
tmpFile, err := root.OpenFile(tmpRelPath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
root.Remove(tmpRelPath)
|
root.Remove(tmpRelPath)
|
||||||
return fmt.Errorf("failed to open temp file: %w", err)
|
return fmt.Errorf("failed to open temp file: %w", err)
|
||||||
|
|
@ -370,6 +370,13 @@ func (r *sandboxFs) WriteFile(path string, data []byte) error {
|
||||||
root.Remove(tmpRelPath)
|
root.Remove(tmpRelPath)
|
||||||
return fmt.Errorf("failed to rename temp file over target: %w", err)
|
return fmt.Errorf("failed to rename temp file over target: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Sync directory to ensure rename is durable
|
||||||
|
if dirFile, err := root.Open("."); err == nil {
|
||||||
|
_ = dirFile.Sync()
|
||||||
|
dirFile.Close()
|
||||||
|
}
|
||||||
|
|
||||||
return nil
|
return nil
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -10,20 +10,28 @@ import (
|
||||||
"fmt"
|
"fmt"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"time"
|
||||||
)
|
)
|
||||||
|
|
||||||
// WriteFileAtomic atomically writes data to a file using a temp file + rename pattern.
|
// WriteFileAtomic atomically writes data to a file using a temp file + rename pattern.
|
||||||
//
|
//
|
||||||
// This guarantees that the target file is either:
|
// This guarantees that the target file is either:
|
||||||
// - Completely written with the new data
|
// - Completely written with the new data
|
||||||
// - Unchanged (if write fails or power loss during write)
|
// - Unchanged (if any step fails before rename)
|
||||||
//
|
//
|
||||||
// The function:
|
// The function:
|
||||||
// 1. Creates a temp file in the same directory
|
// 1. Creates a temp file in the same directory (original untouched)
|
||||||
// 2. Writes data to temp file
|
// 2. Writes data to temp file
|
||||||
// 3. Syncs to disk (critical for SD cards/flash storage)
|
// 3. Syncs data to disk (critical for SD cards/flash storage)
|
||||||
// 4. Sets file permissions
|
// 4. Sets file permissions
|
||||||
// 5. Atomically renames temp file to target path
|
// 5. Syncs directory metadata (ensures rename is durable)
|
||||||
|
// 6. Atomically renames temp file to target path
|
||||||
|
//
|
||||||
|
// Safety guarantees:
|
||||||
|
// - Original file is NEVER modified until successful rename
|
||||||
|
// - Temp file is always cleaned up on error
|
||||||
|
// - Data is flushed to physical storage before rename
|
||||||
|
// - Directory entry is synced to prevent orphaned inodes
|
||||||
//
|
//
|
||||||
// Parameters:
|
// Parameters:
|
||||||
// - path: Target file path
|
// - path: Target file path
|
||||||
|
|
@ -47,51 +55,64 @@ func WriteFileAtomic(path string, data []byte, perm os.FileMode) error {
|
||||||
}
|
}
|
||||||
|
|
||||||
// Create temp file in the same directory (ensures atomic rename works)
|
// Create temp file in the same directory (ensures atomic rename works)
|
||||||
tmpFile, err := os.CreateTemp(dir, ".tmp-*.tmp")
|
// Using a hidden prefix (.tmp-) to avoid issues with some tools
|
||||||
|
tmpFile, err := os.OpenFile(
|
||||||
|
filepath.Join(dir, fmt.Sprintf(".tmp-%d-%d", os.Getpid(), time.Now().UnixNano())),
|
||||||
|
os.O_WRONLY|os.O_CREATE|os.O_EXCL,
|
||||||
|
perm,
|
||||||
|
)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("failed to create temp file: %w", err)
|
return fmt.Errorf("failed to create temp file: %w", err)
|
||||||
}
|
}
|
||||||
tmpPath := tmpFile.Name()
|
|
||||||
|
|
||||||
// Cleanup on error: ensure temp file is removed if anything fails
|
tmpPath := tmpFile.Name()
|
||||||
cleanup := true
|
cleanup := true
|
||||||
|
|
||||||
defer func() {
|
defer func() {
|
||||||
if cleanup {
|
if cleanup {
|
||||||
|
tmpFile.Close()
|
||||||
_ = os.Remove(tmpPath)
|
_ = os.Remove(tmpPath)
|
||||||
}
|
}
|
||||||
}()
|
}()
|
||||||
|
|
||||||
// Write data to temp file
|
// Write data to temp file
|
||||||
|
// Note: Original file is untouched at this point
|
||||||
if _, err := tmpFile.Write(data); err != nil {
|
if _, err := tmpFile.Write(data); err != nil {
|
||||||
tmpFile.Close()
|
|
||||||
return fmt.Errorf("failed to write temp file: %w", err)
|
return fmt.Errorf("failed to write temp file: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// CRITICAL: Force sync to storage medium before rename.
|
// CRITICAL: Force sync to storage medium before any other operations.
|
||||||
// This ensures data is physically written to disk, not just cached.
|
// This ensures data is physically written to disk, not just cached.
|
||||||
// Essential for SD cards, eMMC, and other flash storage on edge devices.
|
// Essential for SD cards, eMMC, and other flash storage on edge devices.
|
||||||
if err := tmpFile.Sync(); err != nil {
|
if err := tmpFile.Sync(); err != nil {
|
||||||
tmpFile.Close()
|
|
||||||
return fmt.Errorf("failed to sync temp file: %w", err)
|
return fmt.Errorf("failed to sync temp file: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Set file permissions
|
// Set file permissions before closing
|
||||||
if err := tmpFile.Chmod(perm); err != nil {
|
if err := tmpFile.Chmod(perm); err != nil {
|
||||||
tmpFile.Close()
|
|
||||||
return fmt.Errorf("failed to set permissions: %w", err)
|
return fmt.Errorf("failed to set permissions: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Close file before rename
|
// Close file before rename (required on Windows)
|
||||||
if err := tmpFile.Close(); err != nil {
|
if err := tmpFile.Close(); err != nil {
|
||||||
return fmt.Errorf("failed to close temp file: %w", err)
|
return fmt.Errorf("failed to close temp file: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Atomic rename: temp file becomes the target
|
// Atomic rename: temp file becomes the target
|
||||||
|
// On POSIX: rename() is atomic
|
||||||
|
// On Windows: Rename() is atomic for files
|
||||||
if err := os.Rename(tmpPath, path); err != nil {
|
if err := os.Rename(tmpPath, path); err != nil {
|
||||||
return fmt.Errorf("failed to rename temp file: %w", err)
|
return fmt.Errorf("failed to rename temp file: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Success: skip cleanup
|
// Sync directory to ensure rename is durable
|
||||||
|
// This prevents the renamed file from disappearing after a crash
|
||||||
|
if dirFile, err := os.Open(dir); err == nil {
|
||||||
|
_ = dirFile.Sync()
|
||||||
|
dirFile.Close()
|
||||||
|
}
|
||||||
|
|
||||||
|
// Success: skip cleanup (file was renamed, no temp to remove)
|
||||||
cleanup = false
|
cleanup = false
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue