merge RC_1_1 into master

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 ":"

include/libtorrent/aux_/session_impl.hpp

@@ -266,10 +266,10 @@ namespace aux {
 			using connection_map = std::set<std::shared_ptr<peer_connection>>;
 			using torrent_map = std::unordered_map<sha1_hash, std::shared_ptr<torrent>>;
 
-			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<settings_pack> pack) override;
-			void apply_settings_pack_impl(settings_pack const& pack
+			void apply_settings_pack_impl(settings_pack const& pack);
-				, bool const init = false);
 			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<settings_pack> pack);
+			void init();
-			void init_dht();
+//			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<listen_endpoint_t>& eps);
 
-			// the settings for the client
-			aux::session_settings m_settings;
-
 			counters m_stats_counters;
 
 			// this is a pool allocator for torrent_peer objects

include/libtorrent/aux_/session_settings.hpp

@@ -68,6 +68,7 @@ namespace libtorrent { namespace aux {
 		{ return get<bool>(m_bools, name, settings_pack::bool_type_base); }
 
 		session_settings();
+		session_settings(settings_pack const&);
 
 	private:
 

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);

src/session.cpp

@@ -322,7 +322,7 @@ namespace aux {
 			ios = m_io_service.get();
 		}
 
-		m_impl = std::make_shared<aux::session_impl>(*ios);
+		m_impl = std::make_shared<aux::session_impl>(std::ref(*ios), std::ref(params.settings));
 		*static_cast<session_handle*>(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)
 		{

src/session_impl.cpp

@@ -391,12 +391,14 @@ namespace aux {
 #endif
 #endif
 
-	session_impl::session_impl(io_service& ios)
+	session_impl::session_impl(io_service& ios, settings_pack const& pack)
-		: m_io_service(ios)
+		: 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<unsigned int>(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 <typename Fun, typename... Args>
@@ -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<std::uint32_t>(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
+		m_io_service.post([this] { this->wrap(&session_impl::init); });
-		// if we're in C++14
-		auto copy = std::make_shared<settings_pack>(std::move(pack));
-		m_io_service.post([this, copy] { this->wrap(&session_impl::init, copy); });
 	}
 
-	void session_impl::init(std::shared_ptr<settings_pack> 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
+#if TORRENT_USE_INVARIANT_CHECKS
-		update_local_download_rate();
+		check_invariant();
-		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
 #endif
 	}
 
@@ -1348,8 +1295,7 @@ namespace {
 		return ret;
 	}
 
-	void session_impl::apply_settings_pack_impl(settings_pack const& pack
+	void session_impl::apply_settings_pack_impl(settings_pack const& pack)
-		, bool const init)
 	{
 		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"
+		session_log("applying settings pack, reopen_listen_port=%s"
-			, init ? "true" : "false", reopen_listen_port ? "true" : "false");
+			, 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<peer_connection*> unique_peers;
-		TORRENT_ASSERT(m_settings.get_int(settings_pack::connections_limit) > 0);
 
 		int unchokes = 0;
 		int unchokes_all = 0;

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);
+	}
 } }
 

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)

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;
+		time_point const now = clock_type::now();
-		int num = 120; // this number is adjusted per version, an estimate
+		if (now > end_time) return count;
-		time_point const end_time = clock_type::now() + seconds(15);
+
-		while (true)
+		ses.wait_for_alert(end_time - now);
+		std::vector<alert*> alerts;
+		ses.pop_alerts(&alerts);
+		for (auto a : alerts)
 		{
-			time_point const now = clock_type::now();
+			std::printf("%d: [%s] %s\n", num, a->what(), a->message().c_str());
-			if (now > end_time) return count;
+			if (a->type() == log_alert::alert_type)
-
-			ses.wait_for_alert(end_time - now);
-			std::vector<alert*> alerts;
-			ses.pop_alerts(&alerts);
-			for (auto a : alerts)
 			{
-				std::printf("%d: [%s] %s\n", num, a->what(), a->message().c_str());
+				std::string const msg = a->message();
-				if (a->type() == log_alert::alert_type)
+				if (msg.find("starting DHT, running: ") != std::string::npos)
-				{
+					count++;
-					std::string const msg = a->message();
-					if (msg.find("about to start DHT") != std::string::npos)
-						count++;
-				}
-				num--;
 			}
-			if (num <= 0) return count;
+			num--;
 		}
-		return count;
+		if (num <= 0) 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);
 	}
+	return count;
+};
 
-	{
+TORRENT_TEST(init_dht_default_bootstrap)
-		settings_pack p = settings();
+{
-		p.set_bool(settings_pack::enable_dht, true);
+	settings_pack p = settings();
-		p.set_int(settings_pack::alert_mask, alert::all_categories);
+	p.set_bool(settings_pack::enable_dht, true);
-		// no default value
+	p.set_int(settings_pack::alert_mask, alert::all_categories);
-		p.set_str(settings_pack::dht_bootstrap_nodes, "test.libtorrent.org:25401:8888");
+	// 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);
+	int const count = count_dht_inits(s);
-		TEST_EQUAL(count, 1);
+	TEST_EQUAL(count, 1);
-	}
+}
 
-	{
+TORRENT_TEST(init_dht_invalid_bootstrap)
-		settings_pack p = settings();
+{
-		p.set_bool(settings_pack::enable_dht, true);
+	settings_pack p = settings();
-		p.set_int(settings_pack::alert_mask, alert::all_categories);
+	p.set_bool(settings_pack::enable_dht, true);
-		// empty value
+	p.set_int(settings_pack::alert_mask, alert::all_categories);
-		p.set_str(settings_pack::dht_bootstrap_nodes, "");
+	// 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);
+	int const count = count_dht_inits(s);
-		TEST_EQUAL(count, 1);
+	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)