Fix seed node threaded DNS lookup

Use copied value of seed node index during thread creation, not
reference.
- fixes segfault

Use boost:🧵: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.
This commit is contained in:
warptangent 2015-01-14 13:14:01 -08:00
parent 23c3d382c8
commit f74792b778
No known key found for this signature in database
GPG key ID: 0E490BEBFBE4E92D

View file

@ -259,54 +259,67 @@ namespace nodetool
std::vector<std::vector<std::string>> dns_results; std::vector<std::vector<std::string>> dns_results;
dns_results.resize(m_seed_nodes_list.size()); dns_results.resize(m_seed_nodes_list.size());
std::unique_ptr<std::atomic_flag[]> dns_finished(new std::atomic_flag[m_seed_nodes_list.size()]); std::list<boost::thread*> dns_threads;
// 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();
uint64_t result_index = 0; uint64_t result_index = 0;
for (const std::string& addr_str : m_seed_nodes_list) for (const std::string& addr_str : m_seed_nodes_list)
{ {
boost::thread* th = new boost::thread([=, &dns_results, &addr_str]
uint64_t result_index_capture = result_index++;
boost::thread t([&]
{ {
LOG_PRINT_L4("dns_threads[" << result_index << "] created for: " << addr_str)
// TODO: care about dnssec avail/valid // TODO: care about dnssec avail/valid
bool avail, valid; bool avail, valid;
std::vector<std::string> addr_list = tools::DNSResolver().get_ipv4(addr_str, avail, valid); std::vector<std::string> addr_list;
dns_results[result_index_capture] = addr_list; try
dns_finished[result_index_capture].clear(); {
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; LOG_PRINT_L4("dns_threads created, now waiting for completion or timeout of " << CRYPTONOTE_DNS_TIMEOUT_MS << "ms");
uint64_t sleep_interval_ms = 100; boost::chrono::system_clock::time_point deadline = boost::chrono::system_clock::now() + boost::chrono::milliseconds(CRYPTONOTE_DNS_TIMEOUT_MS);
while (sleep_count++ * sleep_interval_ms < 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)); if (! th->try_join_until(deadline))
bool all_done = false;
for (uint64_t i = 0; i < m_seed_nodes_list.size(); ++i)
{ {
if (dns_finished[i].test_and_set()) LOG_PRINT_L4("dns_threads[" << i << "] timed out, sending interrupt");
break; th->interrupt();
else
dns_finished[i].clear();
all_done = true;
} }
if (all_done) ++i;
break;
} }
i = 0;
for (const auto& result : dns_results) for (const auto& result : dns_results)
{
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())
{ {
for (const auto& addr_string : result) for (const auto& addr_string : result)
{ {
append_net_address(m_seed_nodes, addr_string + ":18080"); append_net_address(m_seed_nodes, addr_string + ":18080");
} }
} }
++i;
}
if (!m_seed_nodes.size()) if (!m_seed_nodes.size())
{ {