Skip to content

Commit 7c88214

Browse files
cbbm142claude
authored andcommitted
Use select() to retry blocked socket writes instead of spinning or failing
When raw.write() returns None on a blocked non-blocking socket, use select() to wait up to SOCK_WRITE_TIMEOUT seconds for the socket to become writable before retrying. This avoids both the original silent buffer truncation bug and the infinite spin loop that the previous fix could produce. If the socket stays blocked past the timeout, raise BlockingIOError with the write buffer preserved. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent d3e8258 commit 7c88214

3 files changed

Lines changed: 87 additions & 16 deletions

File tree

cheroot/makefile.py

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,16 @@
22

33
# prefer slower Python-based io module
44
import _pyio as io
5+
import select
56
import socket
67

78

89
# Write only 16K at a time to sockets
910
SOCK_WRITE_BLOCKSIZE = 16384
1011

12+
# Seconds to wait for a blocked socket to become writable
13+
SOCK_WRITE_TIMEOUT = 10
14+
1115

1216
class BufferedWriter(io.BufferedWriter):
1317
"""Faux file object attached to a socket object."""
@@ -35,8 +39,20 @@ def _flush_unlocked(self):
3539
)
3640
except io.BlockingIOError as e:
3741
n = e.characters_written
38-
if n:
39-
del self._write_buf[:n]
42+
if n is None:
43+
_, writable, _ = select.select(
44+
[],
45+
[self.raw],
46+
[],
47+
SOCK_WRITE_TIMEOUT,
48+
)
49+
if not writable:
50+
raise io.BlockingIOError(
51+
0,
52+
'raw stream blocked; no bytes written',
53+
)
54+
continue
55+
del self._write_buf[:n]
4056

4157

4258
class StreamReader(io.BufferedReader):

cheroot/makefile.pyi

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import io
22

33
SOCK_WRITE_BLOCKSIZE: int
4+
SOCK_WRITE_TIMEOUT: int
45

56
class BufferedWriter(io.BufferedWriter):
67
def write(self, b): ...

cheroot/test/test_makefile.py

Lines changed: 68 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -54,43 +54,97 @@ def test_bytes_written():
5454

5555

5656
class _RawWriteBlockOnce:
57-
"""Mock raw.write() that returns None on the first call, then writes normally."""
57+
"""Mock raw.write() returning None once, then writing normally."""
5858

5959
def __init__(self):
6060
"""Initialize _RawWriteBlockOnce."""
6161
self.call_count = 0
6262
self.written = bytearray()
6363

6464
def __call__(self, chunk):
65-
"""Return None on first call to simulate a blocked socket write."""
65+
"""Return None on first call to simulate a blocked write."""
6666
self.call_count += 1
6767
if self.call_count == 1:
68-
return (
69-
None # simulates socket returning None on first blocked write
70-
)
68+
return None
7169
self.written.extend(chunk)
7270
return len(chunk)
7371

72+
def fileno(self):
73+
"""Return a fake fd for select()."""
74+
return -1
7475

75-
def test_flush_when_raw_write_returns_none():
76-
"""_flush_unlocked() must not treat None from raw.write() as a byte count.
7776

78-
io.RawIOBase.write() returns None when a non-blocking socket cannot accept
79-
data. del self._write_buf[:None] is equivalent to del self._write_buf[:]
80-
which silently clears the entire buffer, truncating the response without
81-
raising an exception.
77+
class _RawWriteBlockAlways:
78+
"""Mock raw.write() that always returns None."""
79+
80+
def __init__(self):
81+
"""Initialize _RawWriteBlockAlways."""
82+
self.call_count = 0
83+
84+
def __call__(self, chunk):
85+
"""Return None to simulate a permanently blocked socket."""
86+
self.call_count += 1
87+
88+
def fileno(self):
89+
"""Return a fake fd for select()."""
90+
return -1
91+
92+
93+
def test_flush_recovers_from_temporary_block(monkeypatch):
94+
"""_flush_unlocked() retries after select when raw.write() returns None.
95+
96+
A temporarily blocked socket should recover once select() reports
97+
the socket is writable again, delivering all buffered data.
8298
"""
83-
data = b'x' * (makefile.SOCK_WRITE_BLOCKSIZE * 2) # stress the write loop
99+
data = b'x' * (makefile.SOCK_WRITE_BLOCKSIZE * 2)
84100

85101
sock = MockSocket()
86102
wfile = makefile.MakeFile(sock, 'w')
87103
wfile._write_buf.extend(data)
88104

89105
mock = _RawWriteBlockOnce()
90106
wfile.raw.write = mock
107+
108+
# select() reports writable immediately
109+
monkeypatch.setattr(
110+
'cheroot.makefile.select.select',
111+
lambda _rlist, wlist, _xlist, _timeout: ([], wlist, []),
112+
)
91113
wfile._flush_unlocked()
92114

93115
assert bytes(mock.written) == data, (
94-
f'Expected {len(data)} bytes but only {len(mock.written)} reached raw.write(): '
95-
'buffer was silently discarded when raw.write() returned None'
116+
'all buffered data should be written after select retry'
117+
)
118+
119+
120+
def test_flush_raises_on_sustained_block(monkeypatch):
121+
"""_flush_unlocked() raises BlockingIOError after select timeout.
122+
123+
If the socket stays blocked past SOCK_WRITE_TIMEOUT, the write
124+
buffer must be preserved and BlockingIOError raised.
125+
"""
126+
import io
127+
128+
import pytest
129+
130+
data = b'x' * makefile.SOCK_WRITE_BLOCKSIZE
131+
132+
sock = MockSocket()
133+
wfile = makefile.MakeFile(sock, 'w')
134+
wfile._write_buf.extend(data)
135+
136+
mock = _RawWriteBlockAlways()
137+
wfile.raw.write = mock
138+
139+
# select() reports not writable (timeout)
140+
monkeypatch.setattr(
141+
'cheroot.makefile.select.select',
142+
lambda _rlist, _wlist, _xlist, _timeout: ([], [], []),
143+
)
144+
145+
with pytest.raises(io.BlockingIOError):
146+
wfile._flush_unlocked()
147+
148+
assert len(wfile._write_buf) == len(data), (
149+
'write buffer must be preserved when socket stays blocked'
96150
)

0 commit comments

Comments
 (0)