fix(desktop): serialize session-lease binds against stale-subagent cleanup / 修复子代理清理持锁竞态 - #7770
Open
zdjmrq wants to merge 2 commits into
Open
fix(desktop): serialize session-lease binds against stale-subagent cleanup / 修复子代理清理持锁竞态#7770zdjmrq wants to merge 2 commits into
zdjmrq wants to merge 2 commits into
Conversation
…eanup / 修复子代理清理持锁竞态 Problem: The false desktop startup error "this session is already open in another Reasonix window" (esengine#7399, esengine#7627) still reproduced on v1.19.7 even though esengine#7405 already added a bounded startup-lease retry (2 x 50ms) and a process-local parent-liveness probe. A multi-parent stale-subagent sweep on Windows can hold each parent session lease for longer than the whole retry budget, so a concurrent tab build racing that sweep could exhaust its retries and surface the spurious error for a lease that was genuinely free milliseconds later. Root cause: CleanupStaleRunning runs inside every controller build and, per stale subagent parent, acquires the parent session lease, rewrites metadata (15+6N file ops on Windows), then releases. With several leftover running subagents plus AV/Defender scanning, the sweep can hold the lease for hundreds of milliseconds to over a second. The esengine#7405 retry window (2 x 50ms) and its probe could not cover that span, and every probe miss (own unbound build, not-yet-published SessionPath) fell back to the same too-short retry. Fix: Serialize the two lease-active regions inside one process: - boot.Options gains SubagentCleanupSerialized; desktop passes a process-local mutex (App.sessionLeaseCoordinationMu) so the stale sweep's parent-lease probes can never overlap a startup bind. - The two startup bind paths (ensureTabSessionLeaseForRebuild, acquireCandidateSessionLease) hold the same mutex while acquiring. - The startup retry window grows to 10 x 100ms as the cross-process fallback (CLI/serve sweeps are transient single-parent holds). Verification: - go test ./internal/boot ./internal/agent -run "TestNewSubagentStore|TestSubagentStore" -count=1 - cd desktop && go test . -run "TestEnsureTabSessionLeaseForRebuild|TestSessionParentLive|TestCleanupStaleRunningWithDesktopParentProbe|TestMetaNeverReportsReadyWithoutController" -count=1 - cd desktop && go test . -run "Lease|SessionRuntime|Startup|Contention|Rebuild|Recover" -count=1 - go vet ./internal/boot && cd desktop && go vet . New regression test TestEnsureTabSessionLeaseForRebuildWaitsForSerializedCleanup holds the coordination lock and the parent lease for longer than the full retry budget: it reproduces the exact reported error text without the fix and passes with it. Fixes esengine#7627 Follow-up to esengine#7405
github-actions
Bot
added
v2
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.
跟进情况（Timeline）
上次修复方式（#7405）与复发原因
#7405 做了什么
CleanupStaleRunning每次 controller build 都执行（不再缓存"已清扫"标记），扫描 stale running subagent；SubagentParentLive）：cleanup 探活 parent lease 前先检查本进程是否有 live tab 持有该 session；withSessionLeaseContentionRetry），吸收亚毫秒竞争。为何 v1.19.7 仍复现
CleanupStaleRunning对每个 stale parent 执行 acquire → 重写 metadata → release，约 15+6N 次文件操作（N = 该 parent 的 running subagent 数）。Windows + Defender/AV 扫描下，多个崩溃遗留 parent 时持锁总时长可达数百 ms 甚至超过 1 秒，而：SessionPath尚未发布、registry 晚于boot.Build发布）都会回落到同一个过短的重试；即：#7405 缩小了偶发窗口，但没有让"绑定"与"清扫持锁"在进程内真正互斥，重试窗口又不足以覆盖 IO 驱动的持锁时长。
为何选择本修复方式
SingleInstanceLock保证单实例，跨进程窗口只剩 CLI/serve 等，由重试兜底即可最终选择：进程内互斥 + 跨进程重试兜底。desktop 进程内用一把进程级 mutex 把"cleanup 的 parent-lease 探活区间"与"启动绑定的 acquire 区间"串行化；CLI/serve/ACP 等非桌面前端保持原有 lease-only 行为（nil 钩子），跨进程瞬态窗口由加大的重试（10×100ms）覆盖。
本次修复内容
internal/boot/boot.goOptions新增SubagentCleanupSerialized func(fn func() error) error：在钩子内执行 stale-subagent cleanup；nil 保持原有行为。newSubagentStore接受并应用该钩子。desktop/app.goApp新增sessionLeaseCoordinationMu sync.Mutex。ensureTabSessionLeaseForRebuild、acquireCandidateSessionLease的 acquire 闭包持有同一把锁（withSessionLeaseCoordination）。serializedSubagentCleanup，并接入全部 7 个boot.Build调用点（SubagentCleanupSerialized）。sessionLeaseCoordinationMu → a.mu(R)Lock / tab 锁 → agent lease 内部，双向核对无死锁环（canReclaimCurrentProcessSessionLease与 probe 均只用a.mu.RLock）。TestNewSubagentStoreRunsCleanupInsideSerializedCoordinator、TestNewSubagentStoreSerializedCoordinatorErrorPropagates。TestEnsureTabSessionLeaseForRebuildWaitsForSerializedCleanup——模拟 cleanup 持锁超过完整 retry 预算（1.3s），并发绑定必须等待而非失败；无修复时精确复现用户报错文本（已验证红→绿）。验证
go test ./internal/boot/ -run TestNewSubagentStore -count=1✅go test ./internal/agent/ -run TestSubagentStore -count=1✅cd desktop && go test . -run "Lease|SessionRuntime|Startup|Contention|Rebuild|Recover" -count=1✅（93s）go vet（boot + desktop）✅、gofmt干净 ✅TestGoldenBaselineNoExtensions为改动前已存在的 golden 漂移（stash 验证与本次无关）潜在问题 / 若再发生
sessionLeaseContentionRetryAttempts或引入跨进程协调。Documentation-impact: none - internal lifecycle coordination only; existing user documentation remains correct.
Cache-impact: low - process-local lease coordination and retry-window changes only; no provider-visible content or cache key changes.
Cache-guard: focused, race, and vet coverage passed on the PR head (see Verification above).
System-prompt-review: no system prompt, tool schema, configuration, or model input content changed.
Fixes #7627
Follow-up to #7405