Skip to content

Commit 4956c92

Browse files
Copilotsawka
andauthored
Make Wave home config writes atomic and serialized to avoid watcher partial reads (#2945)
`WriteWaveHomeConfigFile()` previously used direct `os.WriteFile`, which can expose truncation/partial-write states to the JSON file watcher. This change switches config persistence to temp-file + rename semantics and serializes writes through a single process-wide lock for config file writes. - **Atomic file write helper** - Added `AtomicWriteFile()` in `pkg/util/fileutil/fileutil.go`. - Writes to `<filename>.tmp` in the same directory, then renames to the target path. - Performs temp-file cleanup on error paths. - Introduced a shared suffix constant (`TempFileSuffix`) used by implementation/tests. - **Config write path update** - Updated `WriteWaveHomeConfigFile()` in `pkg/wconfig/settingsconfig.go` to: - Use a package-level mutex (`configWriteLock`) so only one config write runs at a time (across all config files). - Call `fileutil.AtomicWriteFile(...)` instead of direct `os.WriteFile(...)`. - **Focused coverage for atomic behavior** - Added `pkg/util/fileutil/fileutil_test.go` with tests for: - Successful atomic write (target file contains expected payload and no leftover `.tmp` file). - Rename-failure path cleanup (temp file is removed). ```go func WriteWaveHomeConfigFile(fileName string, m waveobj.MetaMapType) error { configWriteLock.Lock() defer configWriteLock.Unlock() fullFileName := filepath.Join(wavebase.GetWaveConfigDir(), fileName) barr, err := jsonMarshalConfigInOrder(m) if err != nil { return err } return fileutil.AtomicWriteFile(fullFileName, barr, 0644) } ``` <!-- START COPILOT CODING AGENT TIPS --> --- 🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. [Learn more about Advanced Security.](https://gh.io/cca-advanced-security) --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sawka <2722291+sawka@users.noreply.github.com>
1 parent e8ebe88 commit 4956c92

3 files changed

Lines changed: 67 additions & 57 deletions

File tree

pkg/util/fileutil/fileutil.go

Lines changed: 14 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,8 @@ import (
1414
"path/filepath"
1515
"regexp"
1616
"strings"
17-
"time"
1817

1918
"github.com/wavetermdev/waveterm/pkg/wavebase"
20-
"github.com/wavetermdev/waveterm/pkg/wshrpc"
2119
)
2220

2321
func FixPath(path string) (string, error) {
@@ -145,13 +143,21 @@ func DetectMimeTypeWithDirEnt(path string, dirEnt fs.DirEntry) string {
145143
return ""
146144
}
147145

148-
func AddMimeTypeToFileInfo(path string, fileInfo *wshrpc.FileInfo) {
149-
if fileInfo == nil {
150-
return
146+
func AtomicWriteFile(fileName string, data []byte, perm os.FileMode) error {
147+
tmpFileName := fileName + TempFileSuffix
148+
if err := os.WriteFile(tmpFileName, data, perm); err != nil {
149+
if removeErr := os.Remove(tmpFileName); removeErr != nil && !os.IsNotExist(removeErr) {
150+
return fmt.Errorf("failed to write temp file %q: %w (also failed to remove temp file: %v)", tmpFileName, err, removeErr)
151+
}
152+
return err
151153
}
152-
if fileInfo.MimeType == "" {
153-
fileInfo.MimeType = DetectMimeType(path, ToFsFileInfo(fileInfo), false)
154+
if err := os.Rename(tmpFileName, fileName); err != nil {
155+
if removeErr := os.Remove(tmpFileName); removeErr != nil && !os.IsNotExist(removeErr) {
156+
return fmt.Errorf("failed to rename temp file %q to %q: %w (also failed to remove temp file: %v)", tmpFileName, fileName, err, removeErr)
157+
}
158+
return err
154159
}
160+
return nil
155161
}
156162

157163
var (
@@ -203,56 +209,8 @@ func IsInitScriptPath(input string) bool {
203209
return true
204210
}
205211

206-
type FsFileInfo struct {
207-
NameInternal string
208-
ModeInternal os.FileMode
209-
SizeInternal int64
210-
ModTimeInternal int64
211-
IsDirInternal bool
212-
}
213-
214-
func (f FsFileInfo) Name() string {
215-
return f.NameInternal
216-
}
217-
218-
func (f FsFileInfo) Size() int64 {
219-
return f.SizeInternal
220-
}
221-
222-
func (f FsFileInfo) Mode() os.FileMode {
223-
return f.ModeInternal
224-
}
225-
226-
func (f FsFileInfo) ModTime() time.Time {
227-
return time.Unix(0, f.ModTimeInternal)
228-
}
229-
230-
func (f FsFileInfo) IsDir() bool {
231-
return f.IsDirInternal
232-
}
233-
234-
func (f FsFileInfo) Sys() interface{} {
235-
return nil
236-
}
237-
238-
var _ fs.FileInfo = FsFileInfo{}
239-
240-
// ToFsFileInfo converts wshrpc.FileInfo to FsFileInfo.
241-
// It panics if fi is nil.
242-
func ToFsFileInfo(fi *wshrpc.FileInfo) FsFileInfo {
243-
if fi == nil {
244-
panic("ToFsFileInfo: nil FileInfo")
245-
}
246-
return FsFileInfo{
247-
NameInternal: fi.Name,
248-
ModeInternal: fi.Mode,
249-
SizeInternal: fi.Size,
250-
ModTimeInternal: fi.ModTime,
251-
IsDirInternal: fi.IsDir,
252-
}
253-
}
254-
255212
const (
213+
TempFileSuffix = ".tmp"
256214
MaxEditFileSize = 5 * 1024 * 1024 // 5MB
257215
)
258216

pkg/util/fileutil/fileutil_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
package fileutil
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"testing"
7+
)
8+
9+
func TestAtomicWriteFile(t *testing.T) {
10+
tmpDir := t.TempDir()
11+
fileName := filepath.Join(tmpDir, "settings.json")
12+
13+
err := AtomicWriteFile(fileName, []byte(`{"key":"value"}`), 0644)
14+
if err != nil {
15+
t.Fatalf("AtomicWriteFile failed: %v", err)
16+
}
17+
18+
data, err := os.ReadFile(fileName)
19+
if err != nil {
20+
t.Fatalf("ReadFile failed: %v", err)
21+
}
22+
if string(data) != `{"key":"value"}` {
23+
t.Fatalf("unexpected file contents: %q", string(data))
24+
}
25+
if _, err := os.Stat(fileName + TempFileSuffix); !os.IsNotExist(err) {
26+
t.Fatalf("temporary file should not exist, stat err: %v", err)
27+
}
28+
}
29+
30+
func TestAtomicWriteFileRenameErrorCleansTempFile(t *testing.T) {
31+
tmpDir := t.TempDir()
32+
fileName := filepath.Join(tmpDir, "settings.json")
33+
34+
if err := os.Mkdir(fileName, 0755); err != nil {
35+
t.Fatalf("Mkdir failed: %v", err)
36+
}
37+
38+
err := AtomicWriteFile(fileName, []byte(`{"key":"value"}`), 0644)
39+
if err == nil {
40+
t.Fatalf("AtomicWriteFile expected error")
41+
}
42+
if _, statErr := os.Stat(fileName + TempFileSuffix); !os.IsNotExist(statErr) {
43+
t.Fatalf("temporary file should be removed on rename error, stat err: %v", statErr)
44+
}
45+
}

pkg/wconfig/settingsconfig.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ import (
1414
"reflect"
1515
"sort"
1616
"strings"
17+
"sync"
1718

19+
"github.com/wavetermdev/waveterm/pkg/util/fileutil"
1820
"github.com/wavetermdev/waveterm/pkg/util/utilfn"
1921
"github.com/wavetermdev/waveterm/pkg/wavebase"
2022
"github.com/wavetermdev/waveterm/pkg/waveobj"
@@ -25,6 +27,8 @@ const SettingsFile = "settings.json"
2527
const ConnectionsFile = "connections.json"
2628
const ProfilesFile = "profiles.json"
2729

30+
var configWriteLock sync.Mutex
31+
2832
const AnySchema = `
2933
{
3034
"type": "object",
@@ -502,13 +506,16 @@ func ReadWaveHomeConfigFile(fileName string) (waveobj.MetaMapType, []ConfigError
502506
}
503507

504508
func WriteWaveHomeConfigFile(fileName string, m waveobj.MetaMapType) error {
509+
configWriteLock.Lock()
510+
defer configWriteLock.Unlock()
511+
505512
configDirAbsPath := wavebase.GetWaveConfigDir()
506513
fullFileName := filepath.Join(configDirAbsPath, fileName)
507514
barr, err := jsonMarshalConfigInOrder(m)
508515
if err != nil {
509516
return err
510517
}
511-
return os.WriteFile(fullFileName, barr, 0644)
518+
return fileutil.AtomicWriteFile(fullFileName, barr, 0644)
512519
}
513520

514521
// simple merge that overwrites

0 commit comments

Comments
 (0)