Github user bgedik commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1497#discussion_r172752961
--- Diff: lib/cpp/src/thrift/server/TNonblockingServer.cpp ---
@@ -472,6 +472,18 @@ void TNonblockingServer::TConnection::workSocket() {
}
// size known; now get the rest of the frame
transition();
+
+ // If the socket has more data than the frame header, continue to work
on it. This is not strictly necessary for
+ // regular sockets, because if there is more data, libevent will fire
the event handler registered for read
+ // readiness, which will in turn call workSocket(). However, some
socket types (such as TSSLSocket) may have the
+ // data sitting in their internal buffers and from libevent's
perspective, there is no further data available. In
+ // that case, not having this workSocket() call here would result in a
hang as we will never get to work the socket,
+ // despite having more data.
+ if (tSocket_->hasPendingDataToRead())
--- End diff --
It should be an ``if``. At this point, we know the frame size. The next
step is to read the frame data itself. And for that, there is already logic to
work the socket until all the data is read. For SSL specifically, there is no
concern that the SSL buffers can keep pending data forever, because when the
entire data frame arrives, there won't be any incomplete SSL bytes. In any
case, the code should work generically for all socket types.
---
