Repository navigation
Fix disconnect watcher rescheduling a finished asyncio.Timeout (#59) - #60
Merged
Merged
Conversation
With BaseHTTPMiddleware in front (as in our apps) and a server whose send()
takes a moment (Granian), PostgresJsonResponse's disconnect watcher woke up
after almost every response and failed with "Cannot change state of finished
Timeout" ("Task exception was never retrieved"): the middleware's receive()
waits in an anyio task group that it cancels once the response has been sent,
the watcher's own cancellation arrives with it and is swallowed there, and
receive() then returns a synthetic http.disconnect after the stream's cancel
scope has been exited.
The watcher now only reschedules (and only keeps receiving) while the stream
is still running. CancelOnDisconnectRoute's watcher waits on our own Event,
which cannot swallow the cancellation, but gets the same guard defensively.
Bump to 0.16.1.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.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.
Fixes #59
Reproduced (deterministically)
The trigger is
BaseHTTPMiddlewarein front together with a server whosesend()takes a loop iteration or two (Granian; a real socket write). It is not specific to failing queries:tests/_granian_app.py's@app.middleware("http"), ~97 % of all responses (both/okand a query failing before the first byte) loggedTask exception was never retrieved: RuntimeError('Cannot change state of finished Timeout')fromwatch_for_disconnect(598 out of 600 requests). Without the middleware: 0.BaseHTTPMiddleware(PostgresJsonResponse(...))and aFakeClientwhosesend()takes 2 loop iterations fails on every run (1 iteration does not).Mechanism:
BaseHTTPMiddleware'sreceive_or_disconnect()awaits the realreceive()inside an anyio task group and cancels that group onceresponse_sentis set. That cancellation and ourwatcher.cancel()from thefinallyhit the watcher together; asyncio delivers them as oneCancelledError(the secondTask.cancel()only sets_must_cancelon a task whose waiter is already done), anyio recognises it as its own and swallows it, andreceive_or_disconnect()returns a synthetichttp.disconnect. Traced: the watcher comes back fromreceive()withcancelling() == 1and<Timeout [finished]>, soreschedule()raises (it only accepts stateENTERED;EXPIRING/EXPIRED/EXITEDraiseCannot change state of … Timeout,CREATEDraiseshas not been entered). SoTask.cancel()alone is not enough to keep the watcher from running on: whateverreceive()is wrapped in may absorb it.Other interleavings checked:
receive():Task.cancel()on a task whose awaited future is done sets_must_cancel, the next step throwsCancelledErrorat theawait,reschedule()is not reached. Safe; only areceive()that swallows the cancellation breaks this (above).EXPIRING: only the watcher itself reschedules, once, so the scope can't be expiring when it does. Not reachable.finally, i.e. reduce to (a).Fix
PostgresJsonResponse.__call__: astreamedflag, set in thefinallyright beforewatcher.cancel()(still inside the scope). The watcher only keeps callingreceive(), and only callsreschedule(), while it is unset, so a watcher whose cancellation was swallowed returns quietly instead of rescheduling an exited scope (or receiving on after the response).CancelOnDisconnectRoute(defensive): its watcher waits on our ownasyncio.Event, which can't swallow the cancellation, so the race doesn't happen there (also checked under Granian +BaseHTTPMiddleware: 0 errors onmainfor/cancellable-ok). It gets the samehandledguard anyway.Version 0.16.1 (
uv.lockfollows).Tests
test_pg_json_cancel.py::test_a_receive_that_swallows_the_watchers_cancellation_does_not_break_it[failing query / ok query]:BaseHTTPMiddleware+FakeClient(send_iterations=2), collects unhandled loop errors (newtests._asgi.unhandled_loop_errors(), withgc.collect()). Fails onmain, passes with the fix.test_pg_json_granian.py::test_finished_responses_leave_no_failed_watcher_behind[/ok, /fails-at-once]: 20 requests each against real Granian with the middleware, asserts nonever retrievedin the server log. Fails onmain, passes with the fix.test_cancel_on_disconnect.py::test_a_watcher_whose_cancellation_is_swallowed_leaves_the_finished_scope_alone: forces the swallow with anEventwhosewait()turnsCancelledErrorinto a return (the path can't be hit otherwise). Fails without the guard, passes with it.tests/test_pg_json_*.py+tests/test_cancel_on_disconnect.py: 75 passed locally;ty check pgdevkitandruff checkon the changed files are clean.🤖 Generated with Claude Code