From 4a53898764d961374f0874ab9029e9b7c2897b75 Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Mon, 15 Dec 2014 17:23:42 -0500 Subject: DNS seed timeout and fallback --- src/p2p/net_node.inl | 70 +++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 59 insertions(+), 11 deletions(-) (limited to 'src/p2p/net_node.inl') diff --git a/src/p2p/net_node.inl b/src/p2p/net_node.inl index 01fbaa497..c0aaad8ef 100644 --- a/src/p2p/net_node.inl +++ b/src/p2p/net_node.inl @@ -31,6 +31,9 @@ #pragma once #include +#include +#include +#include #include "version.h" #include "string_tools.h" @@ -46,13 +49,13 @@ // We have to look for miniupnpc headers in different places, dependent on if its compiled or external #ifdef UPNP_STATIC - #include - #include - #include + #include + #include + #include #else - #include "miniupnpc.h" - #include "upnpcommands.h" - #include "upnperrors.h" + #include "miniupnpc.h" + #include "upnpcommands.h" + #include "upnperrors.h" #endif #define NET_MAKE_IP(b1,b2,b3,b4) ((LPARAM)(((DWORD)(b1)<<24)+((DWORD)(b2)<<16)+((DWORD)(b3)<<8)+((DWORD)(b4)))) @@ -252,19 +255,64 @@ namespace nodetool // add the result addresses as seed nodes // TODO: at some point add IPv6 support, but that won't be relevant // for some time yet. + + std::vector> dns_results; + + std::vector dns_finished; + + dns_results.resize(m_seed_nodes_list.size()); + dns_finished.resize(m_seed_nodes_list.size()); + + // set each flag, thread will release when finished + for (auto& u : dns_finished) + u.test_and_set(); + + uint64_t result_index = 0; for (const std::string& addr_str : m_seed_nodes_list) { - // TODO: care about dnssec avail/valid - bool avail, valid; - std::vector addr_list = tools::DNSResolver::instance().get_ipv4(addr_str, avail, valid); - for (const std::string& a : addr_list) + + uint64_t result_index_capture = result_index++; + boost::thread t([&] + { + // TODO: care about dnssec avail/valid + bool avail, valid; + std::vector addr_list = tools::DNSResolver().get_ipv4(addr_str, avail, valid); + + dns_results[result_index_capture] = addr_list; + dns_finished[result_index_capture].clear(); + }); + + } + + uint64_t sleep_count = 0; + uint64_t sleep_interval_ms = 100; + while (sleep_count++ * sleep_interval_ms < CRYPTONOTE_DNS_TIMEOUT_MS) + { + boost::this_thread::sleep(boost::posix_time::milliseconds()); + bool all_done = false; + for (auto& done : dns_finished) + { + if (done.test_and_set()) + break; + else + done.clear(); + all_done = true; + } + if (all_done) + break; + } + + for (const auto& result : dns_results) + { + for (const auto& addr_string : result) { - append_net_address(m_seed_nodes, a + ":18080"); + append_net_address(m_seed_nodes, addr_string + ":18080"); } } if (!m_seed_nodes.size()) { + LOG_PRINT_L0("DNS seed node lookup either timed out or failed, falling back to defaults"); append_net_address(m_seed_nodes, "62.210.78.186:18080"); append_net_address(m_seed_nodes, "195.12.60.154:18080"); append_net_address(m_seed_nodes, "54.241.246.125:18080"); -- cgit v1.2.3 From df53c0a595cb472f78954deeb293d983bdf02002 Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Mon, 15 Dec 2014 17:28:11 -0500 Subject: small typo in previous commit --- src/p2p/net_node.inl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/p2p/net_node.inl') diff --git a/src/p2p/net_node.inl b/src/p2p/net_node.inl index c0aaad8ef..ee2d3974f 100644 --- a/src/p2p/net_node.inl +++ b/src/p2p/net_node.inl @@ -288,7 +288,7 @@ namespace nodetool uint64_t sleep_interval_ms = 100; while (sleep_count++ * sleep_interval_ms < CRYPTONOTE_DNS_TIMEOUT_MS) { - boost::this_thread::sleep(boost::posix_time::milliseconds()); + boost::this_thread::sleep(boost::posix_time::milliseconds(sleep_interval_ms)); bool all_done = false; for (auto& done : dns_finished) { -- cgit v1.2.3 From 1b462261b88da00f20c743b8638969b8f1ff2a19 Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Mon, 15 Dec 2014 17:43:12 -0500 Subject: std::atomic_flag has no copy/move constructor, can't have a vector --- src/p2p/net_node.inl | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) (limited to 'src/p2p/net_node.inl') diff --git a/src/p2p/net_node.inl b/src/p2p/net_node.inl index ee2d3974f..17a81db79 100644 --- a/src/p2p/net_node.inl +++ b/src/p2p/net_node.inl @@ -257,15 +257,13 @@ namespace nodetool // for some time yet. std::vector> dns_results; - - std::vector dns_finished; - dns_results.resize(m_seed_nodes_list.size()); - dns_finished.resize(m_seed_nodes_list.size()); + + std::unique_ptr dns_finished(new std::atomic_flag[m_seed_nodes_list.size()]); // set each flag, thread will release when finished - for (auto& u : dns_finished) - u.test_and_set(); + for (uint64_t i = 0; i < m_seed_nodes_list.size(); ++i) + dns_finished[i].test_and_set(); uint64_t result_index = 0; for (const std::string& addr_str : m_seed_nodes_list) @@ -290,12 +288,12 @@ namespace nodetool { boost::this_thread::sleep(boost::posix_time::milliseconds(sleep_interval_ms)); bool all_done = false; - for (auto& done : dns_finished) + for (uint64_t i = 0; i < m_seed_nodes_list.size(); ++i) { - if (done.test_and_set()) + if (dns_finished[i].test_and_set()) break; else - done.clear(); + dns_finished[i].clear(); all_done = true; } if (all_done) -- cgit v1.2.3 From f74792b77881318087312afb27a12dd26d2edb6e Mon Sep 17 00:00:00 2001 From: warptangent Date: Wed, 14 Jan 2015 13:14:01 -0800 Subject: Fix seed node threaded DNS lookup Use copied value of seed node index during thread creation, not reference. - fixes segfault Use boost::thread::try_join_until() instead of an atomic flag result variable for each thread. Add and handle interrupt for thread timeout. - fixes segfault where a thread exceeds requested timeout and tries to assign results to a referenced, but now out-of-scope, variable in the main thread. --- src/p2p/net_node.inl | 69 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 41 insertions(+), 28 deletions(-) (limited to 'src/p2p/net_node.inl') diff --git a/src/p2p/net_node.inl b/src/p2p/net_node.inl index 3454e112c..ee4a10789 100644 --- a/src/p2p/net_node.inl +++ b/src/p2p/net_node.inl @@ -259,53 +259,66 @@ namespace nodetool std::vector> dns_results; dns_results.resize(m_seed_nodes_list.size()); - std::unique_ptr dns_finished(new std::atomic_flag[m_seed_nodes_list.size()]); - - // set each flag, thread will release when finished - for (uint64_t i = 0; i < m_seed_nodes_list.size(); ++i) - dns_finished[i].test_and_set(); - + std::list dns_threads; uint64_t result_index = 0; for (const std::string& addr_str : m_seed_nodes_list) { - - uint64_t result_index_capture = result_index++; - boost::thread t([&] + boost::thread* th = new boost::thread([=, &dns_results, &addr_str] { + LOG_PRINT_L4("dns_threads[" << result_index << "] created for: " << addr_str) // TODO: care about dnssec avail/valid bool avail, valid; - std::vector addr_list = tools::DNSResolver().get_ipv4(addr_str, avail, valid); - - dns_results[result_index_capture] = addr_list; - dns_finished[result_index_capture].clear(); + std::vector addr_list; + + try + { + addr_list = tools::DNSResolver().get_ipv4(addr_str, avail, valid); + LOG_PRINT_L4("dns_threads[" << result_index << "] DNS resolve done"); + boost::this_thread::interruption_point(); + } + catch(const boost::thread_interrupted&) + { + // thread interruption request + // even if we now have results, finish thread without setting + // result variables, which are now out of scope in main thread + LOG_PRINT_L4("dns_threads[" << result_index << "] interrupted"); + return; + } + + LOG_PRINT_L4("dns_threads[" << result_index << "] addr_str: " << addr_str << " number of results: " << addr_list.size()); + dns_results[result_index] = addr_list; }); + dns_threads.push_back(th); + ++result_index; } - uint64_t sleep_count = 0; - uint64_t sleep_interval_ms = 100; - while (sleep_count++ * sleep_interval_ms < CRYPTONOTE_DNS_TIMEOUT_MS) + LOG_PRINT_L4("dns_threads created, now waiting for completion or timeout of " << CRYPTONOTE_DNS_TIMEOUT_MS << "ms"); + boost::chrono::system_clock::time_point deadline = boost::chrono::system_clock::now() + boost::chrono::milliseconds(CRYPTONOTE_DNS_TIMEOUT_MS); + uint64_t i = 0; + for (boost::thread* th : dns_threads) { - boost::this_thread::sleep(boost::posix_time::milliseconds(sleep_interval_ms)); - bool all_done = false; - for (uint64_t i = 0; i < m_seed_nodes_list.size(); ++i) + if (! th->try_join_until(deadline)) { - if (dns_finished[i].test_and_set()) - break; - else - dns_finished[i].clear(); - all_done = true; + LOG_PRINT_L4("dns_threads[" << i << "] timed out, sending interrupt"); + th->interrupt(); } - if (all_done) - break; + ++i; } + i = 0; for (const auto& result : dns_results) { - for (const auto& addr_string : result) + LOG_PRINT_L4("DNS lookup for " << m_seed_nodes_list[i] << ": " << result.size() << " results"); + // if no results for node, thread's lookup likely timed out + if (result.size()) { - append_net_address(m_seed_nodes, addr_string + ":18080"); + for (const auto& addr_string : result) + { + append_net_address(m_seed_nodes, addr_string + ":18080"); + } } + ++i; } if (!m_seed_nodes.size()) -- cgit v1.2.3