fix: state.json の保存競合と非 atomic write を修正(#112) - #119
Conversation
`.codiva/state.json` を直接 writeFile していたため、書き込み中にプロセスが 死ぬと切れた JSON が残り、次回起動時に loadState が空状態へフォールバックして 復元可能なセッションを全部失う可能性があった。また debounce の非同期保存が 飛んでいる最中に最終 flush / 同期 flush が走ると、遅れて完了した古い書き込みが 最新状態を巻き戻す競合もあった。 - utils/state-store: 同一ディレクトリの temp へ書き、fsync → close → rename で 差し替える(同期版も同じ)。temp 名は `<path>.<pid>.<async|sync>.tmp` 固定で、 非同期の書き込みはパスごとに直列化する(同名 temp の同時オープンを防ぎ、 rename の順序を呼び出し順に一致させる)。書き込み失敗時は temp を掃除する。 - bootstrap/persist-controller: snapshot を「書き始めた時点」で読むよう直列化し、 スケジュール時点の古い状態が後から着地しないようにした。同期 flush は世代を 上げ、その最中に走っていた非同期書き込みは完了後に最新を書き直す。 - 保存順の逆転・途中書き込み・失敗時のロールバックをテストで固定。 - docs/ARCHITECTURE.md と .claude/rules/git-and-io.md に不変条件を追記。
| const generation = syncGeneration; | ||
| await saveState(snapshot(), statePath); | ||
| if (generation !== syncGeneration) { | ||
| await saveState(snapshot(), statePath); |
There was a problem hiding this comment.
この修復書き込み自体の途中で flushSync() がもう一度走ると、最新状態が再び巻き戻ります。例えば (1) generation=0 の async 保存開始、(2) sync flush で generation=1、(3) この行の修復用 saveState(snapshotA) 開始、(4) 状態が B へ進んで再度 sync flush(generation=2、B を保存)、(5) snapshotA の rename が後から完了、という順序では A が最後に残り、この後は世代を再確認しません。コメントにある「プロセスが生き残る経路でも順序を壊さない」という契約を満たすには、修復開始時の世代を控えて保存後にも再確認するループ(世代が安定するまで最新 snapshot を保存)にする必要があります。この順序を gate で固定した回帰テストも追加してください。
There was a problem hiding this comment.
ご指摘のとおりでした。修復を 1 回で打ち切っていたため、修復書き込みの最中に入った 2 度目の同期 flush を、その修復自身の rename が巻き戻します。
1011c45 で「最新 snapshot を保存 → 世代を再確認」を世代が安定するまで繰り返すループに変更しました。
let generation = syncGeneration;
for (;;) {
await saveState(snapshot(), statePath);
if (generation === syncGeneration) {
return;
}
generation = syncGeneration;
}各周回で「世代を控える → その後に snapshot() を読む」順序になっているので、書き込み中に入った flush は必ず次の周回で拾われます。無限ループにならないのは flushSync の呼び出し元がシグナルハンドラだけで直後に process.exit するためですが、その根拠はコメントに残しました。
回帰テスト(persist-controller.spec.ts の "repairs again when another synchronous flush lands during the repair write")で、ご指摘の順序を snapshot() を gate にして固定しています:
- async 保存が
stale1で開始(gen=0) - その最中に sync flush →
mid1を保存(gen=1) - 修復書き込みが
stale2で開始 - その最中にもう一度 sync flush →
mid2を保存(gen=2) - 修復書き込みの rename が後から着地
旧実装(修復 1 回)では最終的に stale2 が残って落ちること、新実装では 5 の後にもう一周して final が残ることを確認済みです。
There was a problem hiding this comment.
修正を確認しました。世代が安定するまで saveState(snapshot()) と世代確認を繰り返すため、修復書き込み中に 2 回目以降の flushSync() が入っても、その世代の最新 snapshot で再修復されます。追加テストも旧実装で stale2 が最後に残る順序を再現できており、指摘は解消しています。
|
レビューしました。atomic write と非同期保存の直列化は妥当ですが、修復用の2回目の非同期保存中に再度同期 flush が入ると、古い snapshot が最後に着地する競合が残っています。該当行にタイムラインと修正案をコメントしました。この1点をブロッキング指摘とします。 |
修復用の 2 回目の非同期保存を 1 回だけ行っていたため、その最中にもう一度 同期 flush が走ると(gen=1 で修復開始 → 状態が B へ進み gen=2 で B を保存 → 修復書き込みの rename が後着)、古い snapshot が最後に残っていた。 - 世代が安定するまで「最新 snapshot を保存 → 世代を再確認」を繰り返すループにした。 終了は保証される(flushSync の呼び出し元はシグナルハンドラのみで直後に exit する)。 - 指摘の順序(async 保存 → sync flush → 修復書き込み → sync flush → 修復の rename が後着)を gate で固定した回帰テストを追加。旧実装では `stale2` が残り落ちることを確認済み。
概要
#112の対応。セッション復元用.codiva/state.jsonの保存にあった 2 つの問題を修正しました。writeFileしていたため、書き込み中にプロセスが死ぬと切れた JSON が残り、次回起動でloadStateが破損扱いしてemptyPersistedState()に落ちる(= 復元可能なセッションを全部失う。worktree は残るが codiva から辿れない)。flushAsync()/ シグナル時flushSync()が最新状態を書くと、遅れて完了した古い書き込みが最新状態を巻き戻す。変更内容
src/utils/state-store.tssaveStateSyncも同じ)。rename は POSIX で atomic なので、途中で死んでもstate.jsonは「前回の完全な内容」か「新しい完全な内容」のどちらかになる。<path>.<pid>.<async|sync>.tmp固定。非同期の書き込みはパスごとに直列化することで同名 temp を 2 本同時に開かないようにし、同時に rename の順序が呼び出し順と一致する(= 古い保存が新しい保存を上書きしない)。sync 側は別名なので in-flight の async と衝突しない(そちらの rename はprocess.exitにより実行されないので巻き戻しも起きない)。state.jsonはそのまま残す。src/bootstrap/persist-controller.tssnapshot()は「書き始めた時点」で読む(スケジュール時点で固めない)。これにより、遅れて着地する書き込みが古い状態を書くことが構造的に起きなくなる。flushSyncは世代カウンタを上げ、その最中に走っていた非同期書き込みは完了後に最新 snapshot を書き直す(実際の kill 経路では直後にprocess.exitするが、プロセスが生き残る経路でも順序が壊れないようにするため)。テスト
state-store.spec.ts: 同時保存で最後の呼び出しが勝つ / temp を残さない / 書き込み失敗時に前回のファイルが無傷であること。persist-controller.spec.ts(新規): スケジュール時点ではなく書き込み時点の snapshot を書く / 書き込み中に同期 flush が入っても最新へ復旧する / 失敗後も保存を続ける /flushSyncが投げない。ドキュメント
docs/ARCHITECTURE.md・.claude/rules/git-and-io.mdに「state.json は直接書かない(temp → fsync → rename)」「snapshot は書き始めた時点で読む」を不変条件として追記。対応案のうち見送ったもの
state.json.bakの世代管理を足す価値が薄いと判断しました(復元不能な壊れ方をするのは、そもそも rename が atomic でないファイルシステムの場合のみ)。必要なら別 issue で。テスト計画
kill -TERM→ 再起動して一覧が復元されることCloses #112