diff options
author | Martin Haimberger <martin.haimberger@thincast.com> | 2017-10-06 09:57:27 +0200 |
---|---|---|
committer | James E. King, III <jking@apache.org> | 2017-10-06 05:22:13 -0700 |
commit | 9f9e30b51e3912c0b63258badf5501d3cb2550be (patch) | |
tree | 0874eb042137523579e01026ac3072235afcf365 | |
parent | 39310dad793ca69b4b7217a3b54430e682e5e2a4 (diff) | |
download | thrift-9f9e30b51e3912c0b63258badf5501d3cb2550be.tar.gz |
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
-rw-r--r-- | lib/cpp/src/thrift/transport/TSSLSocket.cpp | 28 |
1 files changed, 23 insertions, 5 deletions
diff --git a/lib/cpp/src/thrift/transport/TSSLSocket.cpp b/lib/cpp/src/thrift/transport/TSSLSocket.cpp index acf23aa9c..ddefb34c9 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()); |