Skip to content

Fix desktop session lease race and remaining runtime data races / 修复桌面端会话租约竞态及其余运行时数据竞态#5968

Merged
SivanCola merged 4 commits into
esengine:main-v2from
SivanCola:fix/session-lease-race
Jul 5, 2026
Merged

Fix desktop session lease race and remaining runtime data races / 修复桌面端会话租约竞态及其余运行时数据竞态#5968
SivanCola merged 4 commits into
esengine:main-v2from
SivanCola:fix/session-lease-race

Conversation

@SivanCola

@SivanCola SivanCola commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • serialize per-tab session lease acquire/release so concurrent blank-session startup and model/settings rebuilds reuse the current tab lease
  • keep model switching from surfacing a false "another Reasonix window" error when the same tab is still binding its lease
  • extend the same fix across the remaining desktop runtime races found in a follow-up concurrency audit:
    • route the five remaining raw sessionLease accesses (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)
    • move tabEventSink.tabID/app under the sink mutex like ctx (same reasoning as the [Bug]: 2个同时进行的会话间进行切换多次,信息错乱,ai不断输出思考过程,永不结束;用户输入信息发生变动 #5352 clearContext fix); Emit snapshots the binding and detach/reattach rebinds via setBinding
    • run detachRuntimeForReplacement (field clone + lease/sink handover + membership check + re-publication) in one a.mu critical section, so runtimes unlinked by DeleteSession/TrashTopic cannot resurrect ([Bug] Session resurrects after deletion — race condition in CloseTab #4384 class) and the disabledMCP map is never read lock-free while the event goroutine writes
    • serialize rebindTabToLoadedSessionPath and clearActiveSessionRuntime with runtimeRebuildMu like every other build+swap path, make Controller.close idempotent (sync.Once), and add build-generation guards so a session rebind strands any in-flight async build instead of double-swapping controllers
    • replace check-then-use reads in bound methods (MetaForTab, Submit-family readiness/read-only checks, ReplayPendingPrompts, shutdown, MCP toggles, AutoResearch, ReloadCommands) and in the build/rebuild goroutines with locked tabRuntimeSnapshot copies; publish/take SharedHostKey under a.mu only
    • snapshot teardown/autosave/topic-assignment reads under the lock (CloseTab, keepOnlyVisibleTab, closeTabRuntime, detachSessionRuntime, maybeAutoTitleTopic); copy bot install sessions under a.mu; publish tray menu items under a.mu; drop the unlocked bot-runtime lazy init

Fixes #5955

Tests

  • go test -race . (desktop, full suite — includes new regressions: concurrent lease-helper hammer with leak assertion, detach membership + lease transfer, sink rebind during Emit, MetaForTab vs build swap, build-generation supersession)
  • go test -race ./internal/control/ (includes new concurrent double-Close regression asserting SessionEnd fires once)
  • go vet + gofmt clean in both modules

@SivanCola SivanCola requested a review from esengine as a code owner July 5, 2026 03:07
@github-actions github-actions Bot added v2 Go rewrite (1.x) — main-v2 branch, active development desktop Wails desktop app (desktop/**) labels Jul 5, 2026
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.
@github-actions github-actions Bot added the agent Core agent loop (internal/agent, internal/control) label Jul 5, 2026
@SivanCola SivanCola changed the title Fix desktop session lease race Fix desktop session lease race and remaining runtime data races / 修复桌面端会话租约竞态及其余运行时数据竞态 Jul 5, 2026
SivanCola added 2 commits July 5, 2026 13:13
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.
@SivanCola SivanCola merged commit bc18992 into esengine:main-v2 Jul 5, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent Core agent loop (internal/agent, internal/control) desktop Wails desktop app (desktop/**) v2 Go rewrite (1.x) — main-v2 branch, active development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: 后台进程有误

1 participant