Skip to content

fix(task): serialize sends with a lock so send() cannot hang - #190

Merged
frostming merged 1 commit into
mainfrom
fix/sender-lock
Nov 9, 2026
Merged

frostming merged 1 commit into
mainfrom
fix/sender-lock

Conversation

@frostming

Copy link
Copy Markdown
Contributor

Problem

MessageSender pushed each frame onto a queue drained by a background loop task, and send() awaited a future the loop settled. Once the loop stopped, nothing settled those futures, so send() could wait forever:

  • a write/drain error ended the loop, leaving queued sends and any later send pending;
  • a send() after close() enqueued behind the stop sentinel;
  • cancelling the loop task (e.g. TaskSupervisor.shutdown()) abandoned in-flight and queued sends.

Fix

The sender only needs to serialize writes, so replace the queue + loop with an asyncio.Lock. StreamWriter.write() buffers a whole frame synchronously, so frames can't interleave; the lock serializes drain() in FIFO order. Writes now run in the caller's task, so errors and cancellation surface directly in send(). After a failed write or close(), further sends raise ConnectionError (chained to the original write error).

MessageSender(writer, supervisor) becomes MessageSender(writer); the only caller is Connection.

Follow-ups exposed by sends no longer hanging:

  • Connection._run_request: a response that can't be sent because the connection already closed is dropped (debug log) instead of failing the task.
  • TaskSupervisor.shutdown(): uses gather(..., return_exceptions=True), so it no longer re-raises errors from tasks that already failed and were reported by the error handlers (this made Connection.close() raise in test_cancellation_cascades_between_sdk_peers).

Behaviour change

close() no longer waits for queued frames to flush. Sends already inside the lock still complete; sends still waiting for the lock raise ConnectionError.

Testing

  • New tests/test_message_sender.py: write order, send after close, write failure fails waiting and later sends, cancelled send releases the lock.
  • make check: all checks passed.
  • uv run python -m pytest: 493 passed, 1 skipped (3 runs), excluding tests/http/test_websocket.py and tests/http/test_ws_cookies.py. 5 tests in those two files fail in my local environment on main as well, unrelated to this change.

🤖 Generated with Claude Code

MessageSender used a queue drained by a background task. Once that loop
stopped (write error, cancellation, or a send after close), queued and
later sends awaited futures that were never settled.

Replace the queue and loop with an asyncio.Lock: writes run in the
caller's task, so errors and cancellation surface directly. After a
failed write or close(), further sends raise ConnectionError.

Also:
- Connection._run_request drops a response that can't be sent because
  the connection already closed, instead of failing the task.
- TaskSupervisor.shutdown() no longer re-raises errors from tasks that
  already failed and were reported by the error handlers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@frostming
frostming merged commit 8a99407 into main Nov 9, 2026
8 checks passed
@frostming
frostming deleted the fix/sender-lock branch November 9, 2026 04:49
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