plugin callback refactor (#823)

grouping plugins per feature

include/libtorrent/alert_manager.hpp

@@ -167,7 +167,7 @@ namespace libtorrent {
 		aux::stack_allocator m_allocations[2];
 
 #ifndef TORRENT_DISABLE_EXTENSIONS
-		std::list<boost::shared_ptr<plugin> > m_ses_extensions;
+		std::list<boost::shared_ptr<plugin>> m_ses_extensions;
 #endif
 	};
 }

include/libtorrent/aux_/session_impl.hpp

@@ -189,8 +189,14 @@ namespace libtorrent
 		{
 			// the size of each allocation that is chained in the send buffer
 			enum { send_buffer_size_impl = 128 };
-			// maximum length of query names which can be registered by extensions
+			// plugin feature-index key map
-			enum { max_dht_query_length = 15 };
+			enum
+			{
+				plugins_all_idx = 0, // to store all plugins
+				plugins_optimistic_unchoke_idx = 1, // optimistic_unchoke_feature
+				plugins_tick_idx = 2, // tick_feature
+				plugins_dht_request_idx = 3 // dht_request_feature
+			};
 
 #if TORRENT_USE_INVARIANT_CHECKS
 			friend class libtorrent::invariant_access;
@@ -1155,22 +1161,7 @@ namespace libtorrent
 
 #ifndef TORRENT_DISABLE_EXTENSIONS
 			// this is a list to allow extensions to potentially remove themselves.
-			std::vector<boost::shared_ptr<plugin> > m_ses_extensions;
+			std::array<std::vector<boost::shared_ptr<plugin>>, 4> m_ses_extensions;
-
-			// the union of all session extensions' implemented_features(). This is
-			// used to exclude callbacks to the session extensions.
-			boost::uint32_t m_session_extension_features;
-
-			// std::string could be used for the query names if only all common
-			// implementations used SSO *glares at gcc*
-			struct extension_dht_query
-			{
-				boost::uint8_t query_len;
-				std::array<char, max_dht_query_length> query;
-				dht_extension_handler_t handler;
-			};
-			typedef std::vector<extension_dht_query> m_extension_dht_queries_t;
-			m_extension_dht_queries_t m_extension_dht_queries;
 #endif
 
 			// if this function is set, it indicates that torrents are allowed

include/libtorrent/aux_/session_interface.hpp

@@ -293,7 +293,7 @@ namespace libtorrent { namespace aux
 			// when recalculating auto-managed torrents. started auto managed
 			// torrents that are inactive are not part of these lists, because they
 			// are not considered for auto managing (they are left started
-			// unconditionallty)
+			// unconditionally)
 			torrent_downloading_auto_managed,
 			torrent_seeding_auto_managed,
 			torrent_checking_auto_managed,

include/libtorrent/extensions.hpp

@@ -187,14 +187,6 @@ namespace libtorrent
 	struct peer_connection_handle;
 	struct torrent_handle;
 
-	// Functions of this type are called to handle incoming DHT requests
-	typedef boost::function<bool(udp::endpoint const& source
-		, bdecode_node const& request, entry& response)> dht_extension_handler_t;
-
-	// Map of query strings to handlers. Note that query strings are limited to 15 bytes.
-	// see max_dht_query_length
-	typedef std::vector<std::pair<std::string, dht_extension_handler_t> > dht_extensions_t;
-
 	// this is the base class for a session plugin. One primary feature
 	// is that it is notified of all torrents that are added to the session,
 	// and can add its own torrent_plugins.
@@ -213,7 +205,15 @@ namespace libtorrent
 			optimistic_unchoke_feature = 1,
 
 			// include this bit if your plugin needs to have on_tick() called
-			tick_feature = 2
+			tick_feature = 2,
+
+			// include this bit if your plugin needs to have on_dht_request()
+			// called
+			dht_request_feature = 4,
+
+			// include this bit if your plugin needs to have on_alert()
+			// called
+			alert_feature = 8,
 		};
 
 		// This function is expected to return a bitmask indicating which features
@@ -238,11 +238,17 @@ namespace libtorrent
 		// called when plugin is added to a session
 		virtual void added(session_handle) {}
 
-		// called after a plugin is added
+		// called when a dht request is received.
-		// allows the plugin to register DHT requests it would like to handle
+		// If your plugin expects this to be called, make sure to include the flag
-		virtual void register_dht_extensions(dht_extensions_t&) {}
+		// ``dht_request_feature`` in the return value from implemented_features().
+		virtual bool on_dht_request(char const* /* query */, int const /* query_len */
+			, udp::endpoint const& /* source */, bdecode_node const& /* message */
+			, entry& /* response */)
+		{ return false; }
 
-		// called when an alert is posted alerts that are filtered are not posted
+		// called when an alert is posted alerts that are filtered are not posted.
+		// If your plugin expects this to be called, make sure to include the flag
+		// ``alert_feature`` in the return value from implemented_features().
 		virtual void on_alert(alert const*) {}
 
 		// return true if the add_torrent_params should be added
@@ -250,7 +256,9 @@ namespace libtorrent
 			, peer_connection_handle const& /* pc */, add_torrent_params& /* p */)
 		{ return false; }
 
