Repository navigation
fix(autohooks): close ariel's connections on the failure path too - #186
Merged
Merged
Conversation
`main()` ran each command as `asyncio.run(<coroutine>)` and called `_close_ariel()` on the NEXT line, with nothing between them. An exception from the command skipped the close, and aiosqlite's worker threads are NON-daemon, so `threading._shutdown` joins them at interpreter exit forever: the process prints its traceback and never returns. systemd keeps the unit `active`, so `Restart=on-failure` never fires. The daemon is the worst case — it is the only command with no `ARIEL_CLI_GUARD`, so the hang has no timer at all. Measured before: a failing command left the interpreter spinning until an external `timeout 10` killed it (exit 124), for both `recall` and `daemon`. After: traceback printed, exit 1, no worker threads, 1.5 s wall. The command dispatch is now wrapped in `try/except BaseException/else`: `else` closes on every `return`, `except` closes and re-raises so the command's own exception is never replaced by a close failure. Wrapping the whole dispatch rather than the individual call sites means a command added later is covered without anyone remembering to close it. An `atexit` hook is NOT a substitute, and that was measured rather than assumed: CPython runs atexit handlers BEFORE `threading._shutdown` joins the workers, and the same hang reproduces with one registered. `shared/connection.py::close_all` had the same defect one level down: `PRAGMA optimize` and `conn.close()` shared one `contextlib.suppress(Exception)`, so a locked handle made the optimize raise and the close never ran — reintroducing exactly the hang this commit removes from the command path. They now have separate suppressions; the optimize is an optimisation, the close is the contract. Found while auditing the same class (left alone, out of scope, each reproduced with a real hang): `scripts/warm_ner_cache.py`, `scripts/warm_embedding_cache.py`, `scripts/dedup_core_twins.py`, `scripts/resolve_conflicts.py`, `scripts/mirror_identity.py` all close only on their success paths.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
Cipher208
added a commit
that referenced
this pull request
Nov 3, 2026
…n close visible (#189) Same class as #186, still present in eight scripts and the stdio server path: `asyncio.run(main())` with `close_all()` as the NEXT statement, no `try/finally`. An exception in the work call skipped the close, aiosqlite's worker threads are non-daemon, `threading._shutdown` joins them forever, and the process prints a traceback and never exits. Reproduced on the exact shape of these scripts: external `timeout 8` -> exit 124; with `finally` -> exit 1 in one second. Fixed at eleven reachable sites: scripts/warm_ner_cache.py, scripts/warm_embedding_cache.py (module-level), scripts/dedup_core_twins.py::run, scripts/dedup_episode_twins.py::run and ::backfill_transitions, scripts/resolve_conflicts.py::run, scripts/mirror_identity.py::run_manifest and ::main._run, scripts/docs_to_wiki.py::_run, scripts/purge_foreign_rows.py::main, and mcp_server/server.py's stdio branch. The stdio branch matters most: it had NO close at all, so a client closing stdin left the server parked in `threading._shutdown` while its parent waited. Measured with a real worker thread: at the base the process exits 124 in both the normal-return and the raising case; now exit 0 and exit 1 respectively. The close failure is logged, not suppressed. A bare `contextlib.suppress(Exception)` would have made a genuinely broken close invisible on the SUCCESS path — exit 0 where the base exited 1, with nothing in the log. Each site now does `except Exception: logger.warning("close_all() failed during shutdown", exc_info=True)`, so the failure is observable while an in-flight exception still propagates unchanged and no exit code is rewritten to compensate. `mirror_identity` also restores `cm.base_dir` in the `finally`: at the base an exception left the module-global pointing at the last manifest base. Not touched: `_shutdown_handler`'s `os._exit(0)` in `mcp_server/server.py` — it is a deliberate way to avoid this hang on the http+dashboard path, and changing it is a separate decision. Found but left alone (pre-existing, outside this package): the same shape in `scripts/sync_skills.py`, `scripts/verify_autohooks.py` and `scripts/backfill_memory_kind.py`. Co-authored-by: Cipher208 <269750686+Cipher208@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Что
main()выполнял команду какasyncio.run(<корутина>), а_close_ariel()стоял следующей строкой, безfinally. Исключение посреди работы пропускало закрытие.Рабочие потоки aiosqlite — не-daemon,
threading._shutdownджойнит их при выходе интерпретатора. Итог: процесс печатает traceback и не завершается; systemd держит юнитactive, поэтомуRestart=on-failureне срабатывает.Демон — худший случай:
_arm_cli_guardставится только приns.command != "daemon", то есть у зависания демона нет даже таймера.Замерено
recallчерез настоящийmain()timeout 10→ exit 124, виситdaemontimeout 10→ exit 124atexit-хукatexitотвергнут замером, а не рассуждением: CPython выполняет atexit-хуки до join потоков.Форма фикса
Диспетчер команд обёрнут в
try/except BaseException/else:else— закрытие на каждомreturn;except— закрытие иraise, чтобы сбой самого закрытия не подменил исходное исключение.Обёрнут весь диспетчер, а не отдельные вызовы: команда, добавленная позже, попадает под закрытие без чьего-либо «не забыть».
Второй дефект, найденный на ревью
shared/connection.py::close_allдержалPRAGMA optimizeиconn.close()в одномsuppress(Exception). Залоченный хэндл → optimize падает →close()не выполняется → не-daemon воркер остаётся → тот же самый хэнг, который мы убираем из командного пути. Теперь два отдельныхsuppress; тестtest_close_all_closes_even_if_optimize_failsпадал до правки.Найдено при аудите того же класса — не чинил, вне зоны
Каждый подтверждён живым зависанием (exit 124):
scripts/warm_ner_cache.py,scripts/warm_embedding_cache.py,scripts/dedup_core_twins.py,scripts/resolve_conflicts.py,scripts/mirror_identity.py— закрывают соединения только на успешных путях.Проверка
Как делалось
Этот пакет — контрольная проверка процесса: спеку получил фиксер-субагент, код писал он, не я. Verifier искал соседние пути того же класса сам, по своей инициативе — именно это и вытащило второй дефект в
close_allи пять скриптов.