CRITICAL: Data race in State.Save() — mutates snapshot under RLock #17

New Issue

clawbot · 2026-02-20T13:49:41+01:00

clawbot commented

2026-02-20 13:49:41 +01:00

Bug

State.Save() in internal/state/state.go acquires s.mu.RLock() but then mutates the snapshot:

func (s *State) Save() error {
    s.mu.RLock()
    defer s.mu.RUnlock()
    s.snapshot.LastUpdated = time.Now().UTC()  // WRITE under RLock!
    ...
}

RLock allows concurrent readers, but this is a write operation. If Save() runs concurrently with GetSnapshot() or another Save(), this is a data race.

Impact

Race condition that can cause corrupted LastUpdated timestamps or panic under the race detector. In practice, Save() is called from the watcher goroutine and GetSnapshot() could be called from HTTP handlers concurrently.

Fix

Either:

Change Save() to use s.mu.Lock() (full write lock), or
Compute LastUpdated before acquiring the lock and pass it in, keeping RLock for the marshal+write portion.

## Bug `State.Save()` in `internal/state/state.go` acquires `s.mu.RLock()` but then mutates the snapshot: ```go func (s *State) Save() error { s.mu.RLock() defer s.mu.RUnlock() s.snapshot.LastUpdated = time.Now().UTC() // WRITE under RLock! ... } ``` `RLock` allows concurrent readers, but this is a write operation. If `Save()` runs concurrently with `GetSnapshot()` or another `Save()`, this is a data race. ## Impact Race condition that can cause corrupted `LastUpdated` timestamps or panic under the race detector. In practice, `Save()` is called from the watcher goroutine and `GetSnapshot()` could be called from HTTP handlers concurrently. ## Fix Either: 1. Change `Save()` to use `s.mu.Lock()` (full write lock), or 2. Compute `LastUpdated` before acquiring the lock and pass it in, keeping `RLock` for the marshal+write portion.

Sign in to join this conversation.