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
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 @@
}
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 @@
}
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 @@
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 @@
}
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 @@
}
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 @@
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 @@
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 @@
}
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 @@
}
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 @@
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());