fix(hook): bound event-delivery teardown — flush/close can never hang process exit#78
Merged
Merged
Conversation
…an never block process exit
A host process (wiki-weaver engine subprocess) finished its entire pipeline
(pipeline:complete -> session:end) and then wedged for 2.7 hours at 0% CPU:
a single CLOSE-WAIT socket to its CI destination was held by httpx
AsyncClient.aclose(), which _DestinationDispatcher.close() awaited with NO
deadline. Related recurring signature: "Task exception was never retrieved
... RuntimeError('Event loop is closed')" from aclose -- teardown tasks
dying unretrieved.
Root cause (logging_handler.py):
- close() line ~1023: `await self._client.aclose()` was unbounded; on a
half-closed (CLOSE-WAIT) connection it blocks forever.
- close() worker join: `await self._worker_task` after cancel() blocks
until cancellation completes -- unbounded if the worker misbehaves.
- _ensure_worker(): asyncio.create_task() with no done-callback, so any
terminal exception was left unretrieved.
Fix (surgical; delivery semantics, config surface, and the local JSONL
write path unchanged):
- New _CLOSE_HARD_TIMEOUT (5s) bounds each previously-unbounded teardown
step via asyncio.wait (NOT wait_for -- wait_for blocks until the inner
cancellation completes, reintroducing the hang for a step that resists
cancellation). On deadline: WARNING + abandon; delivery is documented
best-effort and must never block exit.
- aclose() runs as a task with a done-callback that retrieves late
exceptions: the "Event loop is closed" teardown race is logged at DEBUG
and swallowed; anything else surfaces at WARNING -- never propagated,
never left unretrieved.
- Worker task gets the same done-callback at creation.
- HTTP client timeouts (connect/read/write/pool) were already explicit and
are unchanged.
Tests (tests/test_bounded_teardown.py, 9 new -- 8 fail against the old
code): hung aclose bounded; timeout -> WARNING + clean return; aclose
resisting cancellation still bounded; loop-closed race swallowed at DEBUG;
worker-crash exception retrieved; cancelled worker silent; worker swallowing
cancellation bounded + abandoned; end-to-end against a real accept-then-
never-respond server; happy-path close unchanged. Hot-path AST allowlist
extended for the two zero-cost calls (partial, add_done_callback) on the
worker-start branch.
Module suite: 585 passed, 0 failed (was 577 passed + 8 new-test failures
pre-fix). Repo-level suite: 753 passed.
🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)
Co-Authored-By: Amplifier <240397093+microsoft-amplifier@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
2 participants
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.
Summary
Fixes a production hang: a wiki-weaver engine subprocess that composes this hook finished its entire pipeline (
session:endemitted) and then sat wedged 2.7 hours at 0% CPU on a single CLOSE-WAIT socket to its configured ingestion endpoint (127.0.0.1:8100). The hook'sclose()was blocking forever inhttpx AsyncClient.aclose()with no deadline. A human had to SIGTERM it.Root Cause
Pre-fix (
handlers/logging_handler.py:1023): Unboundedawait self._client.aclose()— per-request httpx timeouts do NOT govern close. Also (:1015-1018) unbounded worker-task join after cancel; (:307)create_taskwith no done-callback.Fix
Bounded, best-effort teardown with two key guarantees:
_CLOSE_HARD_TIMEOUT = 5.0bounds both teardown stepsasyncio.wait— deliberately notwait_for(which would reintroduce the hang for cancellation-resistant steps)acloseas a bounded task, cancelled + abandoned on deadline with a WARNING noting events remain durable in the localevents.jsonl_retrieve_task_exceptiondone-callback on worker + aclose tasksDelivery semantics, config surface, and the local JSONL path unchanged — event delivery is documented best-effort and can now never block process exit.
Test Evidence
9 new tests in
tests/test_bounded_teardown.py:Files Changed
Generated with Amplifier