Jens-G opened a new pull request, #3848: URL: https://github.com/apache/thrift/pull/3848
`TNonblockingServer` sizes the connection read buffer from the length the peer declared, in `transition()`'s `ConnectionState.READ_FRAME_SIZE` case, as soon as the four length bytes have been read and before any payload byte has arrived. A connection that announces a large frame and then sends nothing holds that buffer, rounded up to the next power of two, for as long as it stays open — and `DEFAULT_MAX_CONNECTIONS` is `int.max`. This is the D counterpart of THRIFT-6243, which made the same change in C++. THRIFT-6241 lowered D's default frame size to the library-wide value; this bounds the allocation by what the peer actually sends. ### What changed - `transition()` reserves about a kilobyte to begin reading, rather than the declared frame. - The buffer grows toward the frame size in `SocketState.RECV` as bytes actually arrive, by the same doubling the up-front allocation used. - The doubling moves into a `growReadBuffer()` helper that also stops before overflowing `size_t`; the old in-line loop would have spun forever on a 32-bit `size_t` with `maxFrameSize` raised past 2 GiB. **One part is not a straight translation of the C++ change, and is worth a reviewer's eye.** The socket read was slicing `readBuffer_[readBufferPos_ .. readWant_]`, which only ever stayed inside the allocation *because* the whole frame had been reserved first. Once the buffer can be smaller than the frame, that slice runs past the end of it — and D does not bounds-check a slice taken from a raw pointer, even in a debug build. The read is therefore capped at `min(readBufferSize_, readWant_)`. The buffer can also be *larger* than the frame, whether because doubling overshot or because it was kept from an earlier, larger request, and bytes past `readWant_` belong to the next frame. A frame that fully arrives ends at the same buffer size as before, so there is no change for legitimate traffic in the steady state and none at all for frames of a kilobyte or less. The initial reservation is deliberately `DEFAULT_IDLE_READ_BUFFER_LIMIT`, so a connection carrying only small frames settles at exactly the size `checkIdleBufferLimit()` lets an idle connection keep and is not freed and regrown between requests. **Tradeoff:** a large frame that does arrive now pays the amortized doubling copy that any grow-as-you-go buffer pays — on the order of twice the frame size in copying over the life of the connection, against a single allocation before. That is the cost of not trusting the declared size. ### Tests Two cases in `nonblocking.d`, both written before the change and both proven two-state: - A header declaring 64 MiB followed by no payload leaves the connection's read buffer at a kilobyte. Before the change it was 67108864 bytes. The test closes the client so the connection goes back on the idle stack, where its buffer can be read without racing the I/O thread, with `idleReadBufferLimit` set to 0 so disposal does not free it first. - Two 2 MiB requests over one connection, each filled with a position-dependent pattern and checked against a hash the server computes and returns. Each frame is far larger than the initial reservation, so it is assembled over many libevent callbacks while the buffer doubles, and it has to round-trip byte for byte. Neutralising either half of the change fails: restoring the full-frame reservation puts the first case back to 67108864 bytes, and restoring the uncapped slice aborts the second with `free(): corrupted unsorted chunks`. The existing frame-size unittest's server harness is hoisted to `version(unittest)` module scope, unchanged, so both tests can use it. Verified with dmd 2.087.0, debug and release, three runs each, plus a clean `-w -wi` library build of `lib/d/src` in both modes. Note that no CI job builds D, so this local run is the only coverage. --- 🤖 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]
