From 9a08c64ff7ca9f47b718c665c61a7dfa3088afcb Mon Sep 17 00:00:00 2001 From: Emiel Bruijntjes Date: Thu, 8 Mar 2018 10:37:49 +0100 Subject: [PATCH] more elegant close procedure for tcp connections --- src/linux_tcp/tcpconnected.h | 95 +++++++++++++++++++++++++++--------- src/linux_tcp/tcpresolver.h | 2 +- src/linux_tcp/tcpstate.h | 20 +------- 3 files changed, 76 insertions(+), 41 deletions(-) diff --git a/src/linux_tcp/tcpconnected.h b/src/linux_tcp/tcpconnected.h index bb0ac5d..f05d532 100644 --- a/src/linux_tcp/tcpconnected.h +++ b/src/linux_tcp/tcpconnected.h @@ -54,8 +54,36 @@ private: * @var size_t */ size_t _reallocate = 0; + + /** + * Have we already made the last report to the user (about an error or closed connection?) + * @var bool + */ + bool _finalized = false; + /** + * Close the connection + * @return bool + */ + bool close() + { + // do nothing if already closed + if (_socket < 0) return false; + + // and stop monitoring it + _handler->monitor(_connection, _socket, 0); + + // close the socket + ::close(_socket); + + // forget filedescriptor + _socket = -1; + + // done + return true; + } + /** * Helper method to report an error * @return bool Was an error reported? @@ -65,6 +93,16 @@ private: // some errors are ok and do not (necessarily) mean that we're disconnected if (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR) return false; + // connection can be closed now + close(); + + // if the user has already been notified, we do not have to do anything else + if (_finalized) return true; + + // update the _finalized member before we make the call to user space because + // the user space may destruct this object + _finalized = true; + // we have an error - report this to the user _handler->onError(_connection, strerror(errno)); @@ -110,14 +148,8 @@ public: */ virtual ~TcpConnected() noexcept { - // skip if handler is already forgotten - if (_handler == nullptr) return; - - // we no longer have to monitor the socket - _handler->monitor(_connection, _socket, 0); - // close the socket - close(_socket); + close(); } /** @@ -145,7 +177,7 @@ public: auto result = _out.sendto(_socket); // are we in an error state? - if (result < 0 && reportError()) return nextState(monitor); + if (result < 0 && reportError()) return nextState(monitor); // if buffer is empty by now, we no longer have to check for // writability, but only for readability @@ -255,28 +287,47 @@ public: return TcpState::reportNegotiate(heartbeat); } + /** + * Report to the handler that the object is in an error state. + * @param error + */ + virtual void reportError(const char *error) + { + // close the socket + close(); + + // if the user was already notified of an final state, we do not have to proceed + if (_finalized) return; + + // remember that this is the final call to user space + _finalized = true; + + // pass to handler + _handler->onError(_connection, error); + } + /** * Report to the handler that the connection was nicely closed + * This is the counter-part of the connection->close() call. */ virtual void reportClosed() override { - // we no longer have to monitor the socket - _handler->monitor(_connection, _socket, 0); + // we will shutdown the socket in a very elegant way, we notify the peer + // that we will not be sending out more write operations + shutdown(_socket, SHUT_WR); + + // we still monitor the socket for readability to see if our close call was + // confirmed by the peer + _handler->monitor(_connection, _socket, readable); - // close the socket - close(_socket); + // if the user was already notified of an final state, we do not have to proceed + if (_finalized) return; - // socket is closed now - _socket = -1; + // remember that this is the final call to user space + _finalized = true; - // copy the handler (if might destruct this object) - auto *handler = _handler; - - // reset member before the handler can make a mess of it - _handler = nullptr; - - // notify to handler - handler->onClosed(_connection); + // pass to handler + _handler->onClosed(_connection); } }; diff --git a/src/linux_tcp/tcpresolver.h b/src/linux_tcp/tcpresolver.h index 93399e3..50516aa 100644 --- a/src/linux_tcp/tcpresolver.h +++ b/src/linux_tcp/tcpresolver.h @@ -194,7 +194,7 @@ public: if (_socket >= 0) { // if we need a secure connection, we move to the tls handshake - // @todo catch exception + // @todo catch possible exception if (_secure) return new SslHandshake(_connection, _socket, _hostname, std::move(_buffer), _handler); // otherwise we have a valid regular tcp connection diff --git a/src/linux_tcp/tcpstate.h b/src/linux_tcp/tcpstate.h index 2b5dd3f..1c3a280 100644 --- a/src/linux_tcp/tcpstate.h +++ b/src/linux_tcp/tcpstate.h @@ -36,22 +36,6 @@ protected: TcpHandler *_handler; protected: - /** - * Helper function to reset the handler, and to return the old handler object - * @return TcpHandler* User-supplied handler that was just reset - */ - TcpHandler *reset(TcpHandler *handler) - { - // remember old handler - auto *oldhandler = _handler; - - // install the new handler - _handler = handler; - - // return the old handler - return oldhandler; - } - /** * Protected constructor * @param connection Original TCP connection object @@ -150,7 +134,7 @@ public: virtual void reportError(const char *error) { // pass to handler - reset(nullptr)->onError(_connection, error); + _handler->onError(_connection, error); } /** @@ -182,7 +166,7 @@ public: virtual void reportClosed() { // pass to handler - reset(nullptr)->onClosed(_connection); + _handler->onClosed(_connection); } };