fix: bound add_listener calls with wait_for timeout - #48
Open
rcbevans wants to merge 1 commit into
Open
Conversation
asyncpg's add_listener was the last unbounded I/O on the notify path. A half-open PG connection that accepts TCP but stalls on the LISTEN handshake would wedge the notify loop forever. The codebase already bounds all close operations with wait_for + terminate-on-timeout; this applies the same discipline to add_listener. Adds notify_listener_setup_timeout setting (default 10s, validated positive and finite). Both call sites are wrapped: the reconnect path falls through to the existing except-BaseException cleanup, and the initial setup path closes TaskQ-owned connections (respecting the ownership contract) before re-raising. Five new tests cover stall detection at both call sites, caller-owned conn exclusion, successful-within-timeout, and setting respect.
rcbevans
force-pushed
the
fix/worker-teardown-polish
branch
from
July 30, 2026 04:56
dfb52ef to
2f19188
Compare
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.
Problem
asyncpg'sadd_listenerwas the last unbounded I/O on the NOTIFY path. A half-open PG connection that accepts TCP but stalls on the LISTEN handshake would wedge the notify loop indefinitely - no timeout, no retry, no recovery. The codebase already bounds allclose()operations withwait_for+ terminate-on-timeout (see #37);add_listenerwas the remaining gap.Fix
Both
add_listenercall sites are now wrapped withasyncio.wait_for:reconnect_notify_conn): on timeout, falls through to the existingexcept BaseExceptioncleanup that closes the freshly-built conn. No new error handling needed.notify_listener_loop): on timeout, closes TaskQ-owned connections (respecting the ownership contract) before re-raising. Caller-owned connections are never closed.New setting
notify_listener_setup_timeout(env:TASKQ_NOTIFY_LISTENER_SETUP_TIMEOUT):_positive_finite_floatvalidatoradd_listenercall, not the aggregate setupTests
Five new tests in
TestAddListenerTimeout:add_listenerstall is timeboxed and conn is closedadd_listenersucceeds within timeout (no false positive)add_listenerstall is timeboxed, conn closedCleanup
Replaced em-dashes and box-drawing section separators with plain ASCII hyphens across the three touched files.