From 9b20955b2d4f651eaaec63736cb8aeaaed011be7 Mon Sep 17 00:00:00 2001 From: David Reiss Date: Tue, 8 Apr 2008 06:26:05 +0000 Subject: [PATCH] Thrift error logging improvements Summary: - Move strerror_s to Thrift.h (was previously in TTransportException.h) - Capture errno as soon as syscall returns failure and make it part of error message. - Cleaned up several instances of the wrong error value being printed. - More consistently pass the errno in the TTransport Exception - Add more consistent error logging for the various transport failure modes Reviewed By: dreiss Test Plan: - compile everything. - test on search tier Revert: OK TracCamp Project: Thrift DiffCamp Revision: 11077 git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@665648 13f79535-47bb-0310-9956-ffa450edef68 --- lib/cpp/src/Thrift.cpp | 25 ++++++ lib/cpp/src/Thrift.h | 13 ++- lib/cpp/src/server/TNonblockingServer.cpp | 24 +++-- lib/cpp/src/server/TNonblockingServer.h | 5 +- lib/cpp/src/transport/TFileTransport.cpp | 36 +++++--- lib/cpp/src/transport/TServerSocket.cpp | 87 +++++++++++++------ lib/cpp/src/transport/TSocket.cpp | 77 +++++++++------- lib/cpp/src/transport/TSocket.h | 1 - lib/cpp/src/transport/TTransportException.cpp | 26 +----- lib/cpp/src/transport/TTransportException.h | 2 +- 10 files changed, 185 insertions(+), 111 deletions(-) diff --git a/lib/cpp/src/Thrift.cpp b/lib/cpp/src/Thrift.cpp index 88890d49..963f9389 100644 --- a/lib/cpp/src/Thrift.cpp +++ b/lib/cpp/src/Thrift.cpp @@ -11,6 +11,31 @@ namespace facebook { namespace thrift { TOutput GlobalOutput; +std::string TOutput::strerror_s(int errno_copy) { +#ifndef HAVE_STRERROR_R + return "errno = " + lexical_cast(errno_copy); +#else // HAVE_STRERROR_R + + char b_errbuf[1024] = { '\0' }; +#ifdef STRERROR_R_CHAR_P + char *b_error = strerror_r(errno_copy, b_errbuf, sizeof(b_errbuf)); +#else + char *b_error = b_errbuf; + int rv = strerror_r(errno_copy, b_errbuf, sizeof(b_errbuf)); + if (rv == -1) { + // strerror_r failed. omgwtfbbq. + return "XSI-compliant strerror_r() failed with errno = " + + lexical_cast(errno_copy); + } +#endif + // Can anyone prove that explicit cast is probably not necessary + // to ensure that the string object is constructed before + // b_error becomes invalid? + return std::string(b_error); + +#endif // HAVE_STRERROR_R +} + uint32_t TApplicationException::read(facebook::thrift::protocol::TProtocol* iprot) { uint32_t xfer = 0; std::string fname; diff --git a/lib/cpp/src/Thrift.h b/lib/cpp/src/Thrift.h index 8e93bb0b..6eb20bb8 100644 --- a/lib/cpp/src/Thrift.h +++ b/lib/cpp/src/Thrift.h @@ -21,6 +21,8 @@ #include #include #include +#include +#include #include "TLogging.h" @@ -28,7 +30,7 @@ namespace facebook { namespace thrift { class TOutput { public: - TOutput() : f_(&perrorTimeWrapper) {} + TOutput() : f_(&errorTimeWrapper) {} inline void setOutputFunction(void (*function)(const char *)){ f_ = function; @@ -38,15 +40,18 @@ class TOutput { f_(message); } - inline static void perrorTimeWrapper(const char* msg) { + inline static void errorTimeWrapper(const char* msg) { time_t now; char dbgtime[25]; time(&now); ctime_r(&now, dbgtime); dbgtime[24] = 0; - fprintf(stderr, "%s ", dbgtime); - perror(msg); + fprintf(stderr, "Thrift: %s %s\n", dbgtime, msg); } + + /** Just like strerror_r but returns a C++ string object. */ + static std::string strerror_s(int errno_copy); + private: void (*f_)(const char *); }; diff --git a/lib/cpp/src/server/TNonblockingServer.cpp b/lib/cpp/src/server/TNonblockingServer.cpp index c840bcc8..ebe85a6a 100644 --- a/lib/cpp/src/server/TNonblockingServer.cpp +++ b/lib/cpp/src/server/TNonblockingServer.cpp @@ -50,10 +50,12 @@ class TConnection::Task: public Runnable { // Signal completion back to the libevent thread via a socketpair int8_t b = 0; if (-1 == send(taskHandle_, &b, sizeof(int8_t), 0)) { - GlobalOutput("TNonblockingServer::Task: send"); + string errStr = "TNonblockingServer::Task: send " + TOutput::strerror_s(errno); + GlobalOutput(errStr.c_str()); } if (-1 == ::close(taskHandle_)) { - GlobalOutput("TNonblockingServer::Task: close, possible resource leak"); + string errStr = "TNonblockingServer::Task: close, possible resource leak " + TOutput::strerror_s(errno); + GlobalOutput(errStr.c_str()); } } @@ -139,7 +141,8 @@ void TConnection::workSocket() { } if (errno != ECONNRESET) { - GlobalOutput("TConnection::workSocket() recv -1"); + string errStr = "TConnection::workSocket() recv -1 " + TOutput::strerror_s(errno); + GlobalOutput(errStr.c_str()); } } @@ -175,7 +178,8 @@ void TConnection::workSocket() { return; } if (errno != EPIPE) { - GlobalOutput("TConnection::workSocket() send -1"); + string errStr = "TConnection::workSocket() send -1 " + TOutput::strerror_s(errno); + GlobalOutput(errStr.c_str()); } close(); return; @@ -221,7 +225,8 @@ void TConnection::transition() { // We are setting up a Task to do this work and we will wait on it int sv[2]; if (-1 == socketpair(AF_LOCAL, SOCK_STREAM, 0, sv)) { - GlobalOutput("TConnection::socketpair() failed"); + string errStr = "TConnection::socketpair() failed " + TOutput::strerror_s(errno); + GlobalOutput(errStr.c_str()); // Now we will fall through to the APP_WAIT_TASK block with no response } else { // Create task and dispatch to the thread manager @@ -519,7 +524,8 @@ void TNonblockingServer::handleEvent(int fd, short which) { int flags; if ((flags = fcntl(clientSocket, F_GETFL, 0)) < 0 || fcntl(clientSocket, F_SETFL, flags | O_NONBLOCK) < 0) { - GlobalOutput("thriftServerEventHandler: set O_NONBLOCK"); + string errStr = "thriftServerEventHandler: set O_NONBLOCK (fcntl) " + TOutput::strerror_s(errno); + GlobalOutput(errStr.c_str()); close(clientSocket); return; } @@ -542,7 +548,8 @@ void TNonblockingServer::handleEvent(int fd, short which) { // Done looping accept, now we have to make sure the error is due to // blocking. Any other error is a problem if (errno != EAGAIN && errno != EWOULDBLOCK) { - GlobalOutput("thriftServerEventHandler: accept()"); + string errStr = "thriftServerEventHandler: accept() " + TOutput::strerror_s(errno); + GlobalOutput(errStr.c_str()); } } @@ -564,7 +571,8 @@ void TNonblockingServer::listenSocket() { // Wildcard address error = getaddrinfo(NULL, port, &hints, &res0); if (error) { - GlobalOutput("TNonblockingServer::serve() getaddrinfo"); + string errStr = "TNonblockingServer::serve() getaddrinfo " + string(gai_strerror(error)); + GlobalOutput(errStr.c_str()); return; } diff --git a/lib/cpp/src/server/TNonblockingServer.h b/lib/cpp/src/server/TNonblockingServer.h index 89c69a00..7c6bc7fa 100644 --- a/lib/cpp/src/server/TNonblockingServer.h +++ b/lib/cpp/src/server/TNonblockingServer.h @@ -12,6 +12,8 @@ #include #include #include +#include +#include #include #include @@ -320,7 +322,8 @@ class TConnection { static void taskHandler(int fd, short /* which */, void* v) { assert(fd == ((TConnection*)v)->taskHandle_); if (-1 == ::close(((TConnection*)v)->taskHandle_)) { - GlobalOutput("TConnection::taskHandler close handle failed, resource leak"); + std::string errStr = "TConnection::taskHandler close handle failed, resource leak " + TOutput::strerror_s(errno); + GlobalOutput(errStr.c_str()); } ((TConnection*)v)->transition(); } diff --git a/lib/cpp/src/transport/TFileTransport.cpp b/lib/cpp/src/transport/TFileTransport.cpp index a4b275f3..41c60050 100644 --- a/lib/cpp/src/transport/TFileTransport.cpp +++ b/lib/cpp/src/transport/TFileTransport.cpp @@ -101,8 +101,10 @@ void TFileTransport::resetOutputFile(int fd, string filename, int64_t offset) { flush(); fprintf(stderr, "error, current file (%s) not closed\n", filename_.c_str()); if (-1 == ::close(fd_)) { - GlobalOutput("TFileTransport: error in file close"); - throw TTransportException("TFileTransport: error in file close"); + int errno_copy = errno; + string errStr = "TFileTransport: resetOutputFile() ::close() " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); + throw TTransportException(TTransportException::UNKNOWN, "TFileTransport: error in file close", errno_copy); } } @@ -159,7 +161,9 @@ TFileTransport::~TFileTransport() { // close logfile if (fd_ > 0) { if(-1 == ::close(fd_)) { - GlobalOutput("TFileTransport: error in file close"); + int errno_copy = errno; + string errStr = "TFileTransport: ~TFileTransport() ::close() " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); } } } @@ -315,8 +319,10 @@ void TFileTransport::writerThread() { // empty out both the buffers if (enqueueBuffer_->isEmpty() && dequeueBuffer_->isEmpty()) { if (-1 == ::close(fd_)) { - GlobalOutput("TFileTransport: error in close"); - throw TTransportException("TFileTransport: error in file close"); + int errno_copy = errno; + string errStr = "TFileTransport: writerThread() ::close() " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); + throw TTransportException(TTransportException::UNKNOWN, "TFileTransport: error in file close", errno_copy); } // just be safe and sync to disk fsync(fd_); @@ -362,8 +368,10 @@ void TFileTransport::writerThread() { uint8_t zeros[padding]; bzero(zeros, padding); if (-1 == ::write(fd_, zeros, padding)) { - GlobalOutput("TFileTransport: error while padding zeros"); - throw TTransportException("TFileTransport: error while padding zeros"); + int errno_copy = errno; + string errStr = "TFileTransport: writerThread() error while padding zeros " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); + throw TTransportException(TTransportException::UNKNOWN, "TFileTransport: error while padding zeros", errno_copy); } unflushed += padding; offset_ += padding; @@ -373,8 +381,10 @@ void TFileTransport::writerThread() { // write the dequeued event to the file if (outEvent->eventSize_ > 0) { if (-1 == ::write(fd_, outEvent->eventBuff_, outEvent->eventSize_)) { - GlobalOutput("TFileTransport: error while writing event"); - throw TTransportException("TFileTransport: error while writing event"); + int errno_copy = errno; + string errStr = "TFileTransport: error while writing event " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); + throw TTransportException(TTransportException::UNKNOWN, "TFileTransport: error while writing event", errno_copy); } unflushed += outEvent->eventSize_; @@ -746,10 +756,10 @@ void TFileTransport::openLogFile() { // make sure open call was successful if(fd_ == -1) { - char errorMsg[1024]; - sprintf(errorMsg, "TFileTransport: Could not open file: %s", filename_.c_str()); - GlobalOutput(errorMsg); - throw TTransportException(errorMsg); + int errno_copy = errno; + string errStr = "TFileTransport: openLogFile() ::open() file: " + filename_ + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); + throw TTransportException(TTransportException::NOT_OPEN, errStr, errno_copy); } } diff --git a/lib/cpp/src/transport/TServerSocket.cpp b/lib/cpp/src/transport/TServerSocket.cpp index 38ca3637..2b4e7c34 100644 --- a/lib/cpp/src/transport/TServerSocket.cpp +++ b/lib/cpp/src/transport/TServerSocket.cpp @@ -79,7 +79,9 @@ void TServerSocket::setTcpRecvBuffer(int tcpRecvBuffer) { void TServerSocket::listen() { int sv[2]; if (-1 == socketpair(AF_LOCAL, SOCK_STREAM, 0, sv)) { - GlobalOutput("TServerSocket::init()"); + int errno_copy = errno; + string errStr = "TServerSocket::listen() socketpair() " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); intSock1_ = -1; intSock2_ = -1; } else { @@ -113,36 +115,44 @@ void TServerSocket::listen() { serverSocket_ = socket(res->ai_family, res->ai_socktype, res->ai_protocol); if (serverSocket_ == -1) { - GlobalOutput("TServerSocket::listen() socket"); + int errno_copy = errno; + string errStr = "TServerSocket::listen() socket() " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); close(); - throw TTransportException(TTransportException::NOT_OPEN, "Could not create server socket."); + throw TTransportException(TTransportException::NOT_OPEN, "Could not create server socket.", errno_copy); } // Set reusaddress to prevent 2MSL delay on accept int one = 1; if (-1 == setsockopt(serverSocket_, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one))) { - GlobalOutput("TServerSocket::listen() SO_REUSEADDR"); + int errno_copy = errno; + string errStr = "TServerSocket::listen() setsockopt() SO_REUSEADDR " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); close(); - throw TTransportException(TTransportException::NOT_OPEN, "Could not set SO_REUSEADDR"); + throw TTransportException(TTransportException::NOT_OPEN, "Could not set SO_REUSEADDR", errno_copy); } // Set TCP buffer sizes if (tcpSendBuffer_ > 0) { if (-1 == setsockopt(serverSocket_, SOL_SOCKET, SO_SNDBUF, &tcpSendBuffer_, sizeof(tcpSendBuffer_))) { - GlobalOutput("TServerSocket::listen() SO_SNDBUF"); + int errno_copy = errno; + string errStr = "TServerSocket::listen() setsockopt() SO_SNDBUF " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); close(); - throw TTransportException(TTransportException::NOT_OPEN, "Could not set SO_SNDBUF"); + throw TTransportException(TTransportException::NOT_OPEN, "Could not set SO_SNDBUF", errno_copy); } } if (tcpRecvBuffer_ > 0) { if (-1 == setsockopt(serverSocket_, SOL_SOCKET, SO_RCVBUF, &tcpRecvBuffer_, sizeof(tcpRecvBuffer_))) { - GlobalOutput("TServerSocket::listen() SO_RCVBUF"); + int errno_copy = errno; + string errStr = "TServerSocket::listen() setsockopt() SO_RCVBUF " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); close(); - throw TTransportException(TTransportException::NOT_OPEN, "Could not set SO_RCVBUF"); + throw TTransportException(TTransportException::NOT_OPEN, "Could not set SO_RCVBUF", errno_copy); } } @@ -150,9 +160,11 @@ void TServerSocket::listen() { #ifdef TCP_DEFER_ACCEPT if (-1 == setsockopt(serverSocket_, SOL_SOCKET, TCP_DEFER_ACCEPT, &one, sizeof(one))) { - GlobalOutput("TServerSocket::listen() TCP_DEFER_ACCEPT"); + int errno_copy = errno; + string errStr = "TServerSocket::listen() setsockopt() TCP_DEFER_ACCEPT " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); close(); - throw TTransportException(TTransportException::NOT_OPEN, "Could not set TCP_DEFER_ACCEPT"); + throw TTransportException(TTransportException::NOT_OPEN, "Could not set TCP_DEFER_ACCEPT", errno_copy); } #endif // #ifdef TCP_DEFER_ACCEPT @@ -160,27 +172,37 @@ void TServerSocket::listen() { struct linger ling = {0, 0}; if (-1 == setsockopt(serverSocket_, SOL_SOCKET, SO_LINGER, &ling, sizeof(ling))) { + int errno_copy = errno; + string errStr = "TServerSocket::listen() setsockopt() SO_LINGER " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); close(); - GlobalOutput("TServerSocket::listen() SO_LINGER"); - throw TTransportException(TTransportException::NOT_OPEN, "Could not set SO_LINGER"); + throw TTransportException(TTransportException::NOT_OPEN, "Could not set SO_LINGER", errno_copy); } // TCP Nodelay, speed over bandwidth if (-1 == setsockopt(serverSocket_, IPPROTO_TCP, TCP_NODELAY, &one, sizeof(one))) { + int errno_copy = errno; + string errStr = "TServerSocket::listen() setsockopt() TCP_NODELAY " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); close(); - GlobalOutput("setsockopt TCP_NODELAY"); - throw TTransportException(TTransportException::NOT_OPEN, "Could not set TCP_NODELAY"); + throw TTransportException(TTransportException::NOT_OPEN, "Could not set TCP_NODELAY", errno_copy); } // Set NONBLOCK on the accept socket int flags = fcntl(serverSocket_, F_GETFL, 0); if (flags == -1) { - throw TTransportException(TTransportException::NOT_OPEN, "fcntl() failed"); + int errno_copy = errno; + string errStr = "TServerSocket::listen() fcntl() F_GETFL " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); + throw TTransportException(TTransportException::NOT_OPEN, "fcntl() failed", errno_copy); } if (-1 == fcntl(serverSocket_, F_SETFL, flags | O_NONBLOCK)) { - throw TTransportException(TTransportException::NOT_OPEN, "fcntl() failed"); + int errno_copy = errno; + string errStr = "TServerSocket::listen() fcntl() O_NONBLOCK " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); + throw TTransportException(TTransportException::NOT_OPEN, "fcntl() failed", errno_copy); } // prepare the port information @@ -209,9 +231,11 @@ void TServerSocket::listen() { // Call listen if (-1 == ::listen(serverSocket_, acceptBacklog_)) { - GlobalOutput("TServerSocket::listen() LISTEN"); + int errno_copy = errno; + string errStr = "TServerSocket::listen() listen() " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); close(); - throw TTransportException(TTransportException::NOT_OPEN, "Could not listen"); + throw TTransportException(TTransportException::NOT_OPEN, "Could not listen", errno_copy); } // The socket is now listening! @@ -242,17 +266,22 @@ shared_ptr TServerSocket::acceptImpl() { // a certain number continue; } - GlobalOutput("TServerSocket::acceptImpl() select -1"); - throw TTransportException(TTransportException::UNKNOWN); + int errno_copy = errno; + string errStr = "TServerSocket::acceptImpl() poll() " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); + throw TTransportException(TTransportException::UNKNOWN, "Unknown", errno_copy); } else if (ret > 0) { // Check for an interrupt signal if (intSock2_ >= 0 && FD_ISSET(intSock2_, &fds)) { int8_t buf; if (-1 == recv(intSock2_, &buf, sizeof(int8_t), 0)) { - GlobalOutput("TServerSocket::acceptImpl() interrupt receive"); + int errno_copy = errno; + string errStr = "TServerSocket::acceptImpl() recv() interrupt " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); } throw TTransportException(TTransportException::INTERRUPTED); } + // Check for the actual server socket being ready if (FD_ISSET(serverSocket_, &fds)) { break; @@ -271,7 +300,8 @@ shared_ptr TServerSocket::acceptImpl() { if (clientSocket < 0) { int errno_copy = errno; - GlobalOutput("TServerSocket::accept()"); + string errStr = "TServerSocket::acceptImpl() ::accept() " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); throw TTransportException(TTransportException::UNKNOWN, "accept()", errno_copy); } @@ -279,12 +309,15 @@ shared_ptr TServerSocket::acceptImpl() { int flags = fcntl(clientSocket, F_GETFL, 0); if (flags == -1) { int errno_copy = errno; - GlobalOutput("TServerSocket::select() fcntl GETFL"); + string errStr = "TServerSocket::acceptImpl() fcntl() F_GETFL " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); throw TTransportException(TTransportException::UNKNOWN, "fcntl(F_GETFL)", errno_copy); } + if (-1 == fcntl(clientSocket, F_SETFL, flags & ~O_NONBLOCK)) { int errno_copy = errno; - GlobalOutput("TServerSocket::select() fcntl SETFL"); + string errStr = "TServerSocket::acceptImpl() fcntl() F_SETFL ~O_NONBLOCK " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); throw TTransportException(TTransportException::UNKNOWN, "fcntl(F_SETFL)", errno_copy); } @@ -303,7 +336,9 @@ void TServerSocket::interrupt() { if (intSock1_ >= 0) { int8_t byte = 0; if (-1 == send(intSock1_, &byte, sizeof(int8_t), 0)) { - GlobalOutput("TServerSocket::interrupt()"); + int errno_copy = errno; + string errStr = "TServerSocket::interrupt() send() " + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); } } } diff --git a/lib/cpp/src/transport/TSocket.cpp b/lib/cpp/src/transport/TSocket.cpp index 85a63581..6a166645 100644 --- a/lib/cpp/src/transport/TSocket.cpp +++ b/lib/cpp/src/transport/TSocket.cpp @@ -94,7 +94,7 @@ bool TSocket::peek() { int r = recv(socket_, &buf, 1, MSG_PEEK); if (r == -1) { int errno_copy = errno; - string errStr = "TSocket::peek() " + getSocketInfo(); + string errStr = "TSocket::peek() recv() " + getSocketInfo() + TOutput::strerror_s(errno_copy); GlobalOutput(errStr.c_str()); throw TTransportException(TTransportException::UNKNOWN, "recv()", errno_copy); } @@ -109,7 +109,7 @@ void TSocket::openConnection(struct addrinfo *res) { socket_ = socket(res->ai_family, res->ai_socktype, res->ai_protocol); if (socket_ == -1) { int errno_copy = errno; - string errStr = "TSocket::open() socket " + getSocketInfo(); + string errStr = "TSocket::open() socket() " + getSocketInfo() + TOutput::strerror_s(errno_copy); GlobalOutput(errStr.c_str()); throw TTransportException(TTransportException::NOT_OPEN, "socket()", errno_copy); } @@ -134,26 +134,33 @@ void TSocket::openConnection(struct addrinfo *res) { int flags = fcntl(socket_, F_GETFL, 0); if (connTimeout_ > 0) { if (-1 == fcntl(socket_, F_SETFL, flags | O_NONBLOCK)) { - throw TTransportException(TTransportException::NOT_OPEN, "fcntl() failed"); + int errno_copy = errno; + string errStr = "TSocket::open() fcntl() " + getSocketInfo() + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); + throw TTransportException(TTransportException::NOT_OPEN, "fcntl() failed", errno_copy); } } else { if (-1 == fcntl(socket_, F_SETFL, flags & ~O_NONBLOCK)) { - throw TTransportException(TTransportException::NOT_OPEN, "fcntl() failed"); + int errno_copy = errno; + string errStr = "TSocket::open() fcntl " + getSocketInfo() + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); + throw TTransportException(TTransportException::NOT_OPEN, "fcntl() failed", errno_copy); } } // Connect the socket int ret = connect(socket_, res->ai_addr, res->ai_addrlen); + // success case if (ret == 0) { goto done; } if (errno != EINPROGRESS) { int errno_copy = errno; - string errStr = "TSocket::open() connect " + getSocketInfo(); + string errStr = "TSocket::open() connect() " + getSocketInfo() + TOutput::strerror_s(errno_copy); GlobalOutput(errStr.c_str()); - throw TTransportException(TTransportException::NOT_OPEN, "connect()", errno_copy); + throw TTransportException(TTransportException::NOT_OPEN, "connect() failed", errno_copy); } @@ -164,34 +171,35 @@ void TSocket::openConnection(struct addrinfo *res) { ret = poll(fds, 1, connTimeout_); if (ret > 0) { - // Ensure connected + // Ensure the socket is connected and that there are no errors set int val; socklen_t lon; lon = sizeof(int); int ret2 = getsockopt(socket_, SOL_SOCKET, SO_ERROR, (void *)&val, &lon); if (ret2 == -1) { int errno_copy = errno; - string errStr = "TSocket::open() getsockopt SO_ERROR " + getSocketInfo(); + string errStr = "TSocket::open() getsockopt() " + getSocketInfo() + TOutput::strerror_s(errno_copy); GlobalOutput(errStr.c_str()); - throw TTransportException(TTransportException::NOT_OPEN, "open()", errno_copy); + throw TTransportException(TTransportException::NOT_OPEN, "getsockopt()", errno_copy); } + // no errors on socket, go to town if (val == 0) { goto done; } - int errno_copy = errno; - string errStr = "TSocket::open() SO_ERROR was set " + getSocketInfo(); + string errStr = "TSocket::open() error on socket (after poll) " + getSocketInfo() + TOutput::strerror_s(val); GlobalOutput(errStr.c_str()); - throw TTransportException(TTransportException::NOT_OPEN, "open()", errno_copy); + throw TTransportException(TTransportException::NOT_OPEN, "socket open() error", val); } else if (ret == 0) { - int errno_copy = errno; + // socket timed out string errStr = "TSocket::open() timed out " + getSocketInfo(); GlobalOutput(errStr.c_str()); - throw TTransportException(TTransportException::NOT_OPEN, "open()", errno_copy); + throw TTransportException(TTransportException::NOT_OPEN, "open() timed out"); } else { + // error on poll() int errno_copy = errno; - string errStr = "TSocket::open() poll error " + getSocketInfo(); + string errStr = "TSocket::open() poll() " + getSocketInfo() + TOutput::strerror_s(errno_copy); GlobalOutput(errStr.c_str()); - throw TTransportException(TTransportException::NOT_OPEN, "open()", errno_copy); + throw TTransportException(TTransportException::NOT_OPEN, "poll() failed", errno_copy); } done: @@ -210,8 +218,10 @@ void TSocket::open() { } struct addrinfo hints, *res, *res0; + res = NULL; + res0 = NULL; int error; - char port[sizeof("65536") + 1]; + char port[sizeof("65536")]; memset(&hints, 0, sizeof(hints)); hints.ai_family = PF_UNSPEC; hints.ai_socktype = SOCK_STREAM; @@ -221,7 +231,8 @@ void TSocket::open() { error = getaddrinfo(host_.c_str(), port, &hints, &res0); if (error) { - fprintf(stderr, "getaddrinfo %d: %s\n", error, gai_strerror(error)); + string errStr = "TSocket::open() getaddrinfo() " + getSocketInfo() + string(gai_strerror(error)); + GlobalOutput(errStr.c_str()); close(); throw TTransportException(TTransportException::NOT_OPEN, "Could not resolve host for client socket."); } @@ -310,7 +321,7 @@ uint32_t TSocket::read(uint8_t* buf, uint32_t len) { } // Now it's not a try again case, but a real probblez - string errStr = "TSocket::read() " + getSocketInfo(); + string errStr = "TSocket::read() recv() " + getSocketInfo() + TOutput::strerror_s(errno); GlobalOutput(errStr.c_str()); // If we disconnect with no linger time @@ -329,9 +340,7 @@ uint32_t TSocket::read(uint8_t* buf, uint32_t len) { } // Some other error, whatevz - char buff[1024]; - sprintf(buff, "ERROR errno: %d", errno); - throw TTransportException(TTransportException::UNKNOWN, buff); + throw TTransportException(TTransportException::UNKNOWN, "Unknown", errno); } // The remote host has closed the socket @@ -365,16 +374,16 @@ void TSocket::write(const uint8_t* buf, uint32_t len) { // Fail on a send error if (b < 0) { + int errno_copy = errno; + string errStr = "TSocket::write() send() " + getSocketInfo() + TOutput::strerror_s(errno_copy); + GlobalOutput(errStr.c_str()); + if (errno == EPIPE || errno == ECONNRESET || errno == ENOTCONN) { - int errno_copy = errno; close(); - throw TTransportException(TTransportException::NOT_OPEN, "send()", errno_copy); + throw TTransportException(TTransportException::NOT_OPEN, "write() send()", errno_copy); } - int errno_copy = errno; - string errStr = "TSocket::write() send < 0 " + getSocketInfo(); - GlobalOutput(errStr.c_str()); - throw TTransportException(TTransportException::UNKNOWN, "send", errno_copy); + throw TTransportException(TTransportException::UNKNOWN, "write() send()", errno_copy); } // Fail on blocked send @@ -411,7 +420,8 @@ void TSocket::setLinger(bool on, int linger) { struct linger l = {(lingerOn_ ? 1 : 0), lingerVal_}; int ret = setsockopt(socket_, SOL_SOCKET, SO_LINGER, &l, sizeof(l)); if (ret == -1) { - string errStr = "TSocket::setLinger() " + getSocketInfo(); + int errno_copy = errno; + string errStr = "TSocket::setLinger() setsockopt() " + getSocketInfo() + TOutput::strerror_s(errno_copy); GlobalOutput(errStr.c_str()); } } @@ -426,7 +436,8 @@ void TSocket::setNoDelay(bool noDelay) { int v = noDelay_ ? 1 : 0; int ret = setsockopt(socket_, IPPROTO_TCP, TCP_NODELAY, &v, sizeof(v)); if (ret == -1) { - string errStr = "TSocket::setNoDelay() " + getSocketInfo(); + int errno_copy = errno; + string errStr = "TSocket::setNoDelay() setsockopt() " + getSocketInfo() + TOutput::strerror_s(errno_copy); GlobalOutput(errStr.c_str()); } } @@ -455,7 +466,8 @@ void TSocket::setRecvTimeout(int ms) { struct timeval r = recvTimeval_; int ret = setsockopt(socket_, SOL_SOCKET, SO_RCVTIMEO, &r, sizeof(r)); if (ret == -1) { - string errStr = "TSocket::setRecvTimeout() " + getSocketInfo(); + int errno_copy = errno; + string errStr = "TSocket::setRecvTimeout() setsockopt() " + getSocketInfo() + TOutput::strerror_s(errno_copy); GlobalOutput(errStr.c_str()); } } @@ -477,7 +489,8 @@ void TSocket::setSendTimeout(int ms) { (int)((sendTimeout_%1000)*1000)}; int ret = setsockopt(socket_, SOL_SOCKET, SO_SNDTIMEO, &s, sizeof(s)); if (ret == -1) { - string errStr = "TSocket::setSendTimeout() " + getSocketInfo(); + int errno_copy = errno; + string errStr = "TSocket::setSendTimeout() setsockopt() " + getSocketInfo() + TOutput::strerror_s(errno_copy); GlobalOutput(errStr.c_str()); } } diff --git a/lib/cpp/src/transport/TSocket.h b/lib/cpp/src/transport/TSocket.h index f0495ec3..8d4a5a77 100644 --- a/lib/cpp/src/transport/TSocket.h +++ b/lib/cpp/src/transport/TSocket.h @@ -223,7 +223,6 @@ class TSocket : public TTransport { /** Recv timeout timeval */ struct timeval recvTimeval_; - }; }}} // facebook::thrift::transport diff --git a/lib/cpp/src/transport/TTransportException.cpp b/lib/cpp/src/transport/TTransportException.cpp index e6322657..e231a93a 100644 --- a/lib/cpp/src/transport/TTransportException.cpp +++ b/lib/cpp/src/transport/TTransportException.cpp @@ -14,29 +14,5 @@ using boost::lexical_cast; namespace facebook { namespace thrift { namespace transport { -string TTransportException::strerror_s(int errno_copy) { -#ifndef HAVE_STRERROR_R - return "errno = " + lexical_cast(errno_copy); -#else // HAVE_STRERROR_R - - char b_errbuf[1024] = { '\0' }; -#ifdef STRERROR_R_CHAR_P - char *b_error = strerror_r(errno_copy, b_errbuf, sizeof(b_errbuf)); -#else - char *b_error = b_errbuf; - int rv = strerror_r(errno_copy, b_errbuf, sizeof(b_errbuf)); - if (rv == -1) { - // strerror_r failed. omgwtfbbq. - return "XSI-compliant strerror_r() failed with errno = " + - lexical_cast(errno_copy); - } -#endif - // Can anyone prove that explicit cast is probably not necessary - // to ensure that the string object is constructed before - // b_error becomes invalid? - return string(b_error); - -#endif // HAVE_STRERROR_R -} - }}} // facebook::thrift::transport + diff --git a/lib/cpp/src/transport/TTransportException.h b/lib/cpp/src/transport/TTransportException.h index 73bad805..09129ec5 100644 --- a/lib/cpp/src/transport/TTransportException.h +++ b/lib/cpp/src/transport/TTransportException.h @@ -58,7 +58,7 @@ class TTransportException : public facebook::thrift::TException { TTransportException(TTransportExceptionType type, const std::string& message, int errno_copy) : - facebook::thrift::TException(message + ": " + strerror_s(errno_copy)), + facebook::thrift::TException(message + ": " + TOutput::strerror_s(errno_copy)), type_(type) {} virtual ~TTransportException() throw() {} -- 2.17.1