Skip to content

fix(web): join dashboard eager-reconcile thread on shutdown - #113265

Closed
JoaoMarcos44 wants to merge 1 commit into
NousResearch:mainfrom
JoaoMarcos44:fix/113186-join-eager-reconcile-on-shutdown
Closed

JoaoMarcos44 wants to merge 1 commit into
NousResearch:mainfrom
JoaoMarcos44:fix/113186-join-eager-reconcile-on-shutdown

Conversation

@JoaoMarcos44

Copy link
Copy Markdown

Summary

Dashboard lifespan now keeps the statedb-eager-reconcile thread handle and joins it on shutdown, before hosted-room teardown. A TestClient/lifespan portal can no longer leave native sqlite workers running into pytest/interpreter finalization.

Fixes #113186

Observed problem

  • Incorrect: tests/hermes_cli/test_web_profiles_off_loop.py (and any other file that starts the dashboard lifespan repeatedly) intermittently dies with Fatal Python error: Segmentation fault in sqlite while CI prints 0 failed plus 1 file where no tests ran. The faulthandler dump shows many statedb-eager-reconcile threads in _open_session_db_at_path while the main thread is already in pytest_sessionfinish.
  • Expected: lifespan shutdown owns that startup worker. After a portal exits, no statedb-eager-reconcile thread remains, so later tests and interpreter teardown cannot race sqlite3_step / connection init.
  • Confirmed on current origin/main (fdb7b216ec at publication prep; worktree then rebased onto it): _lifespan starts threading.Thread(..., daemon=True, name="statedb-eager-reconcile") without retaining the object, and finally joins hosted-room-startup but never the reconcile worker.

Root cause

