From 574b34c4c27b9da1f1ff4700adef97db0358a88f Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 25 Sep 2024 16:44:44 -0500 Subject: [PATCH 1/3] Resolve address on each re-connect --- plugins/net_plugin/net_plugin.cpp | 60 +++++++++++++++++-------------- 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index d19674035d..b1e22d1a81 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -348,7 +348,6 @@ namespace eosio { std::string host; connection_ptr c; tcp::endpoint active_ip; - tcp::resolver::results_type ips; }; using connection_details_index = multi_index_container< @@ -418,7 +417,7 @@ namespace eosio { string connect(const string& host, const string& p2p_address); string resolve_and_connect(const string& host, const string& p2p_address); void update_connection_endpoint(connection_ptr c, const tcp::endpoint& endpoint); - void connect(const connection_ptr& c); + void reconnect(const connection_ptr& c); string disconnect(const string& host); void close_all(); @@ -912,7 +911,7 @@ namespace eosio { fc::sha256 conn_node_id; string short_conn_node_id; - string listen_address; // address sent to peer in handshake + const string listen_address; // address sent to peer in handshake string log_p2p_address; string log_remote_endpoint_ip; string log_remote_endpoint_port; @@ -2764,6 +2763,10 @@ namespace eosio { fc_dlog( logger, "Skipping connect due to go_away reason ${r}",("r", reason_str( no_retry ))); return false; } + + if (incoming()) + return false; + if( consecutive_immediate_connection_close > def_max_consecutive_immediate_connection_close || no_retry == benign_other ) { fc::microseconds connector_period = my_impl->connections.get_connector_period(); fc::lock_guard g( conn_mtx ); @@ -2771,9 +2774,10 @@ namespace eosio { return true; // true so doesn't remove from valid connections } } + connection_ptr c = shared_from_this(); strand.post([c]() { - my_impl->connections.connect(c); + my_impl->connections.reconnect(c); }); return true; } @@ -4493,32 +4497,35 @@ namespace eosio { return "invalid peer address"; } - std::lock_guard g( connections_mtx ); + std::unique_lock g( connections_mtx ); if( find_connection_i( peer_address ) ) return "already connected"; + g.unlock(); auto [host, port, type] = split_host_port_type(peer_address); auto resolver = std::make_shared( my_impl->thread_pool.get_executor() ); resolver->async_resolve(host, port, - [resolver, host = host, port = port, peer_address = peer_address, listen_address = listen_address, this]( const boost::system::error_code& err, const tcp::resolver::results_type& results ) { + [this, resolver, host, port, peer_address, listen_address]( const boost::system::error_code& err, const tcp::resolver::results_type& results ) { connection_ptr c = std::make_shared( peer_address, listen_address ); - c->set_heartbeat_timeout( heartbeat_timeout ); - std::lock_guard g( connections_mtx ); - auto [it, inserted] = connections.emplace( connection_detail{ - .host = peer_address, - .c = std::move(c), - .ips = results + c->strand.post([this, resolver, c, err, results, host, port, peer_address]() { + c->set_heartbeat_timeout( heartbeat_timeout ); + std::unique_lock g( connections_mtx ); + connections.emplace( connection_detail{ + .host = peer_address, + .c = c, + }); + g.unlock(); + if( !err ) { + c->connect( results ); + } else { + fc_wlog( logger, "Unable to resolve ${host}:${port} ${error}", + ("host", host)("port", port)( "error", err.message() ) ); + c->set_state(connection::connection_state::closed); + ++(c->consecutive_immediate_connection_close); + } }); - if( !err ) { - it->c->connect( results ); - } else { - fc_wlog( logger, "Unable to resolve ${host}:${port} ${error}", - ("host", host)("port", port)( "error", err.message() ) ); - it->c->set_state(connection::connection_state::closed); - ++(it->c->consecutive_immediate_connection_close); - } } ); return "added connection"; @@ -4536,13 +4543,12 @@ namespace eosio { } } - void connections_manager::connect(const connection_ptr& c) { - std::lock_guard g( connections_mtx ); - const auto& index = connections.get(); - const auto& it = index.find(c); - if( it != index.end() ) { - it->c->connect( it->ips ); - } + void connections_manager::reconnect(const connection_ptr& c) { + std::unique_lock g( connections_mtx ); + auto& index = connections.get(); + index.erase(c); + g.unlock(); + resolve_and_connect(c->peer_address(), c->listen_address); } // called by API From b8e8d4784b0b2f23b8200b1860adbe2715d36a9b Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 25 Sep 2024 17:20:48 -0500 Subject: [PATCH 2/3] Use lock_guard with scope instead of unique_lock --- plugins/net_plugin/net_plugin.cpp | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index b1e22d1a81..539086a27b 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -4497,10 +4497,11 @@ namespace eosio { return "invalid peer address"; } - std::unique_lock g( connections_mtx ); - if( find_connection_i( peer_address ) ) - return "already connected"; - g.unlock(); + { + std::lock_guard g(connections_mtx); + if (find_connection_i(peer_address)) + return "already connected"; + } auto [host, port, type] = split_host_port_type(peer_address); @@ -4511,12 +4512,13 @@ namespace eosio { connection_ptr c = std::make_shared( peer_address, listen_address ); c->strand.post([this, resolver, c, err, results, host, port, peer_address]() { c->set_heartbeat_timeout( heartbeat_timeout ); - std::unique_lock g( connections_mtx ); - connections.emplace( connection_detail{ - .host = peer_address, - .c = c, - }); - g.unlock(); + { + std::lock_guard g( connections_mtx ); + connections.emplace( connection_detail{ + .host = peer_address, + .c = c, + }); + } if( !err ) { c->connect( results ); } else { @@ -4544,10 +4546,11 @@ namespace eosio { } void connections_manager::reconnect(const connection_ptr& c) { - std::unique_lock g( connections_mtx ); - auto& index = connections.get(); - index.erase(c); - g.unlock(); + { + std::lock_guard g( connections_mtx ); + auto& index = connections.get(); + index.erase(c); + } resolve_and_connect(c->peer_address(), c->listen_address); } From 51fd8faba27ecee6487e4f947198a419f323dab3 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 25 Sep 2024 18:16:37 -0500 Subject: [PATCH 3/3] Maintain the reuse of connection object on reconnect --- plugins/net_plugin/net_plugin.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 539086a27b..59f3219068 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -415,7 +415,7 @@ namespace eosio { void add(connection_ptr c); string connect(const string& host, const string& p2p_address); - string resolve_and_connect(const string& host, const string& p2p_address); + string resolve_and_connect(const string& host, const string& p2p_address, const connection_ptr& c = {}); void update_connection_endpoint(connection_ptr c, const tcp::endpoint& endpoint); void reconnect(const connection_ptr& c); string disconnect(const string& host); @@ -4490,7 +4490,11 @@ namespace eosio { return resolve_and_connect( host, p2p_address ); } - string connections_manager::resolve_and_connect( const string& peer_address, const string& listen_address ) { + string connections_manager::resolve_and_connect( const string& peer_address, const string& listen_address, + const connection_ptr& c ) + { + assert(!c || (c->peer_address() == peer_address && c->listen_address == listen_address)); + string::size_type colon = peer_address.find(':'); if (colon == std::string::npos || colon == 0) { fc_elog( logger, "Invalid peer address. must be \"host:port[:|]\": ${p}", ("p", peer_address) ); @@ -4508,8 +4512,8 @@ namespace eosio { auto resolver = std::make_shared( my_impl->thread_pool.get_executor() ); resolver->async_resolve(host, port, - [this, resolver, host, port, peer_address, listen_address]( const boost::system::error_code& err, const tcp::resolver::results_type& results ) { - connection_ptr c = std::make_shared( peer_address, listen_address ); + [this, resolver, c_org{c}, host, port, peer_address, listen_address]( const boost::system::error_code& err, const tcp::resolver::results_type& results ) { + connection_ptr c = c_org ? c_org : std::make_shared( peer_address, listen_address ); c->strand.post([this, resolver, c, err, results, host, port, peer_address]() { c->set_heartbeat_timeout( heartbeat_timeout ); { @@ -4551,7 +4555,7 @@ namespace eosio { auto& index = connections.get(); index.erase(c); } - resolve_and_connect(c->peer_address(), c->listen_address); + resolve_and_connect(c->peer_address(), c->listen_address, c); } // called by API