Jens-G opened a new pull request, #3847:
URL: https://github.com/apache/thrift/pull/3847
`TNonblockingServer`'s `TConnection` reserved the whole declared frame the
moment it read the four-byte length prefix: `transition()` reallocated the read
buffer to the frame size before any payload byte had arrived, so a peer could
make the server reserve up to `getMaxFrameSize()` bytes per connection by
sending only a four-byte header. (The reservation is address space — a
`std::realloc` — rather than resident memory, but it is still an unamplified
header-to-allocation step.)
This bounds the allocation by what the peer actually sends:
- `transition()` reserves only enough to hold the length prefix and begin
reading (the prefix plus about a kilobyte), not the declared frame.
- The buffer grows toward the frame size in `SOCKET_RECV` as bytes are
actually read — by the same doubling the up-front allocation used, now driven
by what the peer sends, with the doubling guarded so it cannot overflow
`uint32_t`.
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 no change at all
for frames of a kilobyte or less. A header with no payload reserves next to
nothing. A large frame that does arrive now pays the amortized doubling copy
any grow-as-you-go buffer pays.
The `uint32_t` overflow guard in the new `growReadBuffer` helper is a small
addition the old inline doubling lacked (the old `newSize *= 2` would loop
forever for a `maxFrameSize` above 2 GiB); it is included here — happy to split
it to its own ticket if a reviewer prefers.
### Test
Three cases in `TNonblockingServerTest`: a Linux-only, forked case asserts
that a 256 MiB frame header with no body does not grow the server's `VmSize`;
two portable cases drive multi-megabyte requests that are assembled over many
libevent callbacks (the buffer doubling as bytes arrive) and must round-trip
byte-for-byte, one freeing the grown buffer at connection close and one between
frames on a live connection.
### Related
This is part (4) of the nonblocking-server frame handling; part (2)+(3),
wiring the server to `TConfiguration`, is **THRIFT-6242**. The two are
independent (not stacked) but both touch `TNonblockingServer.{h,cpp}`, so
whichever lands second will need a small rebase.
---
Prepared with AI assistance (Claude Opus 4.8).
--
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]
