Skip to content

Commit 0201c60

Browse files
committed
[POSIX] Clean up temp files in error cases
1 parent 20980b0 commit 0201c60

2 files changed

Lines changed: 48 additions & 4 deletions

File tree

storage/posix/file_ops.go

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -151,10 +151,19 @@ func overwrite(name string, d []byte) error {
151151
if err != nil {
152152
return fmt.Errorf("failed to create temp file: %w", err)
153153
}
154+
success := false
155+
defer func() {
156+
if !success {
157+
if err := os.Remove(tmpName); err != nil {
158+
slog.WarnContext(context.Background(), "Failed to remove temporary file", slog.String("tmpname", tmpName), slog.Any("error", err))
159+
}
160+
}
161+
}()
154162

155163
if err := os.Rename(tmpName, name); err != nil {
156164
return fmt.Errorf("failed to rename temporary file to target %q: %w", name, err)
157165
}
166+
success = true
158167
return nil
159168
})
160169
}
@@ -186,17 +195,31 @@ func createTemp(prefix string, d []byte) (name string, err error) {
186195
}
187196
}
188197

198+
tmpName := name
199+
success := false
200+
defer func() {
201+
if !success {
202+
if err := os.Remove(tmpName); err != nil {
203+
slog.WarnContext(context.Background(), "Failed to remove temporary file", slog.String("tmpname", tmpName), slog.Any("error", err))
204+
}
205+
name = ""
206+
}
207+
}()
189208
defer func() {
190-
if errC := f.Close(); errC != nil && err == nil {
191-
err = errC
209+
if errC := f.Close(); errC != nil {
210+
if err == nil {
211+
err = errC
212+
}
213+
success = false
192214
}
193215
}()
194216

195-
if n, err := f.Write(d); err != nil {
196-
return "", fmt.Errorf("failed to write to temporary file %q: %w", name, err)
217+
if n, writeErr := f.Write(d); writeErr != nil {
218+
return "", fmt.Errorf("failed to write to temporary file %q: %w", name, writeErr)
197219
} else if l := len(d); n < l {
198220
return "", fmt.Errorf("short write on %q, %d < %d", name, n, l)
199221
}
200222

223+
success = true
201224
return name, nil
202225
}

storage/posix/files_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -655,3 +655,24 @@ func TestFlockLockingCorrectness(t *testing.T) {
655655
t.Fatalf("expected output file to contain %q, got %q (serialization failed)", expected, string(content))
656656
}
657657
}
658+
659+
func TestOverwrite_CleanupOnRenameFailure(t *testing.T) {
660+
tmpDir := t.TempDir()
661+
targetDir := filepath.Join(tmpDir, "target_dir")
662+
if err := os.Mkdir(targetDir, 0755); err != nil {
663+
t.Fatal(err)
664+
}
665+
err := overwrite(targetDir, []byte("some data"))
666+
if err == nil {
667+
t.Fatal("expected overwrite to fail when target is a directory")
668+
}
669+
entries, err := os.ReadDir(tmpDir)
670+
if err != nil {
671+
t.Fatal(err)
672+
}
673+
for _, entry := range entries {
674+
if entry.Name() != "target_dir" {
675+
t.Errorf("found unexpected file/dir after failure: %s", entry.Name())
676+
}
677+
}
678+
}

0 commit comments

Comments
 (0)