test: join auto-title threads at teardown; stop titling in the sidecar replay test
tests/gateway/test_timestamp_sidecar_replay.py crashed the interpreter on CI (native fault, green on rerun) on unrelated PRs. Root cause: every run_conversation turn in its fixture spawns the auto-title upgrade daemon thread (title_generator.maybe_auto_title). That thread outlives the test, fails its model call (no provider under CI), and then writes the derived title into the fixture's SessionDB after the fixture closed it, which reopens sqlite on the daemon thread (_reopen_after_close_locked) and prints the auxiliary-failure warning after pytest capture teardown. At the end of the file the threads are still in native sqlite while the interpreter finalizes: the check_same_thread=False-at-shutdown SIGSEGV shape of #113186. Locally the thread finishes in ~250 ms so the race never shows; on a loaded runner it lands on finalization. Fix the class, not the file: - tests/conftest.py: the autouse SessionDB leak sweep now joins the auto-title upgrade threads (bounded, agent.title_generator. wait_for_title_upgrades) before closing stores, so no title worker outlives its test in any file (5 other files spawn them today). - tests/gateway/test_timestamp_sidecar_replay.py: titling is not under test; the fixture no-ops maybe_auto_title (same as tests/agent/test_tool_call_incremental_persistence.py), so its own db.close() no longer races a worker either. - tests/hermes_state/test_session_db_leak_sweep.py: handoff pair pinning the invariant (a slow upgrade thread started in one test is dead by the next); red on base, green with the fix. Proof (scratch plugin delaying the title model call by 1 s): base: 2 auto-title threads alive at interpreter exit, every thread "SessionDB reopened after close() on thread auto-title"; fixed: no thread spawned / none alive at exit in this file and the other five.
This commit is contained in:
@@ -663,8 +663,18 @@ def _close_leaked_session_dbs():
|
||||
on those ``close()`` releases a refcount rather than closing, so a sweep
|
||||
would silently retire a shared generation that a wider-scoped fixture
|
||||
still holds. The registry owns that lifecycle (``close_all()``).
|
||||
|
||||
Before the sweep, the auto-title upgrade threads a turn spawned are joined
|
||||
(bounded): they hold the turn's SessionDB and write to it (and print to
|
||||
``sys.stdout``) after the turn returns, so left running they race this
|
||||
close (``_reopen_after_close_locked`` on a daemon thread), the next test's
|
||||
capture, and interpreter finalization — the ``Fatal Python error`` /
|
||||
SIGSEGV shape of #113186, seen from ``tests/gateway/test_timestamp_sidecar_replay.py``.
|
||||
"""
|
||||
yield
|
||||
title_generator = sys.modules.get("agent.title_generator") # never imported = nothing spawned
|
||||
if title_generator is not None:
|
||||
title_generator.wait_for_title_upgrades()
|
||||
try:
|
||||
from hermes_state_guard import _test_instance_registry as registry
|
||||
except Exception:
|
||||
|
||||
@@ -57,6 +57,9 @@ def responses_agent(tmp_path, monkeypatch):
|
||||
"hermes_cli.plugins.invoke_hook",
|
||||
lambda hook, **kw: [{"context": POLICY}] if hook == "pre_llm_call" else [],
|
||||
)
|
||||
# Titling is not under test; its daemon thread would outlive the turn holding ``db`` and race
|
||||
# the close below (a cross-thread sqlite reopen at interpreter shutdown crashed CI: #113186).
|
||||
monkeypatch.setattr("agent.title_generator.maybe_auto_title", lambda *args, **kwargs: None)
|
||||
|
||||
def respond(kwargs, **unused):
|
||||
captured.append(deepcopy(kwargs))
|
||||
|
||||
@@ -20,6 +20,9 @@ test is actually closed by the suite-level sweep.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import threading
|
||||
import time
|
||||
|
||||
import hermes_state_guard
|
||||
from hermes_state import SessionDB
|
||||
|
||||
@@ -58,3 +61,24 @@ def test_previously_leaked_instance_was_closed_by_the_sweep():
|
||||
# must have closed the leaked instance (writer conn released), which is
|
||||
# what bounds fd/RSS growth in single-process runs.
|
||||
assert db._conn is None
|
||||
|
||||
|
||||
# Same handoff shape for the auto-title upgrade thread a turn leaves behind: it holds the
|
||||
# turn's SessionDB, so the sweep must join it before closing stores (else the daemon thread
|
||||
# reopens the closed store and races interpreter finalization — the SIGSEGV shape of #113186).
|
||||
_upgrade_thread: list[threading.Thread] = []
|
||||
|
||||
|
||||
def test_slow_title_upgrade_thread_is_left_running_within_the_test():
|
||||
from agent.title_generator import _UPGRADE_THREADS
|
||||
|
||||
thread = threading.Thread(target=time.sleep, args=(1.5,), name="auto-title", daemon=True)
|
||||
_UPGRADE_THREADS.add(thread)
|
||||
thread.start()
|
||||
assert thread.is_alive()
|
||||
_upgrade_thread.append(thread)
|
||||
|
||||
|
||||
def test_title_upgrade_thread_was_joined_before_the_sweep_closed_stores():
|
||||
assert _upgrade_thread, "expected the previous test to have started an upgrade thread"
|
||||
assert not _upgrade_thread.pop().is_alive()
|
||||
|
||||
Reference in New Issue
Block a user