Fix desktop session lease race and remaining runtime data races / 修复桌面端会话租约竞态及其余运行时数据竞态#5968
Merged
Merged
Conversation
Extends the esengine#5955 session-lease fix (esengine#5968) across the remaining shared-state races found in a full desktop concurrency audit: - sessionLease: route the five remaining raw accesses (detach clone, detach nil write, applyRuntimeTab transfer, rebuild reclaim, cross-tab probe) through new sessionLeaseMu helpers (take/adopt/key), so no path can leak or resurrect a lease and re-trigger the false 'another Reasonix window' error. - tabEventSink: tabID/app now live under the sink mutex like ctx; Emit snapshots the binding, and detach/reattach/clear rebind via setBinding (same reasoning as the esengine#5352 clearContext fix). - detachRuntimeForReplacement: clone, lease/sink handover, membership check and re-publication now happen in one a.mu critical section, so a runtime unlinked by DeleteSession/TrashTopic cannot resurrect (esengine#4384 class) and the disabledMCP map is no longer read lock-free while the event goroutine writes ActivityStatus. - rebindTabToLoadedSessionPath and clearActiveSessionRuntime take runtimeRebuildMu like every other build+swap path; Controller.close is idempotent (sync.Once) so racing teardowns cannot re-fire SessionEnd hooks or double-close. - bound methods (MetaForTab, Submit-family readiness/read-only checks, ReplayPendingPrompts, shutdown, MCP toggles, AutoResearch, ReloadCommands) read per-tab fields via locked snapshots instead of check-then-use. - build/rebuild goroutines (buildTabControllerWithContext, SetModelForTab, SetEffortForTab, SetTokenModeForTab, rebuildSettingLocked, clearActiveSessionRuntime) capture a tabRuntimeSnapshot under a.mu before the off-lock boot, and SharedHostKey is published/taken under a.mu only. - teardown/autosave paths (CloseTab, keepOnlyVisibleTab, closeTabRuntime, detachSessionRuntime, maybeAutoTitleTopic, topic assignment) snapshot fields under the lock before I/O. - App-level fixes: bot install poll copies the session under a.mu, tray menu items are published under a.mu, bot runtime lazy init removed. go test -race ./... passes in desktop and internal/control, including new regression tests for concurrent lease helpers, detach membership/transfer, sink rebind during Emit, MetaForTab vs build swap, and idempotent Close.
Review follow-up on the generation-guard commit: the superseded branches in buildTabControllerWithContext cleaned up by taking SharedHostKey (and, in the final branch, the session lease) from the tab itself. Before generation guards those branches only ran for removed tabs, where the tab's fields were the stale build's own; now a rebind supersedes builds on a *live* tab whose replacement build may already have published its own host key and lease, so the stale build was releasing the new runtime's host reference (leaking the host on same-root rebinds, or closing a host still in use on root changes) and could strip the new session's lease. Superseded cleanup now goes through abandonSupersededBuild, which releases only what the stale build itself acquired: its controller, its own rootKey reference, and — via the new releaseSessionLeaseForKey — only a lease still bound to its own session path (removal keeps working because the keys match; a rebind always changes the key). Owned-failure branches keep the take-from-tab behavior, which is correct there. Also drop tabWorkspaceName and tabRemovedForBuild, left unused by the snapshot/generation refactor and flagged by golangci-lint (unused) in CI. Verified: cd desktop && go test -race . passes; golangci-lint v2.12.2 (CI version) reports 0 issues in both modules; new regressions cover releaseSessionLeaseForKey key-matching and the stale-build-vs-replacement ownership scenario from the review.
Review follow-up: rebindTabToLoadedSessionPath observed tab.Ctrl (and ran loadTabSessionProfile's disk I/O) before invalidating the in-flight async build. runtimeRebuildMu does not cover startTabControllerBuild's goroutine, so inside that window the async build still passed its swap-time generation check: it could install its controller right after rebind observed ctrl == nil, the loaded build then silently overwrote it, and the stale runtime plus its shared-host reference leaked (its lease was already covered by the adopt-on-ensure path). The validation section now runs in one a.mu critical section that checks tab membership, compares session keys (same-key resume stays a no-op and leaves an in-flight build alone), bumps buildGeneration + cancels the build context, and only then snapshots tab.Ctrl — making the snapshot authoritative: after the bump the async build can only fall into its superseded branches, which release exactly what it acquired via abandonSupersededBuild. The duplicate bump at the ctrl!=nil branch tail is gone, and the build now re-checks ownership right after its lease bind so a superseded build cannot write its stale session path back onto the rebound tab. New regression stalls a real async build inside its lease bind, rebinds the tab to another session mid-flight, then releases the build to attempt its swap: the rebound controller must survive, and the stale build's lease and shared-host reference must both be released (exactly one host ref remains). Verified: cd desktop && go test -race . passes; golangci-lint v2.12.2 (CI version) 0 issues; gofmt/go vet clean.
This was referenced Jul 5, 2026
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
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
sessionLeaseaccesses (detach clone / nil write, reattach transfer, rebuild reclaim, cross-tab probe) through locked take/adopt/key helpers, so no interleaving can leak a lease (session permanently "held") or resurrect a released one (two writers on one.jsonl)tabEventSink.tabID/appunder the sink mutex likectx(same reasoning as the [Bug]: 2个同时进行的会话间进行切换多次,信息错乱,ai不断输出思考过程,永不结束;用户输入信息发生变动 #5352 clearContext fix);Emitsnapshots the binding and detach/reattach rebinds viasetBindingdetachRuntimeForReplacement(field clone + lease/sink handover + membership check + re-publication) in onea.mucritical section, so runtimes unlinked by DeleteSession/TrashTopic cannot resurrect ([Bug] Session resurrects after deletion — race condition in CloseTab #4384 class) and thedisabledMCPmap is never read lock-free while the event goroutine writesrebindTabToLoadedSessionPathandclearActiveSessionRuntimewithruntimeRebuildMulike every other build+swap path, makeController.closeidempotent (sync.Once), and add build-generation guards so a session rebind strands any in-flight async build instead of double-swapping controllersMetaForTab, Submit-family readiness/read-only checks,ReplayPendingPrompts, shutdown, MCP toggles, AutoResearch,ReloadCommands) and in the build/rebuild goroutines with lockedtabRuntimeSnapshotcopies; publish/takeSharedHostKeyundera.muonlya.mu; publish tray menu items undera.mu; drop the unlocked bot-runtime lazy initFixes #5955
Tests