Skip to content

Release YJ (v0.51.680): explicit session-index rebuild thread ownership (#4993, #3894) - #5003

Merged
nesquena-hermes merged 2 commits into
masterfrom
stage/4993-index-thread
Jun 26, 2026
Merged

nesquena-hermes merged 2 commits into
masterfrom
stage/4993-index-thread

Conversation

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Release YJ (v0.51.680) — session-index rebuild ownership hardening

Ships #4993 (@rodboev) — addresses #3894 (bookkeeping hardening).

What it fixes

_rebuild_session_index_background() cleared the rebuild bookkeeping globals in its finally block when only the target tuple matched — so a late-finishing older worker could wipe a newer rebuild's registered state. The cleanup now also requires _SESSION_INDEX_REBUILD_THREAD is current_thread (under the rebuild lock), so an out-of-order older worker leaves the newer owner intact.

Gate

  • Codex: SAFE TO SHIP — check is under _SESSION_INDEX_REBUILD_LOCK (no TOCTOU); an older worker can't clear a newer same-target owner; the genuine owner still clears the globals (no stuck-owner leak). The +27 test is non-vacuous (simulates a same-target handoff, asserts the newer owner survives).
  • Full suite: 10693 passed (1 unrelated known order-flake test_skills_stats_cache, passes in isolation).
  • Pre-merge head re-check: gated == live (54f2f72).

Credit @rodboev.

@greptile-apps

greptile-apps Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This release PR ships a targeted thread-ownership hardening fix for _rebuild_session_index_background: the finally cleanup now requires both a matching target tuple and that the finishing worker is still the registered owner thread (_SESSION_INDEX_REBUILD_THREAD is current_thread, under the lock), preventing a late-finishing older worker from wiping a newer rebuild's bookkeeping globals.

  • api/models.py: Captures current_thread = threading.current_thread() before the try block and gates the finally cleanup on identity (is) in addition to the existing target-tuple equality check. The check is entirely under _SESSION_INDEX_REBUILD_LOCK, eliminating any TOCTOU window.
  • tests/test_session_index.py: Adds a focused regression test that simulates a same-target thread handoff mid-execution via monkeypatching and asserts the newer owner's state is intact after the older worker's finally block runs.

Confidence Score: 5/5

Safe to merge — the change is a single-line guard addition that is strictly additive and lock-protected, with a direct regression test covering the fixed race.

The fix is minimal and mechanically correct: current_thread is captured before the try block (so it's always available in finally), the identity check is performed inside _SESSION_INDEX_REBUILD_LOCK (same lock that protects all writes to the thread global), and all edge cases — early return, exception path, new thread finishing first, old thread finishing first — leave the globals in the right state. The test is non-vacuous and directly exercises the race that was previously unguarded.

No files require special attention.

Important Files Changed

Filename Overview
api/models.py Adds thread-identity check (_SESSION_INDEX_REBUILD_THREAD is current_thread) to the finally cleanup in _rebuild_session_index_background, preventing a late-finishing older worker from wiping a newer owner's registered state. Change is minimal, lock-protected, and logically sound.
tests/test_session_index.py Adds test_background_rebuild_old_thread_finally_preserves_new_same_target_owner — simulates the exact same-target handoff race using monkeypatching; asserts the newer owner survives the old thread's finally block. Test is non-vacuous and directly validates the fix.
CHANGELOG.md Adds v0.51.680 release entry describing the session-index rebuild thread-ownership fix. Maintained by the release process as expected for this repo.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant OldThread as Old Worker (Thread A)
    participant NewThread as New Worker (Thread B)
    participant Lock as _SESSION_INDEX_REBUILD_LOCK
    participant Globals as Module Globals

    Note over Globals: _THREAD = A, _TARGET = (dir, idx)

    OldThread->>OldThread: "current_thread = threading.current_thread() [= A]"
    OldThread->>Lock: acquire (early check passes)
    Lock-->>OldThread: released

    Note over Caller: Same target triggers new rebuild
    Caller->>Lock: acquire
    Lock-->>Caller: released
    Caller->>Globals: "_THREAD = B, _TARGET = (dir, idx)"
    Caller->>NewThread: start()

    OldThread->>OldThread: _write_session_index() (slow)
    NewThread->>NewThread: _write_session_index() (faster)

    NewThread->>Lock: acquire (finally)
    Lock-->>NewThread: released
    Note over NewThread: B is B ✓ → clears globals
    NewThread->>Globals: "_THREAD = None, _TARGET = None"

    OldThread->>Lock: acquire (finally)
    Lock-->>OldThread: released
    Note over OldThread: _THREAD is None, None is A → False
    OldThread->>OldThread: skip clear (new owner preserved) ✓
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller
    participant OldThread as Old Worker (Thread A)
    participant NewThread as New Worker (Thread B)
    participant Lock as _SESSION_INDEX_REBUILD_LOCK
    participant Globals as Module Globals

    Note over Globals: _THREAD = A, _TARGET = (dir, idx)

    OldThread->>OldThread: "current_thread = threading.current_thread() [= A]"
    OldThread->>Lock: acquire (early check passes)
    Lock-->>OldThread: released

    Note over Caller: Same target triggers new rebuild
    Caller->>Lock: acquire
    Lock-->>Caller: released
    Caller->>Globals: "_THREAD = B, _TARGET = (dir, idx)"
    Caller->>NewThread: start()

    OldThread->>OldThread: _write_session_index() (slow)
    NewThread->>NewThread: _write_session_index() (faster)

    NewThread->>Lock: acquire (finally)
    Lock-->>NewThread: released
    Note over NewThread: B is B ✓ → clears globals
    NewThread->>Globals: "_THREAD = None, _TARGET = None"

    OldThread->>Lock: acquire (finally)
    Lock-->>OldThread: released
    Note over OldThread: _THREAD is None, None is A → False
    OldThread->>OldThread: skip clear (new owner preserved) ✓
Loading

Reviews (1): Last reviewed commit: "Release YJ (v0.51.680): explicit session..." | Re-trigger Greptile

@nesquena-hermes
nesquena-hermes merged commit 60f6779 into master Jun 26, 2026
11 checks passed
@nesquena-hermes
nesquena-hermes deleted the stage/4993-index-thread branch June 26, 2026 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants