diff --git a/ChangeLog b/ChangeLog index 0c14be8c8..131902523 100644 --- a/ChangeLog +++ b/ChangeLog @@ -81,6 +81,7 @@ * resume data no longer has timestamps of files * require C++11 to build libtorrent + * fix issue with initializing settings on session construction * fix issue with receiving interested before metadata * fix IPv6 tracker announce issue * restore path sanitization behavior of ":" diff --git a/include/libtorrent/aux_/session_impl.hpp b/include/libtorrent/aux_/session_impl.hpp index 1c422efaa..f134df8e4 100644 --- a/include/libtorrent/aux_/session_impl.hpp +++ b/include/libtorrent/aux_/session_impl.hpp @@ -266,10 +266,10 @@ namespace aux { using connection_map = std::set>; using torrent_map = std::unordered_map>; - explicit session_impl(io_service& ios); + session_impl(io_service& ios, settings_pack const& pack); ~session_impl() override; - void start_session(settings_pack pack); + void start_session(); void init_peer_class_filter(bool unlimited_local); @@ -369,8 +369,7 @@ namespace aux { void close_connection(peer_connection* p) override; void apply_settings_pack(std::shared_ptr pack) override; - void apply_settings_pack_impl(settings_pack const& pack - , bool const init = false); + void apply_settings_pack_impl(settings_pack const& pack); session_settings const& settings() const override { return m_settings; } settings_pack get_settings() const; @@ -708,6 +707,9 @@ namespace aux { void inc_boost_connections() override { ++m_boost_connections; } + // the settings for the client + aux::session_settings m_settings; + #ifndef TORRENT_NO_DEPRECATE void update_ssl_listen(); void update_dht_upload_rate_limit(); @@ -765,8 +767,8 @@ namespace aux { peer_class_pool m_classes; - void init(std::shared_ptr pack); - void init_dht(); + void init(); +// void init_dht(); void submit_disk_jobs(); @@ -781,9 +783,6 @@ namespace aux { void interface_to_endpoints(std::string const& device, int port , transport ssl, duplex incoming, std::vector& eps); - // the settings for the client - aux::session_settings m_settings; - counters m_stats_counters; // this is a pool allocator for torrent_peer objects diff --git a/include/libtorrent/aux_/session_settings.hpp b/include/libtorrent/aux_/session_settings.hpp index c50afcdbb..b912263f9 100644 --- a/include/libtorrent/aux_/session_settings.hpp +++ b/include/libtorrent/aux_/session_settings.hpp @@ -68,6 +68,7 @@ namespace libtorrent { namespace aux { { return get(m_bools, name, settings_pack::bool_type_base); } session_settings(); + session_settings(settings_pack const&); private: diff --git a/include/libtorrent/settings_pack.hpp b/include/libtorrent/settings_pack.hpp index 6c3ebfdcf..16403658e 100644 --- a/include/libtorrent/settings_pack.hpp +++ b/include/libtorrent/settings_pack.hpp @@ -65,6 +65,7 @@ namespace libtorrent { TORRENT_EXTRA_EXPORT void save_settings_to_dict(aux::session_settings const& s, entry::dictionary_type& sett); TORRENT_EXTRA_EXPORT void apply_pack(settings_pack const* pack, aux::session_settings& sett , aux::session_impl* ses = nullptr); + TORRENT_EXTRA_EXPORT void run_all_updates(aux::session_impl& ses); TORRENT_EXPORT int setting_by_name(std::string const& name); TORRENT_EXPORT char const* name_for_setting(int s); diff --git a/src/session.cpp b/src/session.cpp index b2cfd24dc..6effa2496 100644 --- a/src/session.cpp +++ b/src/session.cpp @@ -322,7 +322,7 @@ namespace aux { ios = m_io_service.get(); } - m_impl = std::make_shared(*ios); + m_impl = std::make_shared(std::ref(*ios), std::ref(params.settings)); *static_cast(this) = session_handle(m_impl); #ifndef TORRENT_DISABLE_EXTENSIONS @@ -338,7 +338,7 @@ namespace aux { m_impl->set_dht_storage(params.dht_storage_constructor); #endif - m_impl->start_session(std::move(params.settings)); + m_impl->start_session(); if (internal_executor) { diff --git a/src/session_impl.cpp b/src/session_impl.cpp index 9adbcf611..5ba644e83 100644 --- a/src/session_impl.cpp +++ b/src/session_impl.cpp @@ -391,12 +391,14 @@ namespace aux { #endif #endif - session_impl::session_impl(io_service& ios) - : m_io_service(ios) + session_impl::session_impl(io_service& ios, settings_pack const& pack) + : m_settings(pack) + , m_io_service(ios) #ifdef TORRENT_USE_OPENSSL , m_ssl_ctx(m_io_service, boost::asio::ssl::context::sslv23) #endif - , m_alerts(m_settings.get_int(settings_pack::alert_queue_size), alert::all_categories) + , m_alerts(m_settings.get_int(settings_pack::alert_queue_size) + , alert_category_t{static_cast(m_settings.get_int(settings_pack::alert_mask))}) , m_disk_thread(m_io_service, m_stats_counters) , m_download_rate(peer_connection::download_channel) , m_upload_rate(peer_connection::upload_channel) @@ -441,6 +443,7 @@ namespace aux { , m_close_file_timer(m_io_service) { update_time_now(); + m_disk_thread.set_settings(&pack); } template @@ -468,14 +471,8 @@ namespace aux { // io_service thread. // TODO: 2 is there a reason not to move all of this into init()? and just // post it to the io_service? - void session_impl::start_session(settings_pack pack) + void session_impl::start_session() { - if (pack.has_val(settings_pack::alert_mask)) - { - m_alerts.set_alert_mask(alert_category_t( - static_cast(pack.get_int(settings_pack::alert_mask)))); - } - #ifndef TORRENT_DISABLE_LOGGING session_log("start session"); #endif @@ -542,15 +539,11 @@ namespace aux { } #endif - // TODO: 3 make this move all the way through to the init() call - // if we're in C++14 - auto copy = std::make_shared(std::move(pack)); - m_io_service.post([this, copy] { this->wrap(&session_impl::init, copy); }); + m_io_service.post([this] { this->wrap(&session_impl::init); }); } - void session_impl::init(std::shared_ptr pack) + void session_impl::init() { - INVARIANT_CHECK; // this is a debug facility // see single_threaded in debug.hpp thread_started(); @@ -588,62 +581,16 @@ namespace aux { session_log(" done starting session"); #endif - apply_settings_pack_impl(*pack, true); + // this applies unchoke settings from m_settings + recalculate_unchoke_slots(); - // call update_* after settings set initialized + // apply all m_settings to this session + run_all_updates(*this); + reopen_listen_sockets(); + reopen_outgoing_sockets(); -#ifndef TORRENT_NO_DEPRECATE - update_local_download_rate(); - update_local_upload_rate(); -#endif - update_download_rate(); - update_upload_rate(); - update_connections_limit(); - update_unchoke_limit(); - update_disk_threads(); - update_resolver_cache_timeout(); - update_ip_notifier(); - update_upnp(); - update_natpmp(); - update_lsd(); - update_peer_fingerprint(); - - 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 +#if TORRENT_USE_INVARIANT_CHECKS + check_invariant(); #endif } @@ -1348,8 +1295,7 @@ namespace { return ret; } - void session_impl::apply_settings_pack_impl(settings_pack const& pack - , bool const init) + void session_impl::apply_settings_pack_impl(settings_pack const& pack) { bool const reopen_listen_port = #ifndef TORRENT_NO_DEPRECATE @@ -1371,26 +1317,26 @@ namespace { != m_settings.get_str(settings_pack::outgoing_interfaces)); #ifndef TORRENT_DISABLE_LOGGING - session_log("applying settings pack, init=%s, reopen_listen_port=%s" - , init ? "true" : "false", reopen_listen_port ? "true" : "false"); + session_log("applying settings pack, reopen_listen_port=%s" + , reopen_listen_port ? "true" : "false"); #endif apply_pack(&pack, m_settings, this); m_disk_thread.set_settings(&pack); - if (init && !reopen_listen_port) + if (!reopen_listen_port) { // no need to call this if reopen_listen_port is true // since the apply_pack will do it update_listen_interfaces(); } - if (init || reopen_listen_port) + if (reopen_listen_port) { reopen_listen_sockets(); } - if (init || reopen_outgoing_port) + if (reopen_outgoing_port) reopen_outgoing_sockets(); } @@ -4237,7 +4183,6 @@ namespace { void session_impl::recalculate_unchoke_slots() { TORRENT_ASSERT(is_single_thread()); - INVARIANT_CHECK; time_point const now = aux::time_now(); time_duration const unchoke_interval = now - m_last_choke; @@ -5712,18 +5657,31 @@ namespace { { INVARIANT_CHECK; -#ifndef TORRENT_DISABLE_LOGGING - session_log("about to start DHT, running: %s, router lookups: %d, aborting: %s" - , m_dht ? "true" : "false", m_outstanding_router_lookups - , m_abort ? "true" : "false"); -#endif - stop_dht(); // postpone starting the DHT if we're still resolving the DHT router - if (m_outstanding_router_lookups > 0) return; + if (m_outstanding_router_lookups > 0) + { +#ifndef TORRENT_DISABLE_LOGGING + session_log("not starting DHT, outstanding router lookups: %d" + , m_outstanding_router_lookups); +#endif + return; + } - if (m_abort) return; + if (m_abort) + { +#ifndef TORRENT_DISABLE_LOGGING + session_log("not starting DHT, aborting"); +#endif + return; + } + +#ifndef TORRENT_DISABLE_LOGGING + session_log("starting DHT, running: %s, router lookups: %d, aborting: %s" + , m_dht ? "true" : "false", m_outstanding_router_lookups + , m_abort ? "true" : "false"); +#endif // TODO: refactor, move the storage to dht_tracker m_dht_storage = m_dht_storage_constructor(m_dht_settings); @@ -7019,7 +6977,6 @@ namespace { TORRENT_ASSERT(num_active_finished == int(m_torrent_lists[torrent_want_peers_finished].size())); std::unordered_set unique_peers; - TORRENT_ASSERT(m_settings.get_int(settings_pack::connections_limit) > 0); int unchokes = 0; int unchokes_all = 0; diff --git a/src/session_settings.cpp b/src/session_settings.cpp index 527521b3b..f4c7b8917 100644 --- a/src/session_settings.cpp +++ b/src/session_settings.cpp @@ -31,6 +31,7 @@ POSSIBILITY OF SUCH DAMAGE. */ #include "libtorrent/aux_/session_settings.hpp" +#include "libtorrent/settings_pack.hpp" namespace libtorrent { namespace aux { @@ -38,5 +39,11 @@ namespace libtorrent { namespace aux { { initialize_default_settings(*this); } + + session_settings::session_settings(settings_pack const& p) + { + initialize_default_settings(*this); + apply_pack(&p, *this); + } } } diff --git a/src/settings_pack.cpp b/src/settings_pack.cpp index 5d34eab44..fdd3cb2af 100644 --- a/src/settings_pack.cpp +++ b/src/settings_pack.cpp @@ -447,6 +447,28 @@ constexpr int CLOSE_FILE_INTERVAL = 0; } } + void run_all_updates(aux::session_impl& ses) + { + typedef void (aux::session_impl::*fun_t)(); + for (int i = 0; i < settings_pack::num_string_settings; ++i) + { + fun_t const& f = str_settings[i].fun; + if (f) (ses.*f)(); + } + + for (int i = 0; i < settings_pack::num_int_settings; ++i) + { + fun_t const& f = int_settings[i].fun; + if (f) (ses.*f)(); + } + + for (int i = 0; i < settings_pack::num_bool_settings; ++i) + { + fun_t const& f = bool_settings[i].fun; + if (f) (ses.*f)(); + } + } + void initialize_default_settings(aux::session_settings& s) { for (int i = 0; i < settings_pack::num_string_settings; ++i) diff --git a/test/test_session.cpp b/test/test_session.cpp index a152aafcd..c6c4cb800 100644 --- a/test/test_session.cpp +++ b/test/test_session.cpp @@ -441,75 +441,76 @@ TORRENT_TEST(save_state_peer_id) } #ifndef TORRENT_DISABLE_LOGGING -TORRENT_TEST(init_dht) + +auto const count_dht_inits = [](session& ses) { - auto count_dht_inits = [](session& ses) + int count = 0; + int num = 120; // this number is adjusted per version, an estimate + time_point const end_time = clock_type::now() + seconds(15); + while (true) { - int count = 0; - int num = 120; // 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) { - 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::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--; + std::string const msg = a->message(); + if (msg.find("starting DHT, running: ") != std::string::npos) + count++; } - if (num <= 0) return count; + num--; } - 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); + 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); - // no default value - p.set_str(settings_pack::dht_bootstrap_nodes, "test.libtorrent.org:25401:8888"); +TORRENT_TEST(init_dht_default_bootstrap) +{ + 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); + lt::session s(p); - int const count = count_dht_inits(s); - TEST_EQUAL(count, 1); - } + 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, ""); +TORRENT_TEST(init_dht_invalid_bootstrap) +{ + 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); + lt::session s(p); - int const count = count_dht_inits(s); - TEST_EQUAL(count, 1); - } + int const count = count_dht_inits(s); + TEST_EQUAL(count, 1); +} + +TORRENT_TEST(init_dht_empty_bootstrap) +{ + 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); } TORRENT_TEST(reopen_network_sockets)