Skip to content

Commit 544f230

Browse files
graingertclaude
andcommitted
don't hand data to a paused transport after a cancelled send
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>
1 parent b05fe6d commit 544f230

3 files changed

Lines changed: 81 additions & 0 deletions

File tree

docs/versionhistory.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,12 @@ This library adheres to `Semantic Versioning 2.0 <http://semver.org/>`_.
8484
- Fixed asyncio task groups leaking unawaited coroutines when a custom task constructor
8585
fails; default task creation is unaffected
8686
(`#1274 <https://github.com/agronholm/anyio/issues/1274>`_; PR by @dsfaccini)
87+
- Fixed ``SocketStream.send()`` on the asyncio backend handing its data to a paused
88+
transport after a previous ``send()`` was cancelled, which appended it to the
89+
transport's write buffer without ever offering it to the operating system. Repeated
90+
cancellation could therefore grow that buffer without bound, despite its zero high
91+
water mark
92+
(`#1299 <https://github.com/agronholm/anyio/pull/1299>`_; PR by @graingert)
8793

8894
**4.14.2**
8995

src/anyio/_backends/_asyncio.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1385,6 +1385,11 @@ async def send(self, item: bytes) -> None:
13851385
with self._send_guard:
13861386
await AsyncIOBackend.checkpoint()
13871387

1388+
# Wait until the transport has flushed anything the OS previously refused,
1389+
# so that this data is not merely appended to the transport's buffer (which
1390+
# would happen if a previous send() was cancelled while waiting below):
1391+
# write() never offers anything to the OS while its buffer is non-empty
1392+
await self._protocol.write_event.wait()
13881393
if self._closed:
13891394
raise ClosedResourceError
13901395
elif self._protocol.exception is not None:

tests/test_sockets.py

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,76 @@ async def test_extra_attributes(
251251
assert stream.extra(SocketAttribute.remote_address) == server_addr
252252
assert stream.extra(SocketAttribute.remote_port) == server_addr[1]
253253

254+
@pytest.mark.parametrize("anyio_backend", asyncio_params)
255+
async def test_cancelled_send_does_not_buffer_the_next_one(
256+
self, server_sock: socket.socket, server_addr: tuple[str, int]
257+
) -> None:
258+
"""
259+
A ``send()`` cancelled while blocked must not let the next one skip ahead.
260+
261+
The transport's write buffer high water mark is 0, so the protocol is paused as
262+
soon as the transport has had to buffer anything the OS would not take. The
263+
next ``send()`` must wait for that buffer to be flushed rather than append to
264+
it: ``transport.write()`` never offers anything to the OS while its buffer is
265+
non-empty, so otherwise repeated cancellation would grow the buffer without
266+
bound, despite the zero high water mark.
267+
"""
268+
# A small receive buffer on the listening socket is inherited by the accepted
269+
# one, so that the sender stays backed up while nothing is being read
270+
server_sock.setsockopt(socket.SOL_SOCKET, socket.SO_RCVBUF, 2048)
271+
async with await connect_tcp(*server_addr) as stream:
272+
client, _ = server_sock.accept()
273+
with client:
274+
stream.extra(SocketAttribute.raw_socket).setsockopt(
275+
socket.SOL_SOCKET, socket.SO_SNDBUF, 2048
276+
)
277+
transport = cast(Any, stream)._transport
278+
protocol = cast(Any, stream)._protocol
279+
280+
async def send_forever() -> None:
281+
while True:
282+
await stream.send(b"x" * 4096)
283+
284+
# Cancel a send once the transport has had to buffer something, so
285+
# that it is left paused with a non-empty buffer
286+
async with create_task_group() as tg:
287+
tg.start_soon(send_forever)
288+
with fail_after(10):
289+
while not transport.get_write_buffer_size():
290+
await checkpoint()
291+
292+
tg.cancel_scope.cancel()
293+
294+
assert transport.get_write_buffer_size()
295+
assert not protocol.write_event.is_set()
296+
297+
# Record whether the next send() hands anything over while the
298+
# transport is still paused. The paused state has to be sampled at the
299+
# moment of the write, as the transport may legitimately be resumed
300+
# (and then paused again) while the sender is waiting.
301+
writes_while_paused = 0
302+
303+
class SpyTransport:
304+
def __getattr__(self, name: str) -> Any:
305+
return getattr(transport, name)
306+
307+
def write(self, data: bytes) -> None:
308+
nonlocal writes_while_paused
309+
if not protocol.write_event.is_set():
310+
writes_while_paused += 1
311+
312+
transport.write(data)
313+
314+
cast(Any, stream)._transport = SpyTransport()
315+
async with create_task_group() as tg:
316+
tg.start_soon(stream.send, b"y" * 4096)
317+
await wait_all_tasks_blocked()
318+
tg.cancel_scope.cancel()
319+
320+
assert not writes_while_paused, (
321+
"send() handed its data to a still-paused transport"
322+
)
323+
254324
async def test_send_receive(
255325
self, server_sock: socket.socket, server_addr: tuple[str, int]
256326
) -> None:

0 commit comments

Comments
 (0)