Repository navigation
Conversation
StreamReader wraps the socket with stdlib's socket.SocketIO, whose inherited _pyio.RawIOBase.read() allocates a bytearray, fills it via readinto(), then shrinks it in place with 'del b[n:]'. For SSL sockets, readinto() goes through ssl.SSLSocket.recv_into(), which creates a local memoryview over that same buffer. On CPython this view is released immediately via deterministic refcounting, but on PyPy buffer exports stay alive until the next GC run, so the subsequent in-place resize can randomly raise: BufferError: Existing exports of data: object cannot be re-sized Add _SocketIO, a thin socket.SocketIO subclass whose read() returns a sliced copy instead of resizing the buffer in place, avoiding the unsafe code path entirely. Opened by Claude, in a Cursor session, on behalf of @deronnax.
Mixed activityActivity patterns show a mix of organic and automated signals. Evidence
Last 5 PRs: This is an automated analysis by AgentScan |
❌ 1 Tests Failed:
View the full list of 1 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
Author
|
Hmmm, actually I don't like this fix (it dupplicates code). Claude keeps telling me that's the only way, but I think I can maybe come up with something better, wait a bit. |
Member
|
While in it, would you mind also not disregarding the PR template? |
Author
|
Closing this in favor of fixing it at the actual source: this was a PyPy bug, not something Really sorry for the PR template, I will pay attention in the future, should any further contributions be made 😬 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On PyPy,
StreamReader.read()(used for socket/SSL reads) can intermittently raise:Root cause
StreamReaderwraps the socket with stdlib'ssocket.SocketIO, whose inherited_pyio.RawIOBase.read()implementation allocates abytearray, fills it viareadinto(), then shrinks it in place withdel b[n:].For SSL sockets,
readinto()goes throughssl.SSLSocket.recv_into(), which creates a localmemoryviewover that same buffer. On CPython this view is released immediately via deterministic refcounting, so the following resize is safe. On PyPy, buffer exports stay alive until the next GC run instead of being released deterministically (documented PyPy/CPython difference), so the in-place resize can randomly fail while that view is still considered alive.This surfaces as a flaky PyPy-only CI failure for any project reading from SSL sockets through cheroot (e.g. Kozea/Radicale#2270, built on
cheroot, hits it inpypy-3.10/pypy-3.11CI).Why
_pyiocan't just be swapped for the acceleratediomakefile.pyhas imported_pyioinstead ofiosince 2015 (originally incherrypy/wsgiserver3.py), with only the comment "prefer slower Python-based io module" and no written rationale. Digging through history:BufferedWriter.write()/_flush_unlocked()andStreamReader.has_data()directly access private attributes (_write_buf,_write_lock,_read_buf,_read_pos) that only exist on the pure-Python_pyioimplementation; CPython's C-acceleratediomodule keeps equivalent state in C, not exposed to subclasses. So swapping toioisn't an option as long as those overrides exist.I also verified on a real PyPy 3.12 install that forcing
ioinstead of_pyiodoes not avoid the bug anyway: PyPy'siomodule is itself pure-Python (no separate C accelerator), so it hits the exact same code path.Fix
Add
_SocketIO, a thinsocket.SocketIOsubclass used byStreamReader, whoseread()returns a sliced copy (bytes(b[:n])) instead of resizing the buffer in place. This avoids the unsafe code path entirely, with no behavior change on CPython.Testing
test_socket_io_read_does_not_resize_buffer) that keeps amemoryviewalive across theread()call (simulating the PyPy situation) and asserts noBufferErroris raised. Verified it fails against the previous implementation and passes with the fix, on CPython.flake8clean.Bug, PR text body and solution done by Claude Sonnet 5.5 Medium, all reviewed by me (deronnax)