Protect udp_channel members with a mutex (#1930)

* Protect udp_channel members with a mutex

* Remove return in void function
This commit is contained in:
cryptocode 2019-04-26 13:42:56 +02:00 committed by GitHub
commit 3fd69b643c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 76 additions and 26 deletions

View file

@ -92,9 +92,9 @@ TEST (network, send_node_id_handshake)
ASSERT_EQ (1, system.nodes[0]->network.size ());
ASSERT_EQ (1, node1->network.size ());
auto list1 (system.nodes[0]->network.udp_channels.list (1));
ASSERT_EQ (node1->network.endpoint (), list1[0]->endpoint);
ASSERT_EQ (node1->network.endpoint (), list1[0]->get_endpoint ());
auto list2 (node1->network.udp_channels.list (1));
ASSERT_EQ (system.nodes[0]->network.endpoint (), list2[0]->endpoint);
ASSERT_EQ (system.nodes[0]->network.endpoint (), list2[0]->get_endpoint ());
node1->stop ();
}
@ -120,14 +120,14 @@ TEST (network, last_contacted)
auto channel2 (system.nodes[0]->network.udp_channels.channel (nano::endpoint (boost::asio::ip::address_v6::loopback (), 24001)));
ASSERT_NE (nullptr, channel2);
// Make sure last_contact gets updated on receiving a non-handshake message
auto timestamp_before_keepalive = channel2->last_packet_received;
auto timestamp_before_keepalive = channel2->get_last_packet_received ();
node1->network.send_keepalive (channel1);
while (system.nodes[0]->stats.count (nano::stat::type::message, nano::stat::detail::keepalive, nano::stat::dir::in) < 2)
{
ASSERT_NO_ERROR (system.poll ());
}
ASSERT_EQ (system.nodes[0]->network.size (), 1);
auto timestamp_after_keepalive = channel2->last_packet_received;
auto timestamp_after_keepalive = channel2->get_last_packet_received ();
ASSERT_GT (timestamp_after_keepalive, timestamp_before_keepalive);
node1->stop ();
@ -2014,11 +2014,11 @@ TEST (network, replace_port)
auto channel (system.nodes[0]->network.udp_channels.insert (nano::endpoint (node1->network.endpoint ().address (), 23000), nano::protocol_version));
if (channel)
{
channel->node_id = node1->node_id.pub;
channel->set_node_id (node1->node_id.pub);
}
}
auto peers_list (system.nodes[0]->network.udp_channels.list (std::numeric_limits<size_t>::max ()));
ASSERT_EQ (peers_list[0]->node_id.get (), node1->node_id.pub);
ASSERT_EQ (peers_list[0]->get_node_id ().get (), node1->node_id.pub);
nano::transport::channel_udp channel (system.nodes[0]->network.udp_channels, node1->network.endpoint ());
system.nodes[0]->network.send_keepalive (channel);
system.deadline_set (5s);
@ -2033,9 +2033,9 @@ TEST (network, replace_port)
}
ASSERT_EQ (system.nodes[0]->network.udp_channels.size (), 1);
auto list1 (system.nodes[0]->network.udp_channels.list (1));
ASSERT_EQ (node1->network.endpoint (), list1[0]->endpoint);
ASSERT_EQ (node1->network.endpoint (), list1[0]->get_endpoint ());
auto list2 (node1->network.udp_channels.list (1));
ASSERT_EQ (system.nodes[0]->network.endpoint (), list2[0]->endpoint);
ASSERT_EQ (system.nodes[0]->network.endpoint (), list2[0]->get_endpoint ());
// Remove correct peer (same node ID)
system.nodes[0]->network.udp_channels.clean_node_id (nano::endpoint (node1->network.endpoint ().address (), 23000), node1->node_id.pub);
ASSERT_EQ (system.nodes[0]->network.udp_channels.size (), 0);

View file

@ -2396,10 +2396,10 @@ TEST (node, peers)
// Confirm that the peers match with the endpoints we are expecting
ASSERT_EQ (1, system.nodes.front ()->network.size ());
auto list1 (system.nodes[0]->network.udp_channels.list (2));
ASSERT_EQ (system.nodes[1]->network.endpoint (), list1[0]->endpoint);
ASSERT_EQ (system.nodes[1]->network.endpoint (), list1[0]->get_endpoint ());
ASSERT_EQ (1, node->network.size ());
auto list2 (system.nodes[1]->network.udp_channels.list (2));
ASSERT_EQ (system.nodes[0]->network.endpoint (), list2[0]->endpoint);
ASSERT_EQ (system.nodes[0]->network.endpoint (), list2[0]->get_endpoint ());
// Stop the peer node and check that it is removed from the store
system.nodes.front ()->stop ();

View file

@ -58,11 +58,11 @@ TEST (peer_container, split)
nano::endpoint endpoint2 (boost::asio::ip::address_v6::loopback (), 101);
auto channel1 (system.nodes[0]->network.udp_channels.insert (endpoint1, 0));
ASSERT_NE (nullptr, channel1);
channel1->last_packet_received = now - std::chrono::seconds (1);
channel1->set_last_packet_received (now - std::chrono::seconds (1));
system.nodes[0]->network.udp_channels.modify (channel1);
auto channel2 (system.nodes[0]->network.udp_channels.insert (endpoint2, 0));
ASSERT_NE (nullptr, channel2);
channel2->last_packet_received = now + std::chrono::seconds (1);
channel2->set_last_packet_received (now + std::chrono::seconds (1));
system.nodes[0]->network.udp_channels.modify (channel2);
ASSERT_EQ (2, system.nodes[0]->network.size ());
ASSERT_EQ (2, system.nodes[0]->network.udp_channels.size ());
@ -70,7 +70,7 @@ TEST (peer_container, split)
ASSERT_EQ (1, system.nodes[0]->network.size ());
ASSERT_EQ (1, system.nodes[0]->network.udp_channels.size ());
auto list (system.nodes[0]->network.udp_channels.list (1));
ASSERT_EQ (endpoint2, list[0]->endpoint);
ASSERT_EQ (endpoint2, list[0]->get_endpoint ());
}
TEST (udp_channels, fill_random_clear)

