From fcbcc250bbe0cd6325b50c58cdff965c387391cc Mon Sep 17 00:00:00 2001 From: Alden Torres Date: Thu, 23 Mar 2017 08:32:56 -0400 Subject: [PATCH] fix setup of DHT logic in session_impl::init (#1830) fix setup of DHT logic in session_impl::init --- include/libtorrent/aux_/session_impl.hpp | 1 + src/session_impl.cpp | 41 +++++++++++++- test/test_session.cpp | 70 ++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 3 deletions(-) diff --git a/include/libtorrent/aux_/session_impl.hpp b/include/libtorrent/aux_/session_impl.hpp index 3898e619b..cff9114de 100644 --- a/include/libtorrent/aux_/session_impl.hpp +++ b/include/libtorrent/aux_/session_impl.hpp @@ -737,6 +737,7 @@ namespace libtorrent peer_class_pool m_classes; void init(std::shared_ptr pack); + void init_dht(); void submit_disk_jobs(); diff --git a/src/session_impl.cpp b/src/session_impl.cpp index 172033960..8b6394d13 100644 --- a/src/session_impl.cpp +++ b/src/session_impl.cpp @@ -630,10 +630,45 @@ namespace aux { update_upnp(); update_natpmp(); update_lsd(); - update_dht(); update_peer_fingerprint(); - update_dht_bootstrap_nodes(); - update_dht_announce_interval(); + + init_dht(); + } + + void session_impl::init_dht() + { + // the need of this elaborated logic is because if the value + // of settings_pack::dht_bootstrap_nodes is not the default, + // then update_dht_bootstrap_nodes is called. For this reason, + // three different cases should be considered. + // 1-) dht_bootstrap_nodes setting not touched + // 2-) dht_bootstrap_nodes changed but not empty + // 3-) dht_bootstrap_nodes set to empty ("") + // TODO: find a solution and refactor to avoid potentially stalling + // for minutes due to the name resolution + +#ifndef TORRENT_DISABLE_DHT + if (m_outstanding_router_lookups == 0) + { + // this can happens because either the setting value was untouched + // or the value in the initial settings is empty + if (m_settings.get_str(settings_pack::dht_bootstrap_nodes).empty()) + { + // case 3) + update_dht(); + update_dht_announce_interval(); + } + else + { + // case 1) + // eventually update_dht() is called when all resolves are done + update_dht_bootstrap_nodes(); + } + } + // else is case 2) + // in this case the call to update_dht_bootstrap_nodes() by the apply settings + // will eventually call update_dht() when all resolves are done +#endif } void session_impl::async_resolve(std::string const& host, int flags diff --git a/test/test_session.cpp b/test/test_session.cpp index fad229c2e..c66ef3836 100644 --- a/test/test_session.cpp +++ b/test/test_session.cpp @@ -394,3 +394,73 @@ TORRENT_TEST(save_state_peer_id) TEST_CHECK(pid3[5] == 'r'); } +TORRENT_TEST(init_dht) +{ + auto count_dht_inits = [](session& ses) + { + int count = 0; + int num = 70; // this number is adjusted per version, an estimate + time_point const end_time = clock_type::now() + seconds(15); + while (true) + { + time_point const now = clock_type::now(); + if (now > end_time) return count; + + ses.wait_for_alert(end_time - now); + std::vector alerts; + ses.pop_alerts(&alerts); + for (auto a : alerts) + { + std::printf("%d: [%s] %s\n", num, a->what(), a->message().c_str()); + if (a->type() == log_alert::alert_type) + { + std::string const msg = a->message(); + if (msg.find("about to start DHT") != std::string::npos) + count++; + } + num--; + } + if (num <= 0) return count; + } + return count; + }; + + { + settings_pack p = settings(); + p.set_bool(settings_pack::enable_dht, true); + p.set_int(settings_pack::alert_mask, alert::all_categories); + // default value + p.set_str(settings_pack::dht_bootstrap_nodes, "dht.libtorrent.org:25401"); + + lt::session s(p); + + int const count = count_dht_inits(s); + TEST_EQUAL(count, 1); + } + + { + settings_pack p = settings(); + p.set_bool(settings_pack::enable_dht, true); + p.set_int(settings_pack::alert_mask, alert::all_categories); + // no default value + p.set_str(settings_pack::dht_bootstrap_nodes, "test.libtorrent.org:25401:8888"); + + lt::session s(p); + + int const count = count_dht_inits(s); + TEST_EQUAL(count, 1); + } + + { + settings_pack p = settings(); + p.set_bool(settings_pack::enable_dht, true); + p.set_int(settings_pack::alert_mask, alert::all_categories); + // empty value + p.set_str(settings_pack::dht_bootstrap_nodes, ""); + + lt::session s(p); + + int const count = count_dht_inits(s); + TEST_EQUAL(count, 1); + } +}