Fix SocketStream.send() writing to a paused transport after a cancelled send by graingert · Pull Request #1299 · agronholm/anyio

graingert · 2026-08-29T18:19:52Z

NOTE Erasing or replacing the contents of this template will result in your pull
request being summarily closed without consideration!

Changes

On the asyncio backend, StreamProtocol sets the transport's write buffer high water
mark to 0, so the protocol is paused as soon as the transport has had to buffer
anything the OS would not take. SocketStream.send(), however, went straight to
transport.write() without waiting on the write event first.

transport.write() never offers anything to the OS while its buffer is non-empty — it
just appends. A send() cancelled while awaiting the write event leaves exactly that
state behind: data in the transport's buffer and a paused protocol. The next send()
then piled its data on top without the OS ever seeing it, so repeated cancellation
could grow the buffer without bound, despite the zero high water mark.

This waits on the write event before writing, as the datagram sockets will do in #1294.

UNIXSocketStream is unaffected (it sends to the raw socket via wait_writable, with
no transport buffer). TLSStream wraps SocketStream and so inherits the fix.

Checklist

If this is a user-facing code change, like a bugfix or a new feature, please ensure that
you've fulfilled the following conditions (where applicable):

You've added tests (in tests/) which would fail without your patch
You've updated the documentation (in docs/), in case of behavior changes or new
features
You've added a new changelog entry (in docs/versionhistory.rst).

On the test

The test asserts on what reaches the peer rather than on transport internals, so it
runs on both backends and needs no mocking: it backs the connection up with two
cancelled sends of 8 MiB (the second soaks up whatever room the peer's
acknowledgements reopened after the first one blocked), cancels a third send, and then
asserts that none of that third payload arrives ahead of a final, uncancelled one. It
fails on every asyncio parameter without the patch and passes on trio either way,
which is the point — trio already behaves this way.

It is skipped on Windows, where the proactor transport does not stay paused while the
connection is backed up, so there is no back-pressure for the test to observe.

SocketStream.send() went straight to transport.write(), which appends to the transport's write buffer without ever offering the data to the OS whenever that buffer is non-empty. A send() cancelled while awaiting the write event leaves exactly that state behind: buffered data and a paused protocol. The next send() then piled its data on top, so repeated cancellation could grow the buffer without bound despite its zero high water mark. Wait on the write event before writing, as the datagram sockets do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Filling the socket buffers with repeated small sends and cancelling once the transport had buffered anything left too little in its buffer: on macOS it drained completely during the few event loop iterations that the cancel scope takes to unwind, so the transport was no longer paused and the assertion on the setup itself failed. Cancel a single send of far more data than the sockets can hold instead, so that the buffer cannot be flushed out from under the test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Asserting on the transport's write buffer only worked on the selector event loop and needed the transport swapped for a proxy to sample the paused state at the moment of the write. It failed outright on the proactor loop, where transport.write() starts an overlapped write of everything it is given rather than buffering it, so the first send completes instead of blocking. Assert on what actually reaches the peer instead: the data of a send that was cancelled while the connection was backed up must never arrive, and that holds on trio too. The first cancelled send is excluded, as its data may already have been handed over on asyncio. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

select() never reported the socket as unwritable on Windows, so the loop waiting for that spun until its deadline and failed every parameter of the test, trio included. Send and cancel a second large payload instead: it soaks up whatever room the peer's acknowledgements reopened after the first one blocked, which is what the readiness check was there to wait for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The asyncio transports there hand a pending buffer to a fresh overlapped write and resume the protocol as soon as the previous one completes, before the OS has accepted any of it. The protocol is therefore only ever paused in passing, and a send cancelled while it is paused still delivers its data, so there is nothing for the test to observe. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Unlike Trio, which runs a cancel_shielded_checkpoint() on the way out of a failed socket call, the closed and broken paths now only get the checkpoint_if_cancelled() at the top. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

agronholm

The main code is fine, I just have some reservations about the test. If it can be simplified, then please do so.

agronholm · 2026-08-31T23:17:54Z

