Skip to content

Commit

Permalink
[ASAN] Access node through a weak_ptr on distributed_work dtor (#2554)
Browse files Browse the repository at this point in the history
* [ASAN] Access node through a weak_ptr on distributed_work dtor

Since there is nothing to force a distributed_work to get destroyed before the node stops, this is a safer way to access the node on the destructor. Fixes some ASAN issues.

* Add a comment explaining the use of node_w

* [ASAN] fix issue in rpc.work_peer_many
  • Loading branch information
guilhermelawless committed Feb 12, 2020
1 parent 0c664bd commit 7ad1b49
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 18 deletions.
30 changes: 17 additions & 13 deletions nano/node/distributed_work.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ std::shared_ptr<request_type> nano::distributed_work::peer_request::get_prepared

nano::distributed_work::distributed_work (nano::node & node_a, nano::work_request const & request_a, std::chrono::seconds const & backoff_a) :
node (node_a),
node_w (node_a.shared ()),
request (request_a),
backoff (backoff_a),
strand (node_a.io_ctx.get_executor ()),
Expand All @@ -35,23 +36,26 @@ elapsed (nano::timer_state::started, "distributed work generation timer")
nano::distributed_work::~distributed_work ()
{
assert (status != work_generation_status::ongoing);
if (!node.stopped && node.websocket_server && node.websocket_server->any_subscriber (nano::websocket::topic::work))
if (auto node_l = node_w.lock ())
{
nano::websocket::message_builder builder;
if (status == work_generation_status::success)
if (!node_l->stopped && node_l->websocket_server && node_l->websocket_server->any_subscriber (nano::websocket::topic::work))
{
node.websocket_server->broadcast (builder.work_generation (request.root, work_result, request.difficulty, node.network_params.network.publish_threshold, elapsed.value (), winner, bad_peers));
}
else if (status == work_generation_status::cancelled)
{
node.websocket_server->broadcast (builder.work_cancelled (request.root, request.difficulty, node.network_params.network.publish_threshold, elapsed.value (), bad_peers));
}
else if (status == work_generation_status::failure_local || status == work_generation_status::failure_peers)
{
node.websocket_server->broadcast (builder.work_failed (request.root, request.difficulty, node.network_params.network.publish_threshold, elapsed.value (), bad_peers));
nano::websocket::message_builder builder;
if (status == work_generation_status::success)
{
node_l->websocket_server->broadcast (builder.work_generation (request.root, work_result, request.difficulty, node_l->network_params.network.publish_threshold, elapsed.value (), winner, bad_peers));
}
else if (status == work_generation_status::cancelled)
{
node_l->websocket_server->broadcast (builder.work_cancelled (request.root, request.difficulty, node_l->network_params.network.publish_threshold, elapsed.value (), bad_peers));
}
else if (status == work_generation_status::failure_local || status == work_generation_status::failure_peers)
{
node_l->websocket_server->broadcast (builder.work_failed (request.root, request.difficulty, node_l->network_params.network.publish_threshold, elapsed.value (), bad_peers));
}
}
stop_once (true);
}
stop_once (true);
}

void nano::distributed_work::start ()
Expand Down
2 changes: 2 additions & 0 deletions nano/node/distributed_work.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ class distributed_work final : public std::enable_shared_from_this<nano::distrib
void add_bad_peer (nano::tcp_endpoint const &);

nano::node & node;
// Only used in destructor, as the node reference can become invalid before distributed_work objects go out of scope
std::weak_ptr<nano::node> node_w;
nano::work_request request;

std::chrono::seconds backoff;
Expand Down
9 changes: 4 additions & 5 deletions nano/rpc_test/rpc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3118,15 +3118,14 @@ TEST (rpc, work_peer_many)
node1.config.work_peers.push_back (std::make_pair (node3.network.endpoint ().address ().to_string (), rpc3.config.port));
node1.config.work_peers.push_back (std::make_pair (node4.network.endpoint ().address ().to_string (), rpc4.config.port));

for (auto i (0); i < 10; ++i)
std::array<std::atomic<uint64_t>, 10> works;
for (auto i (0); i < works.size (); ++i)
{
nano::keypair key1;
std::atomic<uint64_t> work (0);
node1.work_generate (key1.pub, [&work](boost::optional<uint64_t> work_a) {
ASSERT_TRUE (work_a.is_initialized ());
node1.work_generate (key1.pub, [& work = works[i]](boost::optional<uint64_t> work_a) {
work = *work_a;
});
while (nano::work_validate (key1.pub, work))
while (nano::work_validate (key1.pub, works[i]))
{
system1.poll ();
system2.poll ();
Expand Down

0 comments on commit 7ad1b49

Please sign in to comment.