Jens-G opened a new pull request, #3865:
URL: https://github.com/apache/thrift/pull/3865

   JIRA: [THRIFT-6267](https://issues.apache.org/jira/browse/THRIFT-6267)
   Client: py
   
   `Connection.read()` in `lib/py/src/server/TNonblockingServer.py` did 
`self._rbuf += read` on a `bytes` object. A `bytes` object cannot grow, so each 
append copied everything received so far, and a frame that arrives in n pieces 
costs O(n²) bytes of copying. Measured with the new test: a 1 MiB frame in 4 
KiB pieces copied 135,790,596 bytes.
   
   ### Change
   
   - `_rbuf` is a `bytearray`, so appending grows it in place. The consumed 
frame is removed with `del self._rbuf[:end]`.
   - Each message gets `bytes(self._rbuf[:end])`, a copy of exactly its own 
frame. The processing thread still builds `TMemoryBuffer(msg.buffer, 
msg.offset)` as before.
   
   **One difference in behaviour:** a message used to receive the whole buffer 
as it stood. When further bytes arrived in the same read, the transport the 
processor read from therefore continued past the end of its frame into the next 
one. Now it ends at the frame. What follows a frame stays in the buffer for the 
next message, as before.
   
   ### Tests
   
   New `lib/py/test/test_nonblocking_server_read_buffer.py`, registered in both 
lists in `lib/py/Makefile.am`. It uses a socket stand-in that hands out one 
piece per `read()`, the way one `select()` wakeup would.
   
   - **Counting test:** a 1 MiB frame in 257 pieces has to be collected copying 
at most twice its size. The pieces are a `bytes` subclass whose `__radd__` 
counts what `buffer + piece` copies. Growing a `bytearray` is not counted.
   - **Frame boundary:** two frames in one read; each message yields its own 
payload and then nothing.
   - **Split length:** a length word split across three reads.
   - **Remainder:** a frame followed by the start of the next one; the rest 
arrives after the first message was handled.
   
   Against the unmodified server:
   - the counting test fails with 135,790,596 bytes copied against a limit of 
2,097,160;
   - the boundary test fails, because the first message's transport goes on 
with `\x00\x00\x00\x0ethe second one`;
   - the remainder test fails for the same reason.
   
   Mutations, each on its own:
   - **Collecting through `bytes(...) + read`:** the counting test fails.
   - **Handing the message the buffer itself** instead of a copy: all four 
tests fail, because the later `del` empties it.
   - **Copying the whole buffer** instead of the frame: both boundary tests 
fail.
   
   Also run, with the fix:
   - `test_nonblocking_server_read_buffer.py` and 
`test_nonblocking_server_frame_size.py` pass on Python 3.10, 3.12 and 3.14;
   - `thrift_TNonblockingServer.py`, a server with a real client, passes on 
3.10;
   - flake8 is clean.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to