Jens Geyer created THRIFT-6243:
----------------------------------
Summary: Grow the TNonblockingServer read buffer as the payload
arrives, not on the frame header
Key: THRIFT-6243
URL: https://issues.apache.org/jira/browse/THRIFT-6243
Project: Thrift
Issue Type: Bug
Components: C++ - Library
Reporter: Jens Geyer
{{TNonblockingServer}}'s {{TConnection}} reserves the whole declared frame the
moment it reads the four-byte length prefix: {{transition()}} in the
{{APP_READ_FRAME_SIZE}} state reallocates the read buffer to the frame size
({{TNonblockingServer.cpp}}) before any payload byte has arrived. A peer can
therefore make the server reserve up to {{getMaxFrameSize()}} bytes per
connection by sending only a four-byte header, with {{MAX_CONNECTIONS}}
defaulting to {{INT_MAX}}. The reservation is address space rather than
resident memory (it is a {{std::realloc}}), but it is still an unamplified
header-to-allocation step.
This is part (4) of the nonblocking-server frame handling. THRIFT-6183 lowered
the default limit; a separate change wires the server to a {{TConfiguration}}
and adds a pre-allocation budget check. This change 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 the {{SOCKET_RECV}} state as bytes
are actually read -- by the same doubling the up-front allocation used, now
driven by what the peer sends rather than by the size it declares, with the
doubling guarded so it cannot overflow {{uint32_t}}.
A frame that fully arrives ends at the same buffer size as before (the next
power of two at or above the frame size), so there is no change for legitimate
traffic in the steady state and no change at all for frames of a kilobyte or
less (still a single allocation). A header with no payload reserves next to
nothing.
Tradeoff, for the reviewer: 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 {{memmove}} over the life of the connection, against a
single allocation before. This is the cost of not trusting the declared size,
and the same shape the scan that raised this recommended ("allocate as the
payload arrives, not on the header").
Test: TNonblockingServerTest gains three cases. A Linux-only, forked case
announces a 256 MiB frame, sends none of it, and asserts the server's VmSize
does not grow by more than a small slack -- it reads VmSize rather than RSS
because the reservation is committed, not resident; it fails when the buffer is
reserved on the header and passes when it grows with the payload. Two portable
cases drive multi-megabyte requests through the framed client -- each frame far
larger than the initial reservation, so it is assembled over many libevent
callbacks with the buffer doubling as the bytes arrive, and every request must
round-trip byte-for-byte. One frees the grown buffer when the connection closes
(via {{returnConnection}}) and regrows it on the next connection; the other
forces the resize counter to 1 so the buffer is freed between two frames on a
single live connection and regrown from empty mid-stream. Together they cover
the grow, the free, and the regrow-from-empty paths, so a lost byte or a
mishandled empty buffer shows up as a mismatch.
_Drafted with AI assistance (Claude Opus 4.8)._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