The dashboard is a view layer and must not delay the socket, so schema reconcile is a daemon thread started before the lifespan yield (#79531 / #80037, GH-73083). That is still correct for startup.

The missing owner is shutdown. TestClient as a context manager runs lifespan finally at portal exit. Hosted-room recovery is cancelled and joined there; the reconcile worker is not. Each test therefore leaks a daemon thread that can still be inside check_same_thread=False sqlite while:

  1. the next portal starts another reconcile against the same temp state.db, and
  2. pytest later finalizes the interpreter (pytest_sessionfinish).

That is the segfault shape in the issue, not a test-local assertion failure. A per-store lock around read-only SessionDB init does not stop the worker from outliving the app.

Impact and blast radius

  • Affected: dashboard/serve lifespan; CI files that construct TestClient(app) repeatedly (test_web_profiles_off_loop.py, test_web_server.py, and siblings). Production clean shutdown of hermes dashboard / Desktop serve also waits up to 5s for reconcile to finish before hosted-room stop, which avoids a sqlite race with stop_hosted_room_service.
  • Not affected: the reconcile algorithm itself (still read-only-first, still never raises, still daemon so a locked store cannot block bind). Gateway writers. Leak-sweep policy in tests (#112075). Fresh-home hosted-room boot/SIGBUS (#104828).
  • Security: not a trust-boundary or credential issue. Lifecycle/reliability only.
  • Performance: startup path unchanged (thread still starts before yield). Shutdown may wait up to _STATEDB_EAGER_RECONCILE_JOIN_TIMEOUT_S (5.0s) if the worker is still in sqlite.

Solution

In hermes_cli/web_server.py::_lifespan:

  1. Keep statedb_eager_reconcile_thread instead of Thread(...).start().
  2. Join it in finally with a 5.0s bound before hosted-room cancel/stop/join, so two sqlite users are not tearing down at once.
  3. Leave daemon=True so a wedged store still cannot pin Desktop bind (Desktop startup fails with GIL stall on Windows — _warm_gateway_module() import blocks event loop 15-22s #73083).

Alternatives considered and rejected:

  • Serializing read-only opens with a new process-local lock map (#113235): does not join the worker; adds a second lock registry beside hermes_state_registry; leaves the interpreter-finalization race if the thread outlives the portal.
  • Joining from one test fixture: the reporter already noted every TestClient file is exposed; the owner is lifespan.
  • Copying #104828's hosted-room Event ordering / connect_tracked SIGBUS work: different bug class; this PR only closes the outliving reconcile worker.
  • Changing _eager_reconcile_own_session_db or making the thread non-daemon: would either delay bind or change heal semantics.

Related work (not duplicated)

  • Open #113235 (KoNit-K) — lock around read-only SessionDB init. Incomplete for this issue: the reporter's required owner is shutdown join. This PR does not use that lock or that test.
  • Open #104828 (Alish3r) — fresh-home state.db boot/SIGBUS with hosted-room tracking. Complementary. This PR does not take its hosted-room or sqlite-tracking files.
  • Open #112075 (teknium1) — test leak sweep must not SessionDB.close() a live probe thread. Complementary; different files. Join-on-shutdown does not replace a safe sweep.

No issue/PR comments were posted on those threads.

Compatibility and residual risks

  • If reconcile is still running after 5s, join returns and a daemon worker can still exist. That bound matches "do not hang shutdown"; a wedged sqlite lock remains a residual process-lifetime issue.
  • local-runtime-boot is still fire-and-forget. It is not the sqlite path in the dump.
  • Runner-side classification of Segmentation fault in run_tests_parallel.py is intentionally out of scope. The real fix is that the worker must not outlive the app.
  • This does not prove the exact CI page-fault cannot still occur under an untested cross-process topology.

Tests and verification

All commands used scripts/run_tests.sh with HERMES_PYTHON=C:/Users/Nitro/hermes-agent/.venv/Scripts/python.exe from an isolated worktree. Do not treat the full suite as run.

Command Purpose Result
scripts/run_tests.sh tests/hermes_cli/test_web_server_reconcile_shutdown.py -q on unfixed web_server.py (HEAD production file restored) RED: prove the new tests catch missing join 2 failed: live statedb-eager-reconcile thread(s) after TestClient exit; sequential case had two live daemons
Same command after join GREEN 2 passed
Three consecutive focused reruns after tightening the hold-Event test flake check 2 passed, 2 passed, 2 passed (exit 0 each)
Historical: one focused run after an earlier test shape flake observation file retried: first attempt failed assert _alive_reconcile_threads() inside the portal (worker already finished during slow lifespan startup); retry passed. Tests were then rewritten so the in-portal assertion holds the worker on an Event. Not used as final green evidence.
scripts/run_tests.sh tests/hermes_cli/test_web_server_reconcile_shutdown.py tests/hermes_cli/test_web_server_boot_handshake.py tests/hermes_cli/test_web_profiles_off_loop.py -q on final rebased head 52d82ad3ee sibling portals + original flake file 24 passed, 0 failed
scripts/run_tests.sh tests/hermes_cli/test_web_server.py -k 'startup_eager_reconcile' -q existing heal / read-only / never-raises contracts 3 passed (file reports ~189 collected, 3 selected)
ruff check hermes_cli/web_server.py tests/hermes_cli/test_web_server_reconcile_shutdown.py lint All checks passed
git diff --check whitespace clean
Full scripts/run_tests.sh whole suite not executed
Desktop / Windows-only / Docker lanes n/a not executed
graphify update call graph skipped; graphify-out/graph.json absent

Files and documentation

  • hermes_cli/web_server.py — retain and join statedb-eager-reconcile on lifespan shutdown.
  • tests/hermes_cli/test_web_server_reconcile_shutdown.py — behavior contracts: worker is alive during the portal; sequential portals do not leave named threads after shutdown.

No changelog/docs change; this is an internal lifecycle join.

Publication identity

  • Branch: fix/113186-join-eager-reconcile-on-shutdown
  • Head SHA at body draft: 52d82ad3eec6a9fdbefe6b4abea84078cde3fa90
  • Base: origin/main fdb7b216ecdde7ebe8409c2b81d74685784afa62
  • Three-dot files: the two paths above only
  • Isolated worktree: original C:/Users/Nitro/hermes-agent checkout left on main with its pre-existing untracked files untouched

Keep the statedb-eager-reconcile handle and join it in lifespan teardown
before hosted-room stop so TestClient portals cannot leave sqlite workers
alive into interpreter finalization.
@kyssta-exe

Copy link
Copy Markdown

Summary
Fixes the intermittent Fatal Python error: Segmentation fault in sqlite when dashboard lifespan tests run repeatedly: _lifespan started the statedb-eager-reconcile daemon thread fire-and-forget, so each TestClient portal leaked a worker that could still be inside sqlite3_step during interpreter finalization. The thread handle is now retained and joined (5s bound) on shutdown before hosted-room teardown, while staying daemon=True so a wedged store still can't delay bind. RED/GREEN proven with the new tests.

What changed

  • hermes_cli/web_server.py::_lifespan: retains statedb_eager_reconcile_thread, joins with _STATEDB_EAGER_RECONCILE_JOIN_TIMEOUT_S (5.0s) in finally before hosted-room cancel/stop/join
  • tests/hermes_cli/test_web_server_reconcile_shutdown.py (new): worker-alive-during-portal + sequential-portals-leave-no-threads contracts

Strengths

Findings

  • Residual acknowledged in the body (worker surviving past the 5s bound on a wedged store) is real but acceptable; a log.warning on join timeout-exceeded would make that silent case observable. Optional one-liner.
  • Join ordering (reconcile before hosted-room stop) is right to avoid two sqlite users tearing down at once — just confirming the reasoning, no change.

Verdict
Looks good to merge.

Reviewed using Hermes-Agent

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/dashboard Web dashboard / control panel UI (dashboard/, landing) area/sessions Session lifecycle, resume, persistence, history labels Sep 16, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks @JoaoMarcos44 — this was the right diagnosis and the right mechanism (lifespan owns the statedb-eager-reconcile worker and joins it at shutdown), and you had it up first.

The same fix landed on main in #113552 (c15286f44d0), which reached merge before this PR was reviewed. Its one divergence: the worker is a non-daemon thread joined without a timeout — SessionDB's own lock patience already bounds the join, and a timed-out join would leave a live sqlite connection to be closed cross-thread, which is the segfault the fix exists to prevent. tests/hermes_cli/test_web_server_boot_handshake.py::test_lifespan_shutdown_joins_statedb_reconcile_worker covers the invariant your test asserts (no statedb-eager-reconcile thread alive after the TestClient context exits).

Closing as superseded; the runner-side half of #113186 (reporting the segfault as a crash rather than "0 failed, no tests ran") is #115587. Apologies that the credit didn't route through a cherry-pick here — the priority is noted on the issue.

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

Labels

area/sessions Session lifecycle, resume, persistence, history comp/dashboard Web dashboard / control panel UI (dashboard/, landing) P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI flake: tests/hermes_cli/test_web_profiles_off_loop.py segfaults in sqlite under concurrent dashboard startup reconcile threads

4 participants