Jens Geyer created THRIFT-6183:
----------------------------------
Summary: Use the library-wide default frame size in
TNonblockingServer in the C++ library
Key: THRIFT-6183
URL: https://issues.apache.org/jira/browse/THRIFT-6183
Project: Thrift
Issue Type: Bug
Components: C++ - Library
Reporter: Jens Geyer
TNonblockingServer caps the frame it will accept from a connection at its own
MAX_FRAME_SIZE (lib/cpp/src/thrift/server/TNonblockingServer.h:127), which has
been 256 * 1024 * 1024 since THRIFT-1337 landed it in 2011 - years before
TConfiguration existed. The library-wide default is
TConfiguration::DEFAULT_MAX_FRAME_SIZE = 16384000, whose own comment reads
"this value is used consistently across all Thrift libraries".
The two disagree by a factor of 16:
TNonblockingServer::getMaxFrameSize() 268435456
TConfiguration::DEFAULT_MAX_FRAME_SIZE 16384000
TConfiguration appears zero times in TNonblockingServer.h and
TNonblockingServer.cpp, so the server neither reads a configured value nor
inherits the default. It starts from its own constant, and only
setMaxFrameSize() moves it.
That constant is what sizes the read buffer. TConnection::transition() resets
the read buffer to the frame length the peer declared before any payload byte
has arrived, so a four-byte length prefix at the server's own maximum decides
the allocation. Measured on a Linux x86-64 build, one connection sending
nothing but that prefix takes the process from a VmPeak of 145,880 kB to
604,636 kB. MAX_CONNECTIONS defaults to INT_MAX.
Other bindings do not have this gap. Java's AbstractNonblockingServer takes its
per-frame limit from trans_.getMaxFrameSize(), that is from TConfiguration, and
keeps a separate aggregate read-buffer budget on top of it.
The change points TNonblockingServer::MAX_FRAME_SIZE at
TConfiguration::DEFAULT_MAX_FRAME_SIZE. The constant is private, so this is not
an API change, and setMaxFrameSize() still overrides it.
Compatibility: this lowers a shipped default. A deployment that accepts frames
between 16,384,000 and 268,435,456 bytes on TNonblockingServer today and does
not call setMaxFrameSize() will start closing those connections. Needs a
release note.
Out of scope here, both worth their own tickets: TNonblockingServer still does
not consult TConfiguration at all, which is a larger design change; and
lib/cpp/src/thrift/transport/TBufferTransports.h:349 still carries a 256 MB
TFramedTransport::DEFAULT_MAX_FRAME_SIZE that nothing reads any more, which is
public API and is left alone here.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
