Skip to content

fix: bound add_listener calls with wait_for timeout - #48

Open
rcbevans wants to merge 1 commit into
mainfrom
fix/worker-teardown-polish
Open

fix: bound add_listener calls with wait_for timeout#48
rcbevans wants to merge 1 commit into
mainfrom
fix/worker-teardown-polish

Conversation

@rcbevans

Copy link
Copy Markdown
Contributor

Problem

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 indefinitely - no timeout, no retry, no recovery. The codebase already bounds all close() operations with wait_for + terminate-on-timeout (see #37); add_listener was the remaining gap.

Fix

Both add_listener call sites are now wrapped with asyncio.wait_for:

  • Reconnect path (reconnect_notify_conn): on timeout, falls through to the existing except BaseException cleanup that closes the freshly-built conn. No new error handling needed.
  • Initial setup path (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):

  • Default: 10 seconds
  • Validated positive and finite via _positive_finite_float validator
  • Bounds each individual add_listener call, not the aggregate setup

Tests

Five new tests in TestAddListenerTimeout:

  1. Reconnect add_listener stall is timeboxed and conn is closed
  2. Reconnect add_listener succeeds within timeout (no false positive)
  3. Initial listener loop add_listener stall is timeboxed, conn closed
  4. Caller-owned conn stall is timeboxed but conn is NOT closed (ownership contract)
  5. Setting is respected: 0.2s timeout allows 0.1s call, 0.02s timeout cancels it

Cleanup

Replaced em-dashes and box-drawing section separators with plain ASCII hyphens across the three touched files.

@rcbevans
rcbevans requested review from XBeg9, clinzy and kjw-azx July 29, 2026 04:28
@rcbevans rcbevans self-assigned this Jul 29, 2026
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
rcbevans force-pushed the fix/worker-teardown-polish branch from dfb52ef to 2f19188 Compare July 30, 2026 04:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant