fix(lib-jj-helpers): pipeline_lock reclaim の cache 済み content 再奪取レースを修正 - #344
Merged
Conversation
PR #342 CI (windows-latest, 2 コア runner) で concurrent_takeover_with_orphaned_sentinel_single_winner が「得た数: 2」で失敗した 根本原因の修正。出遅れスレッドが除去前に読んだ stale sentinel content を握ったまま reclaim に入ると、勝者が finish_reclaim 完了時に reclaim marker を除去した後で content 由来の同一 gate path を再作成でき、勝者の fresh sentinel を無条件除去して 2 本目の takeover 実行権を得ていた (勝者の rename 前に lock を読めば双方 Acquired)。 2 層で塞ぐ: - finish_reclaim: sentinel が gate を正当化した stale content のままかを検証してから 除去する。不一致 / 消失 / 読取失敗は一切触れず Busy (検証済み: 修正前に決定論 テストが red、修正後 green)。 - reclaim marker を takeover 完了まで保持する (SentinelGate::Acquired が marker path を 運び、takeover_stale_lock が sentinel と共に除去)。takeover 実行中の同一 gate 再作成を構造的に排除。 検証: 決定論再現テスト (red→green) + marker 残留 leak guard + 高競合 stress (32 threads x 400 rounds、opt-in ignored) + 既存 concurrency guard 全 pass + workspace 全 test pass + clippy -D warnings clean。 verify-concurrency-by-observation (計測で確認、推論に頼らない) に従い、 部分書き込み仮説は 91.8 万回読取の実測 (partial=0) で棄却してから本経路を特定した。
📝 WalkthroughWalkthroughstale sentinel の reclaim 前に内容を検証し、競合時は Changespipeline_lock reclaim
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SentinelGate
participant finish_reclaim
participant Sentinel
participant ReclaimMarker
SentinelGate->>finish_reclaim: stale content と marker path を渡す
finish_reclaim->>Sentinel: 現在の内容を検証する
finish_reclaim-->>SentinelGate: 一致しない場合は Busy
finish_reclaim->>Sentinel: 一致時に sentinel を再作成する
finish_reclaim->>ReclaimMarker: takeover 完了まで marker を保持する
SentinelGate->>ReclaimMarker: takeover 完了後に削除する
SentinelGate->>Sentinel: takeover 完了後に削除する
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Verdict: approved (findings 0 件のため .takt/facets/instructions/analyze-coderabbit.md の Verdict Rules 「findings array が空なら approved」に該当。ただし CodeRabbit レビュー自体がまだ処理中で完了していない点に留意 — 完了後に findings が出れば次回発火時に再分析される) Applicable Findings (Critical / High / Major)該当なし (レビュー指摘 0 件) Applicable Findings (Medium 以下)該当なし Filtered (not applicable)該当なし Diff 概要 (レビュー指摘が無いため軽量サマリー)
次のアクション
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib-jj-helpers/src/pipeline_lock.rs (1)
179-186: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuemarker 除去のエラーを sentinel と同じ方針でログしてください。
Line 179-183 は sentinel 除去の失敗を NotFound 以外でログします。Line 184-186 は marker 除去の失敗を完全に無視します。marker が残留すると、同じ stale content を持つ後続スレッドは gate を取得できず
Busyに倒れます。SENTINEL_STALE_SECS経過後にreap_orphaned_reclaim_markerが回収するため自己修復しますが、その間の挙動の原因が観測できません。除去順序 (sentinel → marker) 自体は正しいため、変更はログのみで十分です。♻️ ログ方針を揃える差分
if let Some(marker) = reclaim_marker { - let _ = std::fs::remove_file(&marker); + if let Err(e) = std::fs::remove_file(&marker) { + if e.kind() != std::io::ErrorKind::NotFound { + eprintln!("[pipeline-lock] reclaim marker の除去に失敗 (継続): {}", e); + } + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib-jj-helpers/src/pipeline_lock.rs` around lines 179 - 186, Update the reclaim_marker removal in the takeover cleanup flow to handle errors like the sentinel removal: ignore NotFound, but log other failures with eprintln! and the existing pipeline-lock context. Preserve the current sentinel-then-marker removal order and do not otherwise change the cleanup behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/lib-jj-helpers/src/pipeline_lock.rs`:
- Around line 179-186: Update the reclaim_marker removal in the takeover cleanup
flow to handle errors like the sentinel removal: ignore NotFound, but log other
failures with eprintln! and the existing pipeline-lock context. Preserve the
current sentinel-then-marker removal order and do not otherwise change the
cleanup behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a384fa4d-5153-4fae-8ec1-a8e71eba6032
📒 Files selected for processing (2)
src/lib-jj-helpers/src/pipeline_lock.rssrc/lib-jj-helpers/src/pipeline_lock/tests.rs
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
(前回発火 (issue_comment, 17:50:11Z 投稿) 時点では CodeRabbit レビューが処理中で findings 0 件だったが、今回のレビュー submitted (17:51:47Z) で完了し nitpick 1 件が確定した — 分析価値のある新情報のため再分析。) Applicable Findings (Critical / High / Major)該当なし Applicable Findings (Medium 以下)
Filtered (not applicable)該当なし 次のアクション
|
This was referenced Aug 2, 2026
aloekun
added a commit
that referenced
this pull request
Aug 2, 2026
…3) (#346) PR #342/#343/#344 の post-merge feedback 採用候補 14 件のうち、2026-08-02 の 方針決定 (文書系 10 件 = 1 docs バッチ PR、実装系 4 件 = A/B 系統別 PR) に従い 5 エントリとして登録: - 順位 359: 文書系 10 件の docs バッチ (dev-conventions 集中、Severity High 2 件含む) - 順位 360: cargo test コマンド等価性検証テスト (#342/#343 で連続指摘 = Frequency High) - 順位 361: JJ_VERSION の 2 ファイル一致検証テスト (順位 360 と同一 PR 可) - 順位 362: git subprocess ブランチ名依存引数の custom lint rule (#343 incident 由来) - 順位 363: check-ci-coderabbit の detached HEAD 回帰統合テスト (順位 362 と同一 PR 可)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
finish_reclaimに検証を追加: sentinel が「gate を正当化した stale content のまま」かを確認してから除去する。不一致・消失・読取失敗は一切触れずBusyに倒すSentinelGate::Acquiredが marker path を運び、takeover_stale_lockが sentinel と共に除去)。takeover 実行中の同一 gate 再作成を構造的に排除#[ignore]) を追加reap_orphaned_reclaim_marker等の doc に残っていた誤った前提 (「同じ path が再作成される余地は無い」) を実態に修正Context
Why: PR #342 の CI (windows-latest) で
concurrent_takeover_with_orphaned_sentinel_single_winnerが「得た数: 2」で失敗した。調査の結果 flake ではなく master 由来の実バグ。#342 ブランチは lib-jj-helpers に一切触れていない (diff で確認済み)。メカニズム: 出遅れスレッドが除去前に読んだ stale sentinel content S0 を握ったまま reclaim に入ると、勝者が自己修復を終えて reclaim marker を除去した後でも、content 由来の同一 gate path を再作成して勝ててしまう。旧実装はそこで sentinel を検証なしに除去していたため、勝者の fresh sentinel を破壊して 2 本目の takeover 実行権が発生し、勝者の rename 前に lock を読めば双方
Acquiredになる。なぜ CI でだけ発生したか: 「S0 を読んでから gate を作るまでの µs 窓」での preemption が必要で、2 vCPU runner + 8 スレッドでは現実的に起きるが、多コア開発機ではほぼ起きない (12,800 回 + affinity 2 コア固定でも 0 件)。round 0 で出たのはスレッド起動スタガーが最大のため。ubuntu leg にも同じレースはある (Windows 固有ではない)。
副次的意義: ADR-065 の CI matrix (2 コア Windows runner) が、多コアのローカルでは再現不能な実バグを land 前に炙り出した実例 (ADR-063 の Linux lock レース発見と同型)。
Validation
reclaim_stale_sentinelを直接呼ぶ。修正前 FAILED (CI と同じ強奪挙動をスレッド並行なしで 100% 再現)、修正後 okcargo test -p lib-jj-helpers: 全 pass (新規 3 テスト含む)。高競合 stress 32×400 passcargo test --workspace: 全 pass / clippy-D warnings: cleanpnpm pushpre-push review: simplicity / security 両 approved、pr_size 209 行References
Summary by CodeRabbit