You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by jk...@apache.org on 2017/10/06 12:23:45 UTC
thrift git commit: THRIFT-4331: C++ TSSLSocket fixes for huge message
handling Client: C++
Repository: thrift
Updated Branches:
refs/heads/master 39310dad7 -> 9f9e30b51
THRIFT-4331: C++ TSSLSocket fixes for huge message handling
Client: C++
fixed issue with large messages, where waitForEvent was called
mutliple times waiting for SSL_read() to get bytes and running
in the retry timeout.
fixed issue where poll was not using the right flags.
This fixes #1363
Project: http://git-wip-us.apache.org/repos/asf/thrift/repo
Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/9f9e30b5
Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/9f9e30b5
Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/9f9e30b5
Branch: refs/heads/master
Commit: 9f9e30b51e3912c0b63258badf5501d3cb2550be
Parents: 39310da
Author: Martin Haimberger <ma...@thincast.com>
Authored: Fri Oct 6 09:57:27 2017 +0200
Committer: James E. King, III <jk...@apache.org>
Committed: Fri Oct 6 05:22:13 2017 -0700
----------------------------------------------------------------------
lib/cpp/src/thrift/transport/TSSLSocket.cpp | 28 +++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/thrift/blob/9f9e30b5/lib/cpp/src/thrift/transport/TSSLSocket.cpp
----------------------------------------------------------------------
diff --git a/lib/cpp/src/thrift/transport/TSSLSocket.cpp b/lib/cpp/src/thrift/transport/TSSLSocket.cpp
index acf23aa..ddefb34 100644
--- a/lib/cpp/src/thrift/transport/TSSLSocket.cpp
+++ b/lib/cpp/src/thrift/transport/TSSLSocket.cpp
@@ -295,8 +295,9 @@ bool TSSLSocket::peek() {
}
case SSL_ERROR_WANT_READ:
case SSL_ERROR_WANT_WRITE:
- waitForEvent(error == SSL_ERROR_WANT_READ);
- continue;
+ // in the case of SSL_ERROR_SYSCALL we want to wait for an read event again
+ waitForEvent(error != SSL_ERROR_WANT_WRITE);
+ continue;
default:;// do nothing
}
string errors;
@@ -338,6 +339,7 @@ void TSSLSocket::close() {
}
case SSL_ERROR_WANT_READ:
case SSL_ERROR_WANT_WRITE:
+ // in the case of SSL_ERROR_SYSCALL we want to wait for an write/read event again
waitForEvent(error == SSL_ERROR_WANT_READ);
rc = 2;
default:;// do nothing
@@ -387,6 +389,7 @@ uint32_t TSSLSocket::read(uint8_t* buf, uint32_t len) {
break;
}
int32_t errno_copy = THRIFT_GET_SOCKET_ERROR;
+ unsigned int waitEventReturn;
switch (error) {
case SSL_ERROR_SYSCALL:
if ((errno_copy != THRIFT_EINTR)
@@ -408,7 +411,8 @@ uint32_t TSSLSocket::read(uint8_t* buf, uint32_t len) {
}
throw TTransportException(TTransportException::INTERNAL_ERROR, "too much recv retries");
}
- else if (waitForEvent(error == SSL_ERROR_WANT_READ) == TSSL_EINTR ) {
+ // in the case of SSL_ERROR_SYSCALL we want to wait for an read event again
+ else if ((waitEventReturn = waitForEvent(error != SSL_ERROR_WANT_WRITE)) == TSSL_EINTR ) {
// repeat operation
if (readRetryCount_ < maxRecvRetries_) {
// THRIFT_EINTR needs to be handled manually and we can tolerate
@@ -417,7 +421,15 @@ uint32_t TSSLSocket::read(uint8_t* buf, uint32_t len) {
}
throw TTransportException(TTransportException::INTERNAL_ERROR, "too much recv retries");
}
- continue;
+ else if (waitEventReturn == TSSL_DATA) {
+ // in case of SSL and huge thrift packets, there may be a number of
+ // socket operations, before any data becomes available by SSL_read().
+ // Therefore the number of retries should not be increased and
+ // the operation should be repeated.
+ readRetryCount_--;
+ continue;
+ }
+ throw TTransportException(TTransportException::INTERNAL_ERROR, "unkown waitForEvent return value");
default:;// do nothing
}
string errors;
@@ -451,6 +463,7 @@ void TSSLSocket::write(const uint8_t* buf, uint32_t len) {
return;
}
else {
+ // in the case of SSL_ERROR_SYSCALL we want to wait for an write event again
waitForEvent(error == SSL_ERROR_WANT_READ);
continue;
}
@@ -494,6 +507,7 @@ uint32_t TSSLSocket::write_partial(const uint8_t* buf, uint32_t len) {
return 0;
}
else {
+ // in the case of SSL_ERROR_SYSCALL we want to wait for an write event again
waitForEvent(error == SSL_ERROR_WANT_READ);
continue;
}
@@ -579,6 +593,7 @@ void TSSLSocket::initializeHandshake() {
}
else {
// repeat operation
+ // in the case of SSL_ERROR_SYSCALL we want to wait for an write/read event again
waitForEvent(error == SSL_ERROR_WANT_READ);
rc = 2;
}
@@ -610,6 +625,7 @@ void TSSLSocket::initializeHandshake() {
}
else {
// repeat operation
+ // in the case of SSL_ERROR_SYSCALL we want to wait for an write/read event again
waitForEvent(error == SSL_ERROR_WANT_READ);
rc = 2;
}
@@ -759,7 +775,9 @@ unsigned int TSSLSocket::waitForEvent(bool wantRead) {
struct THRIFT_POLLFD fds[2];
memset(fds, 0, sizeof(fds));
fds[0].fd = fdSocket;
- fds[0].events = wantRead ? THRIFT_POLLIN : THRIFT_POLLOUT;
+ // use POLLIN also on write operations too, this is needed for operations
+ // which requires read and write on the socket.
+ fds[0].events = wantRead ? THRIFT_POLLIN : THRIFT_POLLIN | THRIFT_POLLOUT;
if (interruptListener_) {
fds[1].fd = *(interruptListener_.get());