From d3cd5684479ecff9de316ece5c8669dd12b4dcd3 Mon Sep 17 00:00:00 2001 From: Alden Torres Date: Fri, 17 Jun 2016 22:02:21 -0400 Subject: [PATCH] plugin callback refactor (#823) grouping plugins per feature --- include/libtorrent/alert_manager.hpp | 2 +- include/libtorrent/aux_/session_impl.hpp | 27 +++---- include/libtorrent/aux_/session_interface.hpp | 2 +- include/libtorrent/extensions.hpp | 36 ++++++---- include/libtorrent/socket_io.hpp | 3 +- src/session_impl.cpp | 72 +++++++------------ test/test_direct_dht.cpp | 11 +-- 7 files changed, 67 insertions(+), 86 deletions(-) diff --git a/include/libtorrent/alert_manager.hpp b/include/libtorrent/alert_manager.hpp index 2adb843c5..f28994281 100644 --- a/include/libtorrent/alert_manager.hpp +++ b/include/libtorrent/alert_manager.hpp @@ -167,7 +167,7 @@ namespace libtorrent { aux::stack_allocator m_allocations[2]; #ifndef TORRENT_DISABLE_EXTENSIONS - std::list > m_ses_extensions; + std::list> m_ses_extensions; #endif }; } diff --git a/include/libtorrent/aux_/session_impl.hpp b/include/libtorrent/aux_/session_impl.hpp index 36dc6152c..b6216cea4 100644 --- a/include/libtorrent/aux_/session_impl.hpp +++ b/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 - enum { max_dht_query_length = 15 }; + // plugin feature-index key map + 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 > 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 query; - dht_extension_handler_t handler; - }; - typedef std::vector m_extension_dht_queries_t; - m_extension_dht_queries_t m_extension_dht_queries; + std::array>, 4> m_ses_extensions; #endif // if this function is set, it indicates that torrents are allowed diff --git a/include/libtorrent/aux_/session_interface.hpp b/include/libtorrent/aux_/session_interface.hpp index 72e16fa4c..cbc8d1182 100644 --- a/include/libtorrent/aux_/session_interface.hpp +++ b/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, diff --git a/include/libtorrent/extensions.hpp b/include/libtorrent/extensions.hpp index 395a88bb2..7f3e7ac2d 100644 --- a/include/libtorrent/extensions.hpp +++ b/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 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 > 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 - // allows the plugin to register DHT requests it would like to handle - virtual void register_dht_extensions(dht_extensions_t&) {} + // called when a dht request is received. + // If your plugin expects this to be called, make sure to include the flag + // ``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 diff --git a/include/libtorrent/socket_io.hpp b/include/libtorrent/socket_io.hpp index 18a812ae8..9eb7db930 100644 --- a/include/libtorrent/socket_io.hpp +++ b/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_EXPORT std::string endpoint_to_bytes(udp::endpoint const& ep); + TORRENT_EXTRA_EXPORT std::string endpoint_to_bytes(udp::endpoint const& ep); TORRENT_EXTRA_EXPORT void hash_address(address const& ip, sha1_hash& h); namespace detail diff --git a/src/session_impl.cpp b/src/session_impl.cpp index 3b076bd83..f87adea66 100644 --- a/src/session_impl.cpp +++ b/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 p(new session_plugin_wrapper(ext)); - m_ses_extensions.push_back(p); - m_session_extension_features |= p->implemented_features(); + add_ses_extension(p); } void session_impl::add_ses_extension(boost::shared_ptr ext) @@ -820,27 +816,19 @@ namespace aux { TORRENT_ASSERT(is_single_thread()); TORRENT_ASSERT_VAL(ext, ext); - m_ses_extensions.push_back(ext); - m_alerts.add_extension(ext); - ext->added(session_handle(this)); - m_session_extension_features |= ext->implemented_features(); + boost::uint32_t const features = ext->implemented_features(); - // get any DHT queries the plugin would like to handle - // and record them in m_extension_dht_queries for lookup - // later - dht_extensions_t dht_ext; - ext->register_dht_extensions(dht_ext); - for (dht_extensions_t::iterator e = dht_ext.begin(); - e != dht_ext.end(); ++e) - { - TORRENT_ASSERT(e->first.size() <= max_dht_query_length); - if (e->first.size() > max_dht_query_length) continue; - extension_dht_query registration; - 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); - } + m_ses_extensions[plugins_all_idx].push_back(ext); + + if (features & plugin::optimistic_unchoke_feature) + m_ses_extensions[plugins_optimistic_unchoke_idx].push_back(ext); + if (features & plugin::tick_feature) + m_ses_extensions[plugins_tick_idx].push_back(ext); + if (features & plugin::dht_request_feature) + m_ses_extensions[plugins_dht_request_idx].push_back(ext); + if (features & plugin::alert_feature) + m_alerts.add_extension(ext); + ext->added(session_handle(this)); } #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_CATCH(std::exception&) {} - } + TORRENT_TRY { + 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((*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 const& torrent_ptr, void* userdata) { - for (auto& e : m_ses_extensions) + for (auto& e : m_ses_extensions[plugins_all_idx]) { boost::shared_ptr 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 (m_extension_dht_queries_t::iterator i = m_extension_dht_queries.begin(); - i != m_extension_dht_queries.end(); ++i) + for (auto& ext : m_ses_extensions[plugins_dht_request_idx]) { - if (query_len == i->query_len - && memcmp(i->query.data(), query, query_len) == 0 - && i->handler(request.addr, request.message, response)) + if (ext->on_dht_request(query, query_len + , request.addr, request.message, response)) return true; } #else diff --git a/test/test_direct_dht.cpp b/test/test_direct_dht.cpp index 206be5c65..dc0be451a 100644 --- a/test/test_direct_dht.cpp +++ b/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 - , bdecode_node const& request, entry& response) + virtual bool on_dht_request(char const* /* query */, int const /* query_len */ + , 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;