View file

@ -2431,9 +2431,10 @@ void nano::json_handler::peers ()
{
boost::property_tree::ptree pending_tree;
pending_tree.put ("protocol_version", std::to_string (channel->network_version));
if (channel->node_id.is_initialized ())
auto node_id_l (channel->get_node_id ());
if (node_id_l.is_initialized ())
{
pending_tree.put ("node_id", channel->node_id.get ().to_account ());
pending_tree.put ("node_id", node_id_l.get ().to_account ());
}
else
{

View file

@ -323,7 +323,7 @@ nano::endpoint nano::transport::udp_channels::tcp_peer ()
{
result = i->endpoint ();
channels.get<last_tcp_attempt_tag> ().modify (i, [](channel_udp_wrapper & wrapper_a) {
wrapper_a.channel->last_tcp_attempt = std::chrono::steady_clock::now ();
wrapper_a.channel->set_last_tcp_attempt (std::chrono::steady_clock::now ());
});
i = n;
}
@ -493,7 +493,7 @@ public:
auto channel (node.network.udp_channels.insert (endpoint, message_a.header.version_using));
if (channel)
{
channel->node_id = message_a.response->first;
channel->set_node_id (message_a.response->first);
}
}
}
@ -517,7 +517,7 @@ public:
auto channel (node.network.udp_channels.channel (endpoint));
if (channel)
{
channel->last_packet_received = std::chrono::steady_clock::now ();
channel->set_last_packet_received (std::chrono::steady_clock::now ());
node.network.udp_channels.modify (channel);
node.process_message (message_a, channel);
}

View file

@ -1,5 +1,6 @@
#pragma once
#include <mutex>
#include <nano/node/common.hpp>
#include <nano/node/stats.hpp>
#include <nano/node/transport/transport.hpp>
@ -25,10 +26,56 @@ namespace transport
{
return &channels == &other_a.channels && endpoint == other_a.endpoint;
}
nano::endpoint get_endpoint () const
{
std::lock_guard<std::mutex> lk (channel_mutex);
return endpoint;
}
std::chrono::steady_clock::time_point get_last_tcp_attempt () const
{
std::lock_guard<std::mutex> lk (channel_mutex);
return last_tcp_attempt;
}
void set_last_tcp_attempt (std::chrono::steady_clock::time_point const time_a)
{
std::lock_guard<std::mutex> lk (channel_mutex);
last_tcp_attempt = time_a;
}
std::chrono::steady_clock::time_point get_last_packet_received () const
{
std::lock_guard<std::mutex> lk (channel_mutex);
return last_packet_received;
}
void set_last_packet_received (std::chrono::steady_clock::time_point const time_a)
{
std::lock_guard<std::mutex> lk (channel_mutex);
last_packet_received = time_a;
}
boost::optional<nano::account> get_node_id () const
{
std::lock_guard<std::mutex> lk (channel_mutex);
return node_id;
}
void set_node_id (nano::account node_id_a)
{
std::lock_guard<std::mutex> lk (channel_mutex);
node_id = node_id_a;
}
unsigned network_version{ nano::protocol_version };
private:
mutable std::mutex channel_mutex;
nano::endpoint endpoint;
std::chrono::steady_clock::time_point last_tcp_attempt{ std::chrono::steady_clock::time_point () };
std::chrono::steady_clock::time_point last_packet_received{ std::chrono::steady_clock::time_point () };
unsigned network_version{ nano::protocol_version };
boost::optional<nano::account> node_id{ boost::none };
private:
@ -108,15 +155,15 @@ namespace transport
std::shared_ptr<nano::transport::channel_udp> channel;
nano::endpoint endpoint () const
{
return channel->endpoint;
return channel->get_endpoint ();
}
std::chrono::steady_clock::time_point last_packet_received () const
{
return channel->last_packet_received;
return channel->get_last_packet_received ();
}
std::chrono::steady_clock::time_point last_tcp_attempt () const
{
return channel->last_tcp_attempt;
return channel->get_last_tcp_attempt ();
}
boost::asio::ip::address ip_address () const
{
@ -124,9 +171,10 @@ namespace transport
}
nano::account node_id () const
{
if (channel->node_id.is_initialized ())
auto node_id_l (channel->get_node_id ());
if (node_id_l.is_initialized ())
{
return channel->node_id.get ();
return node_id_l.get ();
}
else
{

View file

@ -1942,9 +1942,10 @@ void nano_qt::advanced_actions::refresh_peers ()
version->setData (QVariant (channel->network_version), Qt::DisplayRole);
items.push_back (version);
QString node_id ("");
if (channel->node_id.is_initialized ())
auto node_id_l (channel->get_node_id ());
if (node_id_l.is_initialized ())
{
node_id = channel->node_id.get ().to_account ().c_str ();
node_id = node_id_l.get ().to_account ().c_str ();
}
items.push_back (new QStandardItem (node_id));
peers_model->appendRow (items);