From e617161c8c59dd07c833d103a6136ebb96c925b3 Mon Sep 17 00:00:00 2001 From: Emiel Bruijntjes Date: Mon, 5 Nov 2018 15:49:22 +0100 Subject: [PATCH] the state::abort() method is no longer needed --- src/linux_tcp/sslconnected.h | 18 ------ src/linux_tcp/sslhandshake.h | 19 ------ src/linux_tcp/sslshutdown.h | 19 ------ src/linux_tcp/tcpclosed.h | 11 ---- src/linux_tcp/tcpconnected.h | 18 ------ src/linux_tcp/tcpconnection.cpp | 30 ++------- src/linux_tcp/tcpresolver.h | 20 ------ src/linux_tcp/tcpsecurestate.h | 106 -------------------------------- src/linux_tcp/tcpshutdown.h | 18 ------ src/linux_tcp/tcpstate.h | 7 --- 10 files changed, 4 insertions(+), 262 deletions(-) delete mode 100644 src/linux_tcp/tcpsecurestate.h diff --git a/src/linux_tcp/sslconnected.h b/src/linux_tcp/sslconnected.h index 2220a71..048e6b0 100644 --- a/src/linux_tcp/sslconnected.h +++ b/src/linux_tcp/sslconnected.h @@ -418,24 +418,6 @@ public: // done return this; } - - /** - * Close the connection immediately - * @param monitor object to check if connection object is still active - * @return TcpState the new state - */ - virtual TcpState *abort(const Monitor &monitor) override - { - // close the connection right now - if (!close(monitor)) return nullptr; - - // tell the connection that it failed (this eventually ends up in our reportError() method) - // @todo to we indeed need this? - //_connection->fail(); - - // go to the final state (if not yet disconnected) - return monitor.valid() ? new TcpClosed(this) : nullptr; - } /** * Send data over the connection diff --git a/src/linux_tcp/sslhandshake.h b/src/linux_tcp/sslhandshake.h index 251eef9..9c763cd 100644 --- a/src/linux_tcp/sslhandshake.h +++ b/src/linux_tcp/sslhandshake.h @@ -268,25 +268,6 @@ public: } } } - - /** - * Close the connection immediately - * @param monitor object to check if connection object is still active - * @return TcpState the new state - */ - virtual TcpState *abort(const Monitor &monitor) override - { - // close the socket - close(); - - // report to the user that the handshake was aborted - // @todo do we need this? - //_handler->onError(_connection, "ssl handshake aborted"); - - // done, go to the closed state (plus check if connection still exists, because - // after the onError() call the user space program may have destructed that object) - return monitor.valid() ? new TcpClosed(this) : nullptr; - } }; /** diff --git a/src/linux_tcp/sslshutdown.h b/src/linux_tcp/sslshutdown.h index 02e7d12..b7f5762 100644 --- a/src/linux_tcp/sslshutdown.h +++ b/src/linux_tcp/sslshutdown.h @@ -167,25 +167,6 @@ public: } } } - - /** - * Abort the shutdown operation immediately - * @param monitor Monitor that can be used to check if the tcp connection is still alive - * @return TcpState - */ - virtual TcpState *abort(const Monitor &monitor) override - { - // cleanup the connection - // @todo this also calls onClosed() - cleanup(); - - // report to user-space that the ssl shutdown was aborted - // @todo - //_handler->onError(_connection, "ssl shutdown aborted"); - - // go to the final state (if not yet disconnected) - return monitor.valid() ? new TcpClosed(this) : nullptr; - } }; /** diff --git a/src/linux_tcp/tcpclosed.h b/src/linux_tcp/tcpclosed.h index ebd2561..90d5fd6 100644 --- a/src/linux_tcp/tcpclosed.h +++ b/src/linux_tcp/tcpclosed.h @@ -41,17 +41,6 @@ public: * Destructor */ virtual ~TcpClosed() noexcept = default; - - /** - * Abort the operation - * @param monitor Monitor that can be used to check if the tcp connection is still alive - * @return TcpState - */ - virtual TcpState *abort(const Monitor &monitor) override - { - // connection was closed and stays closed - return this; - } }; /** diff --git a/src/linux_tcp/tcpconnected.h b/src/linux_tcp/tcpconnected.h index 7b4c956..c3c1d37 100644 --- a/src/linux_tcp/tcpconnected.h +++ b/src/linux_tcp/tcpconnected.h @@ -257,24 +257,6 @@ public: return this; } - /** - * Close the connection immediately - * @param monitor object to check if connection object is still active - * @return TcpState the new state - */ - virtual TcpState *abort(const Monitor &monitor) override - { - // close the connection right now - if (!close(monitor)) return nullptr; - - // fail the connection (this ends up in our reportError() method) - // @todo should this not happen - //_connection->fail(); - - // go to the final state (if not yet disconnected) - return monitor.valid() ? new TcpClosed(this) : nullptr; - } - /** * When the AMQP transport layer is closed * @param monitor Object that can be used if connection is still alive diff --git a/src/linux_tcp/tcpconnection.cpp b/src/linux_tcp/tcpconnection.cpp index 4fa471a..59c4c2b 100644 --- a/src/linux_tcp/tcpconnection.cpp +++ b/src/linux_tcp/tcpconnection.cpp @@ -123,39 +123,17 @@ void TcpConnection::flush() */ bool TcpConnection::close(bool immediate) { - // @todo what if not yet connected / still doing a lookup? + // @todo what if not yet connected / still doing a lookup / already disconnected? // if no immediate disconnect is needed, we can simply start the closing handshake if (!immediate) return _connection.close(); - // a call to user-space will be made, so we need to monitor if "this" is destructed - Monitor monitor(this); - - // pass to the underlying connection to start the amqp-closing handshake - _connection.close(); - - // if the user-space code destructed the connection, there is nothing else to do - if (!monitor.valid()) return true; - - // remember the old state (this is necessary because _state may be modified by user-code) - auto *oldstate = _state.get(); - - // abort the operation - // @todo does this call user-space stuff? - auto *newstate = _state->abort(monitor); - - // if the state did not change, we do not have to update a member, - // when the newstate is nullptr, the object is (being) destructed - // and we do not have to do anything else either - if (newstate == nullptr || newstate == oldstate) return true; - - // replace it with the new implementation - _state.reset(newstate); - // fail the connection / report the error to user-space - // @todo what if channels are not even ready? _connection.fail("connection prematurely closed by client"); + // change the state + _state.reset(new TcpClosed(_state.get())); + // done, we return true because the connection is closed return true; } diff --git a/src/linux_tcp/tcpresolver.h b/src/linux_tcp/tcpresolver.h index d3af215..8384ebc 100644 --- a/src/linux_tcp/tcpresolver.h +++ b/src/linux_tcp/tcpresolver.h @@ -242,26 +242,6 @@ public: return proceed(monitor); } - /** - * Close the connection immediately - * @param monitor object to check if connection object is still active - * @return TcpState the new state - */ - virtual TcpState *abort(const Monitor &monitor) override - { - // just wait for the other thread to be ready - _thread.join(); - - // close the socket - if (_socket >= 0) { ::close(_socket); _socket = -1; } - - // inform user space that the connection is cancelled - _parent->onError(this, "tcp connect aborted"); - - // go to the final state (if not yet disconnected) - return monitor.valid() ? new TcpClosed(this) : nullptr; - } - /** * Send data over the connection * @param buffer buffer to send diff --git a/src/linux_tcp/tcpsecurestate.h b/src/linux_tcp/tcpsecurestate.h deleted file mode 100644 index 77aae4d..0000000 --- a/src/linux_tcp/tcpsecurestate.h +++ /dev/null @@ -1,106 +0,0 @@ -/** - * TcpSecureState.h - * - * Utility class that takes care of setting a new state. It contains - * a number of checks that prevents that the state is overwritten - * if the object is destructed in the meantime - * - * @author Emiel Bruijntjes - * @copyright 2018 Copernica BV - */ - -/** - * Include guard - */ -#pragma once - -/** - * Begin of namespace - */ -namespace AMQP { - -/** - * Class definition - */ -class TcpSecureState -{ -private: - /** - * Monitor to check the validity of the connection - * @var Monitor - */ - Monitor _monitor; - - /** - * Reference to the pointer to the state that should be updated - * @var std::unique_ptr - */ - std::unique_ptr &_state; - - /** - * The old pointer - * @var TcpState* - */ - const TcpState *_old; - -public: - /** - * Constructor - * @param watchable the object that can be destructor - * @param state the old state value - */ - TcpSecureState(Watchable *watchable, std::unique_ptr &state) : - _monitor(watchable), _state(state), _old(state.get()) {} - - /** - * No copying - * @param that - */ - TcpSecureState(const TcpSecureState &that) = delete; - - /** - * Destructor - */ - virtual ~TcpSecureState() = default; - - /** - * Expose the monitor - * @return Monitor - */ - const Monitor &monitor() const { return _monitor; } - - /** - * Assign a new state - * @param state this is a newly allocated state - * @return bool true if the object is still valid - */ - bool assign(TcpState *state) - { - // do nothing if the state did not change, or if object was destructed - if (_old == state || state == nullptr) return _monitor.valid(); - - // can we assign a new state? - if (_monitor.valid()) - { - // assign the - _state.reset(state); - - // object is still valid - return true; - } - else - { - // otherwise the object is destructed and the new state should be destructed too - delete state; - - // object is no longer valid - return false; - } - } -}; - -/** - * End of namespace - */ -} - diff --git a/src/linux_tcp/tcpshutdown.h b/src/linux_tcp/tcpshutdown.h index 75f5c26..e6f3142 100644 --- a/src/linux_tcp/tcpshutdown.h +++ b/src/linux_tcp/tcpshutdown.h @@ -114,24 +114,6 @@ public: // move to next state return monitor.valid() ? new TcpClosed(this) : nullptr; } - - /** - * Abort the operation, immediately proceed to the final state - * @param monitor Monitor that can be used to check if the tcp connection is still alive - * @return TcpState New implementation object - */ - virtual TcpState *abort(const Monitor &monitor) override - { - // close the socket completely - cleanup(); - - // report the error to user-space - // @todo do we have to report this? - //_handler->onError(_connection, "tcp shutdown aborted"); - - // move to next state - return monitor.valid() ? new TcpClosed(this) : nullptr; - } }; /** diff --git a/src/linux_tcp/tcpstate.h b/src/linux_tcp/tcpstate.h index b9a250a..1f8a2da 100644 --- a/src/linux_tcp/tcpstate.h +++ b/src/linux_tcp/tcpstate.h @@ -110,13 +110,6 @@ public: */ virtual TcpState *flush(const Monitor &monitor) { return this; } - /** - * Abort the operation, immediately proceed to the final state - * @param monitor Monitor that can be used to check if the tcp connection is still alive - * @return TcpState New implementation object - */ - virtual TcpState *abort(const Monitor &monitor) = 0; - /** * Install max-frame size * @param heartbeat suggested heartbeat