-		// called once per second
+		// called once per second.
+		// If your plugin expects this to be called, make sure to include the flag
+		// ``tick_feature`` in the return value from implemented_features().
 		virtual void on_tick() {}
 
 		// called when choosing peers to optimistically unchoke. peer's will be

include/libtorrent/socket_io.hpp

@@ -51,8 +51,7 @@ namespace libtorrent
 	TORRENT_EXTRA_EXPORT tcp::endpoint parse_endpoint(std::string str, error_code& ec);
 
 	TORRENT_EXTRA_EXPORT std::string address_to_bytes(address const& a);
-	// internal
+	TORRENT_EXTRA_EXPORT std::string endpoint_to_bytes(udp::endpoint const& ep);
-	TORRENT_EXPORT std::string endpoint_to_bytes(udp::endpoint const& ep);
 	TORRENT_EXTRA_EXPORT void hash_address(address const& ip, sha1_hash& h);
 
 	namespace detail

src/session_impl.cpp

@@ -396,9 +396,6 @@ namespace aux {
 		, m_download_connect_attempts(0)
 		, m_next_scrape_torrent(0)
 		, m_tick_residual(0)
-#ifndef TORRENT_DISABLE_EXTENSIONS
-		, m_session_extension_features(0)
-#endif
 		, m_deferred_submit_disk_jobs(false)
 		, m_pending_auto_manage(false)
 		, m_need_auto_manage(false)
@@ -635,7 +632,7 @@ namespace aux {
 #endif
 
 #ifndef TORRENT_DISABLE_EXTENSIONS
-		for (auto& ext : m_ses_extensions)
+		for (auto& ext : m_ses_extensions[plugins_all_idx])
 		{
 			TORRENT_TRY {
 				ext->save_state(*eptr);
@@ -782,7 +779,7 @@ namespace aux {
 #endif
 
 #ifndef TORRENT_DISABLE_EXTENSIONS
-		for (auto& ext : m_ses_extensions)
+		for (auto& ext : m_ses_extensions[plugins_all_idx])
 		{
 			TORRENT_TRY {
 				ext->load_state(*e);
@@ -811,8 +808,7 @@ namespace aux {
 
 		boost::shared_ptr<plugin> p(new session_plugin_wrapper(ext));
 
-		m_ses_extensions.push_back(p);
+		add_ses_extension(p);
-		m_session_extension_features |= p->implemented_features();
 	}
 
 	void session_impl::add_ses_extension(boost::shared_ptr<plugin> ext)
@@ -820,27 +816,19 @@ namespace aux {
 		TORRENT_ASSERT(is_single_thread());
 		TORRENT_ASSERT_VAL(ext, ext);
 
-		m_ses_extensions.push_back(ext);
+		boost::uint32_t const features = ext->implemented_features();
-		m_alerts.add_extension(ext);
-		ext->added(session_handle(this));
-		m_session_extension_features |= ext->implemented_features();
 
-		// get any DHT queries the plugin would like to handle
+		m_ses_extensions[plugins_all_idx].push_back(ext);
-		// and record them in m_extension_dht_queries for lookup
+
-		// later
+		if (features & plugin::optimistic_unchoke_feature)
-		dht_extensions_t dht_ext;
+			m_ses_extensions[plugins_optimistic_unchoke_idx].push_back(ext);
-		ext->register_dht_extensions(dht_ext);
+		if (features & plugin::tick_feature)
-		for (dht_extensions_t::iterator e = dht_ext.begin();
+			m_ses_extensions[plugins_tick_idx].push_back(ext);
-			e != dht_ext.end(); ++e)
+		if (features & plugin::dht_request_feature)
-		{
+			m_ses_extensions[plugins_dht_request_idx].push_back(ext);
-			TORRENT_ASSERT(e->first.size() <= max_dht_query_length);
+		if (features & plugin::alert_feature)
-			if (e->first.size() > max_dht_query_length) continue;
+			m_alerts.add_extension(ext);
-			extension_dht_query registration;
+		ext->added(session_handle(this));
-			registration.query_len = uint8_t(e->first.size());
-			std::copy(e->first.begin(), e->first.end(), registration.query.begin());
-			registration.handler = e->second;
-			m_extension_dht_queries.push_back(registration);
-		}
 	}
 
 #endif // TORRENT_DISABLE_EXTENSIONS
@@ -3140,14 +3128,11 @@ namespace aux {
 		}
 
 #ifndef TORRENT_DISABLE_EXTENSIONS
-		if (m_session_extension_features & plugin::tick_feature)
+		for (auto& ext : m_ses_extensions[plugins_tick_idx])
 		{
-			for (auto& ext : m_ses_extensions)
+			TORRENT_TRY {
-			{
+				ext->on_tick();
-				TORRENT_TRY {
+			} TORRENT_CATCH(std::exception&) {}
-					ext->on_tick();
-				} TORRENT_CATCH(std::exception&) {}
-			}
 		}
 #endif
 
@@ -3828,7 +3813,8 @@ namespace aux {
 			, opt_unchoke.end(), &last_optimistic_unchoke_cmp);
 
 #ifndef TORRENT_DISABLE_EXTENSIONS
-		if (m_session_extension_features & plugin::optimistic_unchoke_feature)
+		auto const& plugins = m_ses_extensions[plugins_optimistic_unchoke_idx];
+		if (plugins.size() > 0)
 		{
 			// if there is an extension that wants to reorder the optimistic
 			// unchoke peers, first convert the vector into one containing
@@ -3840,7 +3826,7 @@ namespace aux {
 			{
 				peers.push_back(peer_connection_handle(static_cast<peer_connection*>((*i)->connection)->self()));
 			}
-			for (auto& e : m_ses_extensions)
+			for (auto& e : plugins)
 			{
 				if (e->on_optimistic_unchoke(peers))
 					break;
@@ -4231,7 +4217,7 @@ namespace aux {
 		, peer_connection* pc)
 	{
 #ifndef TORRENT_DISABLE_EXTENSIONS
-		for (auto& e : m_ses_extensions)
+		for (auto& e : m_ses_extensions[plugins_all_idx])
 		{
 			add_torrent_params p;
 			if (e->on_unknown_torrent(info_hash, peer_connection_handle(pc->self()), p))
@@ -4630,7 +4616,7 @@ namespace aux {
 	void session_impl::add_extensions_to_torrent(
 		boost::shared_ptr<torrent> const& torrent_ptr, void* userdata)
 	{
-		for (auto& e : m_ses_extensions)
+		for (auto& e : m_ses_extensions[plugins_all_idx])
 		{
 			boost::shared_ptr<torrent_plugin> tp(e->new_torrent(
 				torrent_ptr->get_handle(), userdata));
@@ -6618,14 +6604,10 @@ namespace aux {
 		, dht::msg const& request, entry& response)
 	{
 #ifndef TORRENT_DISABLE_EXTENSIONS
-		if (query_len > max_dht_query_length) return false;
+		for (auto& ext : m_ses_extensions[plugins_dht_request_idx])
-
-		for (m_extension_dht_queries_t::iterator i = m_extension_dht_queries.begin();
-			i != m_extension_dht_queries.end(); ++i)
 		{
-			if (query_len == i->query_len
+			if (ext->on_dht_request(query, query_len
-				&& memcmp(i->query.data(), query, query_len) == 0
+				, request.addr, request.message, response))
-				&& i->handler(request.addr, request.message, response))
 				return true;
 		}
 #else

test/test_direct_dht.cpp

@@ -47,15 +47,16 @@ namespace
 
 struct test_plugin : plugin
 {
-	virtual void register_dht_extensions(dht_extensions_t& ext)
+	virtual boost::uint32_t implemented_features()
 	{
-		ext.push_back(dht_extensions_t::value_type("test_good", &good_response));
+		return plugin::dht_request_feature;
 	}
 
-	static bool good_response(udp::endpoint const& source
+	virtual bool on_dht_request(char const* /* query */, int const /* query_len */
-		, bdecode_node const& request, entry& response)
+		, udp::endpoint const& /* source */, bdecode_node const& message
+		, entry& response)
 	{
-		if (request.dict_find_string_value("q") == "test_good")
+		if (message.dict_find_string_value("q") == "test_good")
 		{
 			response["r"]["good"] = 1;
 			return true;