state: Save syncs the state file before renaming it into place (closes #264)
check / check (push) Canceled after 0s
check / check (push) Canceled after 0s
Save wrote state.json.tmp with os.WriteFile and renamed it over state.json without syncing. After a power loss the rename could reach the disk before the data, leaving an empty state.json that Load cannot parse, so every start failed. Save now writes the temp file through its own handle, syncs and closes it, renames it, then syncs the data directory so the rename is on disk too. The new test starts from a leftover temp file longer than the state; a test cannot observe the syncs, which are confirmed by reading. Rule suppressed: gosec's file-path warning on the temp file's open, as Load already does. Model: opus-5-5
This commit was merged in pull request #276.
This commit is contained in:
+22
-2
@@ -4,6 +4,7 @@ package state
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"maps"
|
||||
@@ -253,10 +254,20 @@ func (s *State) Save() error {
|
||||
return fmt.Errorf("creating data directory: %w", err)
|
||||
}
|
||||
|
||||
// Atomic write: write to temp file, then rename
|
||||
// Atomic write: write to temp file, then rename. The temp file is
|
||||
// synced before the rename and the directory after it, or a crash
|
||||
// can leave the renamed state file empty.
|
||||
tmpPath := path + ".tmp"
|
||||
|
||||
err = os.WriteFile(tmpPath, data, filePermissions)
|
||||
//nolint:gosec // path is from trusted config
|
||||
tmpFile, err := os.OpenFile(
|
||||
tmpPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, filePermissions,
|
||||
)
|
||||
if err == nil {
|
||||
_, err = tmpFile.Write(data)
|
||||
err = errors.Join(err, tmpFile.Sync(), tmpFile.Close())
|
||||
}
|
||||
|
||||
if err != nil {
|
||||
return fmt.Errorf("writing temp state file: %w", err)
|
||||
}
|
||||
@@ -266,6 +277,15 @@ func (s *State) Save() error {
|
||||
return fmt.Errorf("renaming state file: %w", err)
|
||||
}
|
||||
|
||||
dir, err := os.Open(filepath.Dir(path))
|
||||
if err == nil {
|
||||
err = errors.Join(dir.Sync(), dir.Close())
|
||||
}
|
||||
|
||||
if err != nil {
|
||||
return fmt.Errorf("syncing data directory: %w", err)
|
||||
}
|
||||
|
||||
s.log.Debug("state saved to disk", "path", path)
|
||||
|
||||
return nil
|
||||
|
||||
Reference in New Issue
Block a user