Skip to content

fix: TOCTOU race in close_sessions_for_transport - #77570

Closed
eaglezzz0522-cloud wants to merge 1 commit into
NousResearch:mainfrom
eaglezzz0522-cloud:fix/77192-toctou-race
Closed

fix: TOCTOU race in close_sessions_for_transport#77570
eaglezzz0522-cloud wants to merge 1 commit into
NousResearch:mainfrom
eaglezzz0522-cloud:fix/77192-toctou-race

Conversation

@eaglezzz0522-cloud

Copy link
Copy Markdown

Summary

The transport-disconnect teardown in _close_sessions_for_transport had a TOCTOU (time-of-check-time-of-use) race: it took a snapshot of owned sessions inside _sessions_lock, then iterated over them without re-validating the transport. A concurrent session.resume reconnect could rebind a session to a new transport between the snapshot and the close/detach.

Changes

  • tui_gateway/server.py: snapshot session IDs under _sessions_lock, then re-validate each sessions transport under _session_resume_lock before acting
  • If a session was already torn down or reattached since the snapshot, skip it
  • _session_resume_lock is the same lock used by session.resumes warm-reuse rebind, so the race is eliminated

Verification

  • The lock ordering matches the existing _session_resume_lock usage in _reuse_live_payload
  • _teardown_popped_session is used instead of the removed _close_session_by_id for the close path
  • Grace-orchan reaper (_schedule_ws_orphan_reap) still fires for detached sessions

Closes #77192

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tui Terminal UI (ui-tui/ + tui_gateway/) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state duplicate This issue or pull request already exists labels Aug 3, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #77129 — it implements the same transport-ownership revalidation and adds focused regression coverage.

@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Seven PRs address or reference the same WS disconnect/session.resume transport-ownership TOCTOU reported by the issue complex. #77129 carries the recorded best-fix verdict for the underlying issues and the corrected race-interleaving regression test; #53525 proposes the same core repair with broader transport-rebind coverage, while the remaining PRs are duplicates or stacked variants.

Related pull requests

Duplicates

#77205, #77535, and #77570 duplicate the core repair in #77129; #77206 and #77212 are stacked/recombined variants of that repair plus the RAH-05 test. #53525 overlaps on the core fix but retains a distinct broader transport-rebind salvage path.

Suggested consolidation

Keep open #77129 with a salvage path as the consolidated implementation: retain its fix plus the deterministic real-interleaving regression test, consistent with the recorded best-fix verdicts and the [contributor:50 commits] consolidation decision. Keep #53525 open with a salvage path for its broader prompt.submit/live-payload/queued-prompt ownership-boundary coverage; close #77570 and #77535 as duplicates of #77129, and retain #77205, #77206, and #77212 as closed duplicate/stacked references.

Complex graph

flowchart TD
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I77127(["issue #77127 (open)"])
    I77192(["issue #77192 (open)"])
    subgraph Dup53525 ["PRs duplicating each other"]
        P53525["PR #53525 (open)"]
        P77129["PR #77129 (open)"]
        P77205["PR #77205 (closed)"]
        P77206["PR #77206 (closed)"]
        P77212["PR #77212 (closed)"]
        P77535["PR #77535 (open)"]
        P77570["PR #77570 (open)"]
    end
    P77570 -.->|partial| I77127
    P77570 -.->|partial| I77192
    class I77127 open
    class I77192 open
    class P53525 open
    class P77129 open
    class P77205 closed
    class P77206 closed
    class P77212 closed
    class P77535 open
    class P77570 open
    class P77129 best
    class P77129 best
    class P77129 best
    class P77205 best
    class P77206 best
    class P77212 best
    class P77212 best
    class P77570 target
    click I77127 "https://github.com/NousResearch/hermes-agent/issues/77127"
    click I77192 "https://github.com/NousResearch/hermes-agent/issues/77192"
    click P53525 "https://github.com/NousResearch/hermes-agent/pull/53525"
    click P77129 "https://github.com/NousResearch/hermes-agent/pull/77129"
    click P77205 "https://github.com/NousResearch/hermes-agent/pull/77205"
    click P77206 "https://github.com/NousResearch/hermes-agent/pull/77206"
    click P77212 "https://github.com/NousResearch/hermes-agent/pull/77212"
    click P77535 "https://github.com/NousResearch/hermes-agent/pull/77535"
    click P77570 "https://github.com/NousResearch/hermes-agent/pull/77570"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 7 pull requests and 3 issues in this complex. Each diff was read against this issue; Assessment working set: 49 kB of PR diffs, 27 kB of issue/PR text, 6 kB of discussion (15 comments), 27 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@eaglezzz0522-cloud

Copy link
Copy Markdown
Author

Closing as duplicate of the earlier first-round PR (#77534 / #77129 / #77536 / #77537 respectively). The original individual PRs remain open for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WS disconnect/reconnect TOCTOU fix exists only on an unmerged branch, missing from main (RAH-06)

3 participants