+    ) -> None:
+        """
+        Handing data to a paused transport merely appends it to the write buffer, from
+        where it is delivered anyway, so a cancelled ``send()`` must not have done so.


Suggested change

where it is delivered anyway, so a cancelled ``send()`` must not have done so.

If a ``send()`` was cancelled after the data was written to the buffer, the

next call must ensure that the previous send completed one way or another

before attempting to send its own data.

agronholm · 2026-08-31T23:18:29Z

+                # Back the connection up, and then soak up any room that the peer's
+                # acknowledgements may have reopened in the meantime, so that the OS
+                # cannot take another byte. Nothing is read from the peer until further
+                # down, so the connection stays that way.
+                await send_and_cancel(payload)
+                await send_and_cancel(payload)
+
+                # On Windows, a transport can be genuinely backed up without its
+                # pause_writing() having fired yet: that only happens as a side effect
+                # of the next write() call discovering it, by which point that call's
+                # own data is already appended to the write buffer. Spend that one on
+                # a throwaway payload so the transport is *known* paused going into
+                # the next send() below, before it ever calls write() again.
+                await send_and_cancel(b"r" * 64)
+
+                # Now that the transport is known paused, the OS still never accepted
+                # any of this, so none of it may reach the peer
+                await send_and_cancel(b"c" * 64)
+
+                # Drain the peer until a final, uncancelled send() has arrived; data
+                # that a cancelled send() wrongly handed over would arrive first


I don't think a mere test warrants this many lines of commentary. Would it be possible to simplify this test with mocks instead of relying on whimsy OS-level behavior?

agronholm · 2026-08-31T23:26:53Z

+                # that a cancelled send() wrongly handed over would arrive first
+                received = bytearray()
+                arrived = False
+                with fail_after(60):


This is a no-op given the 20 second default timeout we have for tests.

graingert force-pushed the socketstream-cancelled-send-backpressure branch from ce86c7c to 544f230 Compare August 29, 2026 18:21

graingert mentioned this pull request Aug 29, 2026

call set_write_buffer_limits(0) for UDPSockets so that send returns once data hits the kernel #1294

Open

3 tasks

graingert marked this pull request as draft August 29, 2026 18:41

graingert and others added 3 commits August 29, 2026 19:41

graingert marked this pull request as ready for review August 29, 2026 18:58

graingert and others added 2 commits August 30, 2026 16:34

Merge branch 'master' into socketstream-cancelled-send-backpressure

b7e8c97

use checkpoint_if_cancelled() and yield after the send instead

31b87e0

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

graingert requested a review from agronholm August 30, 2026 15:37

graingert and others added 2 commits August 30, 2026 17:57

don't checkpoint when send() raises without sending

654e513

Unlike Trio, which runs a cancel_shielded_checkpoint() on the way out of a failed socket call, the closed and broken paths now only get the checkpoint_if_cancelled() at the top. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

state only what CI shows in the Windows skip reason

443572b

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

graingert force-pushed the socketstream-cancelled-send-backpressure branch from 2fef77f to 443572b Compare August 30, 2026 19:02

graingert marked this pull request as draft August 30, 2026 19:48

fix on windows

9e7558d

graingert marked this pull request as ready for review August 30, 2026 20:31

Moved the changelog entry

fb4d188

agronholm requested changes Aug 31, 2026

View reviewed changes

Navigation Menu

Uh oh!

Fix SocketStream.send() writing to a paused transport after a cancelled send - #1299

Fix SocketStream.send() writing to a paused transport after a cancelled send#1299
graingert wants to merge 11 commits into
agronholm:masterfrom
graingert:socketstream-cancelled-send-backpressure

graingert commented Aug 29, 2026 •

edited

Loading

Uh oh!

agronholm left a comment

Uh oh!

agronholm Aug 31, 2026

Uh oh!

agronholm Aug 31, 2026

Uh oh!

agronholm Aug 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        where it is delivered anyway, so a cancelled ``send()`` must not have done so.
+        If a ``send()`` was cancelled after the data was written to the buffer, the
+        next call must ensure that the previous send completed one way or another
+        before attempting to send its own data.

Uh oh!

Conversation

graingert commented Aug 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Checklist

On the test

Uh oh!

agronholm left a comment

Choose a reason for hiding this comment

Uh oh!

agronholm Aug 31, 2026

Choose a reason for hiding this comment

Uh oh!

agronholm Aug 31, 2026

Choose a reason for hiding this comment

Uh oh!

agronholm Aug 31, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

graingert commented Aug 29, 2026 •

edited

Loading