From 5dfb1cc4b4d039c448ca5cddf34f71da8ed7a916 Mon Sep 17 00:00:00 2001 From: Gunnar Beutner Date: Sat, 26 May 2012 20:00:03 +0200 Subject: [PATCH] Cleaned up socket error handling. --- base/exception.cpp | 22 +++++++++++ base/exception.h | 5 +++ base/observable.h | 10 +++++ base/socket.cpp | 80 ++++++++++++++++++++++++++++++-------- base/socket.h | 20 ++++++++-- base/tcpclient.cpp | 22 ++++++----- base/tcpserver.cpp | 6 ++- base/tcpsocket.cpp | 15 ++++--- base/tlsclient.cpp | 24 ++---------- base/variant.cpp | 2 +- base/variant.h | 2 +- icinga/jsonrpcendpoint.cpp | 2 +- 12 files changed, 150 insertions(+), 60 deletions(-) diff --git a/base/exception.cpp b/base/exception.cpp index bdc0e82ac..5a6dcc95b 100644 --- a/base/exception.cpp +++ b/base/exception.cpp @@ -26,6 +26,7 @@ using namespace icinga; */ Exception::Exception(void) { + m_Code = 0; m_Message = NULL; } @@ -36,6 +37,7 @@ Exception::Exception(void) */ Exception::Exception(const char *message) { + m_Code = 0; m_Message = NULL; SetMessage(message); } @@ -48,6 +50,26 @@ Exception::~Exception(void) throw() Memory::Free(m_Message); } +/** + * Retrieves the error code for the exception. + * + * @returns The error code. + */ +int Exception::GetCode(void) const +{ + return m_Code; +} + +/** + * Sets the error code for the exception. + * + * @param code The error code. + */ +void Exception::SetCode(int code) +{ + m_Code = code; +} + /** * Retrieves the description for the exception. * diff --git a/base/exception.h b/base/exception.h index b47b3bdce..3ab702e64 100644 --- a/base/exception.h +++ b/base/exception.h @@ -35,15 +35,18 @@ public: Exception(const char *message); virtual ~Exception(void) throw(); + int GetCode(void) const; const char *GetMessage(void) const; virtual const char *what(void) const throw(); protected: + void SetCode(int code); void SetMessage(const char *message); private: char *m_Message; + int m_Code; }; #define DEFINE_EXCEPTION_CLASS(klass) \ @@ -101,6 +104,7 @@ public: { string msg = message + ": " + FormatErrorCode(errorCode); SetMessage(msg.c_str()); + SetCode(errorCode); } /** @@ -129,6 +133,7 @@ public: { string msg = message + ": " + FormatErrorCode(errorCode); SetMessage(msg.c_str()); + SetCode(errorCode); } /** diff --git a/base/observable.h b/base/observable.h index 2fb0b7bfd..26721895a 100644 --- a/base/observable.h +++ b/base/observable.h @@ -86,6 +86,16 @@ public: } } + /** + * Checks whether there's at least one observer. + * + * @returns true if there are one or more observers, false otherwise. + */ + bool HasObservers(void) const + { + return !m_Observers.empty(); + } + private: vector m_Observers; }; diff --git a/base/socket.cpp b/base/socket.cpp index e5b697cc8..a876b3dcb 100644 --- a/base/socket.cpp +++ b/base/socket.cpp @@ -132,28 +132,53 @@ void Socket::CloseInternal(bool from_dtor) } /** - * Handles a socket error by calling the OnError event. + * Retrieves the last error that occured for the socket. + * + * @returns An error code. */ -void Socket::HandleSocketError(void) +int Socket::GetError(void) const { int opt; socklen_t optlen = sizeof(opt); int rc = getsockopt(GetFD(), SOL_SOCKET, SO_ERROR, (char *)&opt, &optlen); - if (rc >= 0 && opt != 0) { - SocketErrorEventArgs sea; - sea.Code = opt; -#ifdef _WIN32 - sea.Message = Win32Exception::FormatErrorCode(sea.Code); -#else /* _WIN32 */ - sea.Message = PosixException::FormatErrorCode(sea.Code); -#endif /* _WIN32 */ - OnError(sea); - } + if (rc >= 0) + return opt; - Close(); - return; + return 0; +} + +/** + * Retrieves the last socket error. + * + * @returns An error code. + */ +int Socket::GetLastSocketError(void) +{ +#ifdef _WIN32 + return WSAGetLastError(); +#else /* _WIN32 */ + return errno; +#endif /* _WIN32 */ +} + +/** + * Handles a socket error by calling the OnError event or throwing an exception + * when there are no observers for the OnError event. + * + * @param exception An exception. + */ +void Socket::HandleSocketError(const Exception& exception) +{ + if (OnError.HasObservers()) { + SocketErrorEventArgs sea(exception); + OnError(sea); + + Close(); + } else { + throw exception; + } } /** @@ -164,7 +189,8 @@ void Socket::HandleSocketError(void) */ int Socket::ExceptionEventHandler(const EventArgs&) { - HandleSocketError(); + HandleSocketError(SocketException( + "select() returned fd in except fdset", GetError())); return 0; } @@ -218,7 +244,8 @@ string Socket::GetClientAddress(void) socklen_t len = sizeof(sin); if (getsockname(GetFD(), (sockaddr *)&sin, &len) < 0) { - HandleSocketError(); + HandleSocketError(SocketException( + "getsockname() failed", GetError())); return string(); } @@ -237,10 +264,29 @@ string Socket::GetPeerAddress(void) socklen_t len = sizeof(sin); if (getpeername(GetFD(), (sockaddr *)&sin, &len) < 0) { - HandleSocketError(); + HandleSocketError(SocketException( + "getpeername() failed", GetError())); return string(); } return GetAddressFromSockaddr((sockaddr *)&sin, len); } + +/** + * Constructor for the SocketException class. + * + * @param message The error message. + * @param errorCode The error code. + */ +SocketException::SocketException(const string& message, int errorCode) +{ +#ifdef _WIN32 + string details = Win32Exception::FormatErrorCode(errorCode); +#else /* _WIN32 */ + string details = PosixException::FormatErrorCode(errorCode); +#endif /* _WIN32 */ + + string msg = message + ": " + details; + SetMessage(msg.c_str()); +} \ No newline at end of file diff --git a/base/socket.h b/base/socket.h index e52e16833..1d56c23d7 100644 --- a/base/socket.h +++ b/base/socket.h @@ -29,8 +29,10 @@ namespace icinga { */ struct I2_BASE_API SocketErrorEventArgs : public EventArgs { - int Code; /**< The error code. */ - string Message; /**< A message describing the error. */ + const Exception& Error; + + SocketErrorEventArgs(const Exception& exception) + : Error(exception) { } }; /** @@ -74,7 +76,10 @@ public: protected: Socket(void); - void HandleSocketError(void); + int GetError(void) const; + static int GetLastSocketError(void); + void HandleSocketError(const Exception& exception); + virtual void CloseInternal(bool from_dtor); private: @@ -85,6 +90,15 @@ private: static string GetAddressFromSockaddr(sockaddr *address, socklen_t len); }; +/** + * A socket exception. + */ +class SocketException : public Exception +{ +public: + SocketException(const string& message, int errorCode); +}; + } #endif /* SOCKET_H */ diff --git a/base/tcpclient.cpp b/base/tcpclient.cpp index 83d04f6b8..0045cff5a 100644 --- a/base/tcpclient.cpp +++ b/base/tcpclient.cpp @@ -76,8 +76,8 @@ void TcpClient::Connect(const string& node, const string& service) int rc = getaddrinfo(node.c_str(), service.c_str(), &hints, &result); if (rc < 0) { - HandleSocketError(); - + HandleSocketError(SocketException( + "getaddrinfo() failed", GetLastSocketError())); return; } @@ -94,19 +94,23 @@ void TcpClient::Connect(const string& node, const string& service) rc = connect(fd, info->ai_addr, info->ai_addrlen); #ifdef _WIN32 - if (rc < 0 && WSAGetLastError() != WSAEWOULDBLOCK) + if (rc < 0 && WSAGetLastError() != WSAEWOULDBLOCK) { #else /* _WIN32 */ - if (rc < 0 && errno != EINPROGRESS) + if (rc < 0 && errno != EINPROGRESS) { #endif /* _WIN32 */ + closesocket(fd); + SetFD(INVALID_SOCKET); + continue; + } break; } - if (fd == INVALID_SOCKET) - HandleSocketError(); - freeaddrinfo(result); + + if (fd == INVALID_SOCKET) + HandleSocketError(InvalidArgumentException()); } /** @@ -151,7 +155,7 @@ int TcpClient::ReadableEventHandler(const EventArgs&) return 0; if (rc <= 0) { - HandleSocketError(); + HandleSocketError(SocketException("recv() failed", GetError())); return 0; } @@ -177,7 +181,7 @@ int TcpClient::WritableEventHandler(const EventArgs&) rc = send(GetFD(), (const char *)m_SendQueue->GetReadBuffer(), m_SendQueue->GetSize(), 0); if (rc <= 0) { - HandleSocketError(); + HandleSocketError(SocketException("send() failed", GetError())); return 0; } diff --git a/base/tcpserver.cpp b/base/tcpserver.cpp index de6c4a642..a5c3d6dcd 100644 --- a/base/tcpserver.cpp +++ b/base/tcpserver.cpp @@ -67,7 +67,8 @@ void TcpServer::Listen(void) int rc = listen(GetFD(), SOMAXCONN); if (rc < 0) { - HandleSocketError(); + HandleSocketError(SocketException( + "listen() failed", GetError())); return; } } @@ -88,7 +89,8 @@ int TcpServer::ReadableEventHandler(const EventArgs&) fd = accept(GetFD(), (sockaddr *)&addr, &addrlen); if (fd < 0) { - HandleSocketError(); + HandleSocketError(SocketException( + "accept() failed", GetError())); return 0; } diff --git a/base/tcpsocket.cpp b/base/tcpsocket.cpp index 97733e977..2cce586d1 100644 --- a/base/tcpsocket.cpp +++ b/base/tcpsocket.cpp @@ -33,7 +33,8 @@ void TcpSocket::MakeSocket(int family) int fd = socket(family, SOCK_STREAM, 0); if (fd == INVALID_SOCKET) { - HandleSocketError(); + HandleSocketError(SocketException( + "socket() failed", GetLastSocketError())); return; } @@ -70,8 +71,10 @@ void TcpSocket::Bind(string node, string service, int family) hints.ai_protocol = IPPROTO_TCP; hints.ai_flags = AI_PASSIVE; - if (getaddrinfo(node.empty() ? NULL : node.c_str(), service.c_str(), &hints, &result) < 0) { - HandleSocketError(); + if (getaddrinfo(node.empty() ? NULL : node.c_str(), + service.c_str(), &hints, &result) < 0) { + HandleSocketError(SocketException( + "getaddrinfo() failed", GetLastSocketError())); return; } @@ -110,8 +113,8 @@ void TcpSocket::Bind(string node, string service, int family) break; } - if (fd == INVALID_SOCKET) - HandleSocketError(); - freeaddrinfo(result); + + if (fd == INVALID_SOCKET) + HandleSocketError(InvalidArgumentException()); } diff --git a/base/tlsclient.cpp b/base/tlsclient.cpp index bba6d40d6..a9e1ebc80 100644 --- a/base/tlsclient.cpp +++ b/base/tlsclient.cpp @@ -132,7 +132,8 @@ int TlsClient::ReadableEventHandler(const EventArgs&) return 0; default: - HandleSSLError(); + HandleSocketError(OpenSSLException( + "SSL_read failed", ERR_get_error())); return 0; } @@ -174,7 +175,8 @@ int TlsClient::WritableEventHandler(const EventArgs&) return 0; default: - HandleSSLError(); + HandleSocketError(OpenSSLException( + "SSL_write failed", ERR_get_error())); return 0; } @@ -229,24 +231,6 @@ void TlsClient::CloseInternal(bool from_dtor) TcpClient::CloseInternal(from_dtor); } -/** - * Handles an OpenSSL error. - */ -void TlsClient::HandleSSLError(void) -{ - int code = ERR_get_error(); - - if (code != 0) { - SocketErrorEventArgs sea; - sea.Code = code; - sea.Message = OpenSSLException::FormatErrorCode(sea.Code); - OnError(sea); - } - - Close(); - return; -} - /** * Factory function for the TlsClient class. * diff --git a/base/variant.cpp b/base/variant.cpp index b41f0b0c9..3c906df27 100644 --- a/base/variant.cpp +++ b/base/variant.cpp @@ -69,7 +69,7 @@ long Variant::GetInteger(void) const * * @returns The variant's value as a bool. */ -long Variant::GetBool(void) const +bool Variant::GetBool(void) const { Convert(VariantInteger); diff --git a/base/variant.h b/base/variant.h index b2e24382e..19db37a76 100644 --- a/base/variant.h +++ b/base/variant.h @@ -69,7 +69,7 @@ public: VariantType GetType(void) const; long GetInteger(void) const; - long GetBool(void) const; + bool GetBool(void) const; string GetString(void) const; Object::Ptr GetObject(void) const; diff --git a/icinga/jsonrpcendpoint.cpp b/icinga/jsonrpcendpoint.cpp index 6608bed7e..81ce236c0 100644 --- a/icinga/jsonrpcendpoint.cpp +++ b/icinga/jsonrpcendpoint.cpp @@ -132,7 +132,7 @@ int JsonRpcEndpoint::ClientClosedHandler(const EventArgs&) int JsonRpcEndpoint::ClientErrorHandler(const SocketErrorEventArgs& ea) { - cerr << "Error occured for JSON-RPC socket: Code=" << ea.Code << "; Message=" << ea.Message << endl; + cerr << "Error occured for JSON-RPC socket: Code=" << ea.Error.GetCode() << "; Message=" << ea.Error.GetMessage() << endl; return 0; }