[
https://issues.apache.org/jira/browse/THRIFT-6243?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer resolved THRIFT-6243.
--------------------------------
Fix Version/s: 0.25.0
Assignee: Jens Geyer
Resolution: Fixed
> 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
> Assignee: Jens Geyer
> Priority: Major
> Fix For: 0.25.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> {{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)
