Improve reliability in two critical paths: - make config saves atomic by writing to a temp file in the target directory, syncing it, and renaming it into place so crashes cannot leave a half-written JSON config behind - serialize runtime state transitions with a dedicated mutex so concurrent state updates from run() and writerLoop() cannot double-record transitions or increment counters twice Also remove an unreachable nil-check after cloneFrame() to keep the engine loop honest and easier to reason about.main
| @@ -147,6 +147,7 @@ type Engine struct { | |||||
| startedAt time.Time | startedAt time.Time | ||||
| wg sync.WaitGroup | wg sync.WaitGroup | ||||
| runtimeState atomic.Value | runtimeState atomic.Value | ||||
| stateMu sync.Mutex // guards setRuntimeState check-then-store (NEW-2 fix) | |||||
| chunksProduced atomic.Uint64 | chunksProduced atomic.Uint64 | ||||
| totalSamples atomic.Uint64 | totalSamples atomic.Uint64 | ||||
| @@ -571,13 +572,9 @@ func (e *Engine) run(ctx context.Context) { | |||||
| updateMaxDuration(&e.maxGenerateNs, genDur) | updateMaxDuration(&e.maxGenerateNs, genDur) | ||||
| updateMaxDuration(&e.maxUpsampleNs, upDur) | updateMaxDuration(&e.maxUpsampleNs, upDur) | ||||
| // cloneFrame never returns nil when src is non-nil (NEW-3: dead nil check removed) | |||||
| enqueued := cloneFrame(frame) | enqueued := cloneFrame(frame) | ||||
| enqueued.EnqueuedAt = time.Now() | enqueued.EnqueuedAt = time.Now() | ||||
| if enqueued == nil { | |||||
| e.lastError.Store("engine: frame clone failed") | |||||
| e.underruns.Add(1) | |||||
| continue | |||||
| } | |||||
| if err := e.frameQueue.Push(ctx, enqueued); err != nil { | if err := e.frameQueue.Push(ctx, enqueued); err != nil { | ||||
| if ctx.Err() != nil { | if ctx.Err() != nil { | ||||
| @@ -699,6 +696,11 @@ func cloneFrame(src *output.CompositeFrame) *output.CompositeFrame { | |||||
| } | } | ||||
| func (e *Engine) setRuntimeState(state RuntimeState) { | func (e *Engine) setRuntimeState(state RuntimeState) { | ||||
| // NEW-2 fix: hold stateMu so that concurrent calls from run() and | |||||
| // writerLoop() cannot both see prev != state and both record a | |||||
| // spurious duplicate transition. | |||||
| e.stateMu.Lock() | |||||
| defer e.stateMu.Unlock() | |||||
| now := time.Now() | now := time.Now() | ||||
| prev := e.currentRuntimeState() | prev := e.currentRuntimeState() | ||||
| if prev != state { | if prev != state { | ||||
| @@ -4,6 +4,7 @@ import ( | |||||
| "encoding/json" | "encoding/json" | ||||
| "fmt" | "fmt" | ||||
| "os" | "os" | ||||
| "path/filepath" | |||||
| "strconv" | "strconv" | ||||
| "strings" | "strings" | ||||
| ) | ) | ||||
| @@ -250,7 +251,34 @@ func Save(path string, cfg Config) error { | |||||
| return err | return err | ||||
| } | } | ||||
| data = append(data, '\n') | data = append(data, '\n') | ||||
| return os.WriteFile(path, data, 0o644) | |||||
| // NEW-1 fix: write to a temp file in the same directory, then rename atomically. | |||||
| // A direct os.WriteFile on the target leaves a corrupt file if the process | |||||
| // crashes mid-write. os.Rename is atomic on POSIX filesystems. | |||||
| dir := filepath.Dir(path) | |||||
| tmp, err := os.CreateTemp(dir, ".fmrtx-config-*.json.tmp") | |||||
| if err != nil { | |||||
| return fmt.Errorf("config save: create temp: %w", err) | |||||
| } | |||||
| tmpPath := tmp.Name() | |||||
| if _, err := tmp.Write(data); err != nil { | |||||
| _ = tmp.Close() | |||||
| _ = os.Remove(tmpPath) | |||||
| return fmt.Errorf("config save: write temp: %w", err) | |||||
| } | |||||
| if err := tmp.Sync(); err != nil { | |||||
| _ = tmp.Close() | |||||
| _ = os.Remove(tmpPath) | |||||
| return fmt.Errorf("config save: sync temp: %w", err) | |||||
| } | |||||
| if err := tmp.Close(); err != nil { | |||||
| _ = os.Remove(tmpPath) | |||||
| return fmt.Errorf("config save: close temp: %w", err) | |||||
| } | |||||
| if err := os.Rename(tmpPath, path); err != nil { | |||||
| _ = os.Remove(tmpPath) | |||||
| return fmt.Errorf("config save: rename: %w", err) | |||||
| } | |||||
| return nil | |||||
| } | } | ||||
| func (c Config) Validate() error { | func (c Config) Validate() error { | ||||