From 5a3cd673e9ce8cd63fd1bb5df6fc522bb9cac34d Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 3 Jul 2023 10:04:57 -0500 Subject: [PATCH 1/4] GH-1349 Close on async_read closed socket. Always shutdown socket. --- plugins/net_plugin/net_plugin.cpp | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 963f1a8bb8..2e4952dabd 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -1022,10 +1022,8 @@ namespace eosio { void connection::_close( connection* self, bool reconnect, bool shutdown ) { self->socket_open = false; boost::system::error_code ec; - if( self->socket->is_open() ) { - self->socket->shutdown( tcp::socket::shutdown_both, ec ); - self->socket->close( ec ); - } + self->socket->shutdown( tcp::socket::shutdown_both, ec ); + self->socket->close( ec ); self->socket.reset( new tcp::socket( my_impl->thread_pool.get_executor() ) ); self->flush_queues(); self->connecting = false; @@ -2489,7 +2487,18 @@ namespace eosio { boost::asio::bind_executor( strand, [conn = shared_from_this(), socket=socket]( boost::system::error_code ec, std::size_t bytes_transferred ) { // may have closed connection and cleared pending_message_buffer - if( !conn->socket_is_open() || socket != conn->socket ) return; + if (!conn->socket_is_open() && conn->socket_open) { // if socket_open then close not called + peer_dlog( conn, "async_read socket not open, closing"); + conn->close(); + return; + } + if (socket != conn->socket ) { // different socket, conn must have created a new socket, make sure previous is closed + peer_dlog( conn, "async_read diff socket closing"); + boost::system::error_code ec; + socket->shutdown( tcp::socket::shutdown_both, ec ); + socket->close( ec ); + return; + } bool close_connection = false; try { From d46d7e903ad51bca5ec1584b12b1fd33525a5766 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 3 Jul 2023 10:05:23 -0500 Subject: [PATCH 2/4] GH-1349 Cleanup duplicate check --- plugins/net_plugin/net_plugin.cpp | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 2e4952dabd..bd93128d96 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -1050,6 +1050,7 @@ namespace eosio { self->cancel_wait(); self->latest_msg_time = std::chrono::system_clock::time_point::min(); self->latest_blk_time = std::chrono::system_clock::time_point::min(); + self->org = std::chrono::nanoseconds{0}; if( reconnect && !shutdown ) { my_impl->start_conn_timer( std::chrono::milliseconds( 100 ), connection_wptr() ); @@ -2851,6 +2852,7 @@ namespace eosio { peer_lib_num = msg.last_irreversible_block_num; std::unique_lock g_conn( conn_mtx ); last_handshake_recv = msg; + auto c_time = last_handshake_sent.time; g_conn.unlock(); connecting = false; @@ -2876,14 +2878,9 @@ namespace eosio { return; } - if( peer_address().empty() ) { + if( incoming() ) { set_connection_type( msg.p2p_address ); - } - std::unique_lock g_conn( conn_mtx ); - if( peer_address().empty() || last_handshake_recv.node_id == fc::sha256()) { - auto c_time = last_handshake_sent.time; - g_conn.unlock(); peer_dlog( this, "checking for duplicate" ); std::shared_lock g_cnts( my_impl->connections_mtx ); for(const auto& check : my_impl->connections) { @@ -2929,9 +2926,7 @@ namespace eosio { } } } else { - peer_dlog( this, "skipping duplicate check, addr == ${pa}, id = ${ni}", - ("pa", peer_address())( "ni", last_handshake_recv.node_id ) ); - g_conn.unlock(); + peer_dlog(this, "skipping duplicate check, addr == ${pa}, id = ${ni}", ("pa", peer_address())("ni", msg.node_id)); } if( msg.chain_id != my_impl->chain_id ) { From bd7ff14a6136219ff2f9c902718d63e892dc7a83 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 3 Jul 2023 20:15:08 -0500 Subject: [PATCH 3/4] GH-1349 If socket closed, but close not call, call close --- plugins/net_plugin/net_plugin.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index bd93128d96..1f051eca00 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -1242,7 +1242,7 @@ namespace eosio { try { c->buffer_queue.clear_out_queue(); // May have closed connection and cleared buffer_queue - if( !c->socket_is_open() || socket != c->socket ) { + if( !c->socket->is_open() || socket != c->socket ) { peer_ilog( c, "async write socket ${r} before callback", ("r", c->socket_is_open() ? "changed" : "closed") ); c->close(); return; @@ -2488,7 +2488,7 @@ namespace eosio { boost::asio::bind_executor( strand, [conn = shared_from_this(), socket=socket]( boost::system::error_code ec, std::size_t bytes_transferred ) { // may have closed connection and cleared pending_message_buffer - if (!conn->socket_is_open() && conn->socket_open) { // if socket_open then close not called + if (!conn->socket->is_open() && conn->socket_is_open()) { // if socket_open then close not called peer_dlog( conn, "async_read socket not open, closing"); conn->close(); return; From 59b0d08749ee864d0c431ca2f68a9147a03085a5 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 5 Jul 2023 07:42:17 -0500 Subject: [PATCH 4/4] GH-1349 close mismatched socket --- plugins/net_plugin/net_plugin.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 1f051eca00..7271d393cc 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -1242,11 +1242,18 @@ namespace eosio { try { c->buffer_queue.clear_out_queue(); // May have closed connection and cleared buffer_queue - if( !c->socket->is_open() || socket != c->socket ) { - peer_ilog( c, "async write socket ${r} before callback", ("r", c->socket_is_open() ? "changed" : "closed") ); + if (!c->socket->is_open() && c->socket_is_open()) { // if socket_open then close not called + peer_ilog(c, "async write socket closed before callback"); c->close(); return; } + if (socket != c->socket ) { // different socket, c must have created a new socket, make sure previous is closed + peer_ilog( c, "async write socket changed before callback"); + boost::system::error_code ec; + socket->shutdown( tcp::socket::shutdown_both, ec ); + socket->close( ec ); + return; + } if( ec ) { if( ec.value() != boost::asio::error::eof ) {