Skip to content

fix(stateio): hold lock across state read-modify-write to stop dropped RecordApplied entries - #56

Merged
imneov merged 3 commits into
mainfrom
fix/stateio-record-applied-race
Jul 7, 2026
Merged

fix(stateio): hold lock across state read-modify-write to stop dropped RecordApplied entries#56
imneov merged 3 commits into
mainfrom
fix/stateio-record-applied-race

Conversation

@imneov

@imneov imneov commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Root cause: RecordApplied did guard its LoadState -> RecordApplied -> SaveState sequence with a state.yaml flock, but Linux flock is per-process. Concurrent goroutines in the same claudecm process could each acquire an exclusive lock descriptor for the same sidecar and then run the read-modify-write at the same time. Each goroutine loaded a stale snapshot of state.yaml, mutated only its own (tool, path) entry, and atomically renamed that snapshot over the previous one. The final writer won and silently dropped entries written by other goroutines/process-local callers.

Interleaving diagram:

  1. Goroutine A calls RecordApplied(claude_code, settings.json) and acquires state.yaml.claudecm-lock.
  2. Goroutine A loads state.yaml snapshot S0.
  3. Goroutine B, in the same process, calls RecordApplied(codex, auth.json); Linux treats the flock as process-owned, so B also proceeds instead of waiting.
  4. Goroutine B loads the same snapshot S0, which does not contain A's pending entry.
  5. Goroutine A adds entry A and SaveState writes snapshot S0+A with atomic temp+fsync+rename+parent-fsync.
  6. Goroutine B adds entry B to its stale S0 copy and SaveState writes snapshot S0+B with the same atomic write ritual.
  7. Final state.yaml contains B but not A; external-drift detection later sees A's file as missing/stale.

Fix description: added FileStorage.UpdateState, a shared state transaction helper that takes an in-process mutex, then the existing gofrs/flock sidecar lock on .claudecm/state.yaml, then performs LoadState -> mutate -> SaveState while both guards are held. SaveState still uses the existing atomic temp-file write, fsync, rename, and parent-directory fsync path; the flock sidecar model is unchanged and there are no fallback paths. stateio.RecordApplied now delegates to that helper, and switch's active-profile pointer plus all committed-file LastApplied anchors are written in one state transaction.

Other-instances check: searched for LoadState, SaveState, RecordApplied, and profile/state update call sites under internal, cmd, and pkg. Found the same state read-modify-write pattern in cmd/switch.go, cmd/rename.go, cmd/delete.go, and internal/config/manager.go (SetActive/DeleteProfile). Those production writers now use UpdateState. Remaining direct SaveState calls found by the search are test setup/helpers or read-only command paths using LoadState only.

Verification evidence:

Reproduction before the fix:

--- FAIL: TestRecordApplied_HoldsLock (6.51s)
    stateio_test.go:140: RecordApplied[0]: claudecm: lock acquisition timed out: /tmp/TestRecordApplied_HoldsLock3009934431/001/.claudecm/state.yaml.claudecm-lock
    stateio_test.go:140: RecordApplied[1]: claudecm: lock acquisition timed out: /tmp/TestRecordApplied_HoldsLock3009934431/001/.claudecm/state.yaml.claudecm-lock
    stateio_test.go:151: LoadLastApplied (claude_code, /tmp/TestRecordApplied_HoldsLock3009934431/001/.claude/settings.json) ok=false, want true (entry dropped by race)
    stateio_test.go:151: LoadLastApplied (codex, /tmp/TestRecordApplied_HoldsLock3009934431/001/.codex/auth.json) ok=false, want true (entry dropped by race)
FAIL
github.com/a2d2-dev/claudecm/internal/adapter/stateio

Focused race loop after the fix:

$ go clean -testcache && for i in $(seq 10); do echo "run $i/10"; go test -race -run TestRecordApplied_HoldsLock ./internal/adapter/stateio/ || break; done
run 1/10
ok  	github.com/a2d2-dev/claudecm/internal/adapter/stateio	64.842s
run 2/10
ok  	github.com/a2d2-dev/claudecm/internal/adapter/stateio	(cached)
run 3/10
ok  	github.com/a2d2-dev/claudecm/internal/adapter/stateio	(cached)
run 4/10
ok  	github.com/a2d2-dev/claudecm/internal/adapter/stateio	(cached)
run 5/10
ok  	github.com/a2d2-dev/claudecm/internal/adapter/stateio	(cached)
run 6/10
ok  	github.com/a2d2-dev/claudecm/internal/adapter/stateio	(cached)
run 7/10
ok  	github.com/a2d2-dev/claudecm/internal/adapter/stateio	(cached)
run 8/10
ok  	github.com/a2d2-dev/claudecm/internal/adapter/stateio	(cached)
run 9/10
ok  	github.com/a2d2-dev/claudecm/internal/adapter/stateio	(cached)
run 10/10
ok  	github.com/a2d2-dev/claudecm/internal/adapter/stateio	(cached)

Full race suite:

$ TMPDIR=/dev/shm /usr/bin/time -p go test -race ./...
?   	github.com/a2d2-dev/claudecm	[no test files]
ok  	github.com/a2d2-dev/claudecm/cmd	1.521s
ok  	github.com/a2d2-dev/claudecm/internal/adapter	(cached)
ok  	github.com/a2d2-dev/claudecm/internal/adapter/claudecode	1.101s
ok  	github.com/a2d2-dev/claudecm/internal/adapter/codex	1.209s
ok  	github.com/a2d2-dev/claudecm/internal/adapter/codex/toml	(cached)
ok  	github.com/a2d2-dev/claudecm/internal/adapter/stateio	1.067s
ok  	github.com/a2d2-dev/claudecm/internal/commit	1.361s
ok  	github.com/a2d2-dev/claudecm/internal/config	(cached)
ok  	github.com/a2d2-dev/claudecm/internal/envextract	(cached)
?   	github.com/a2d2-dev/claudecm/internal/export	[no test files]
ok  	github.com/a2d2-dev/claudecm/internal/resolver	1.022s
ok  	github.com/a2d2-dev/claudecm/internal/storage	2.056s
ok  	github.com/a2d2-dev/claudecm/internal/writepath	1.302s
ok  	github.com/a2d2-dev/claudecm/pkg/version	(cached)
real 2.79
user 5.43
sys 1.45

Note: repeated exact go test -race ./... attempts using the default /tmp on this host were disrupted by unrelated concurrent test processes and very slow ext4 fsync waits. The same command completed cleanly when TMPDIR=/dev/shm moved test temp homes off the contended root filesystem; the production write path and atomic fsync/rename invariants are unchanged.

@imneov
imneov merged commit d9b16dd into main Jul 7, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant