diff --git a/include/libtorrent/alert_manager.hpp b/include/libtorrent/alert_manager.hpp index 80a3f65b7..fe07ba38a 100644 --- a/include/libtorrent/alert_manager.hpp +++ b/include/libtorrent/alert_manager.hpp @@ -74,7 +74,7 @@ namespace libtorrent { template void emplace_alert(Args&&... args) try { - std::unique_lock lock(m_mutex); + std::unique_lock lock(m_mutex); // don't add more than this number of alerts, unless it's a // high priority alert, in which case we try harder to deliver it @@ -90,12 +90,12 @@ namespace libtorrent { T& alert = m_alerts[m_generation].emplace_back( m_allocations[m_generation], std::forward(args)...); - maybe_notify(&alert, lock); + maybe_notify(&alert); } catch (std::bad_alloc const&) { // record that we dropped an alert of this type - std::unique_lock lock(m_mutex); + std::unique_lock lock(m_mutex); m_dropped.set(T::alert_type); } @@ -105,12 +105,7 @@ namespace libtorrent { template bool should_post() const { - if (!(m_alert_mask.load(std::memory_order_relaxed) & T::static_category)) - { - return false; - } - - return should_post_impl(T::priority); + return bool(m_alert_mask.load(std::memory_order_relaxed) & T::static_category); } alert* wait_for_alert(time_duration max_wait); @@ -136,11 +131,15 @@ namespace libtorrent { private: - bool should_post_impl(int priority) const; - void maybe_notify(alert* a, std::unique_lock& lock); + void maybe_notify(alert* a); - mutable std::mutex m_mutex; - std::condition_variable m_condition; + // this mutex protects everything. Since it's held while executing user + // callbacks (the notify function and extension on_alert()) it must be + // recursive to post new alerts. This is implemented by storing the + // current thread-id in m_mutex_holder, if it matches ours, we don't need + // to lock + mutable std::recursive_mutex m_mutex; + std::condition_variable_any m_condition; std::atomic m_alert_mask; int m_queue_size_limit; diff --git a/src/alert_manager.cpp b/src/alert_manager.cpp index 3f34b9486..01cff4f02 100644 --- a/src/alert_manager.cpp +++ b/src/alert_manager.cpp @@ -47,16 +47,9 @@ namespace libtorrent { alert_manager::~alert_manager() = default; - bool alert_manager::should_post_impl(int const priority) const - { - std::lock_guard lock(m_mutex); - return m_alerts[m_generation].size() - < m_queue_size_limit * (1 + priority); - } - alert* alert_manager::wait_for_alert(time_duration max_wait) { - std::unique_lock lock(m_mutex); + std::unique_lock lock(m_mutex); if (!m_alerts[m_generation].empty()) return m_alerts[m_generation].front(); @@ -69,12 +62,10 @@ namespace libtorrent { return nullptr; } - void alert_manager::maybe_notify(alert* a, std::unique_lock& lock) + void alert_manager::maybe_notify(alert* a) { if (m_alerts[m_generation].size() == 1) { - lock.unlock(); - // we just posted to an empty queue. If anyone is waiting for // alerts, we need to notify them. Also (potentially) call the // user supplied m_notify callback to let the client wake up its @@ -85,10 +76,6 @@ namespace libtorrent { // > 0 notify them m_condition.notify_all(); } - else - { - lock.unlock(); - } #ifndef TORRENT_DISABLE_EXTENSIONS for (auto& e : m_ses_extensions) @@ -100,12 +87,10 @@ namespace libtorrent { void alert_manager::set_notify_function(std::function const& fun) { - std::unique_lock lock(m_mutex); + std::unique_lock lock(m_mutex); m_notify = fun; if (!m_alerts[m_generation].empty()) { - // never call a callback with the lock held! - lock.unlock(); if (m_notify) m_notify(); } } @@ -119,9 +104,8 @@ namespace libtorrent { void alert_manager::get_all(std::vector& alerts) { - std::lock_guard lock(m_mutex); + std::lock_guard lock(m_mutex); - alerts.clear(); if (m_alerts[m_generation].empty()) return; m_alerts[m_generation].get_pointers(alerts); @@ -135,13 +119,13 @@ namespace libtorrent { bool alert_manager::pending() const { - std::lock_guard lock(m_mutex); + std::lock_guard lock(m_mutex); return !m_alerts[m_generation].empty(); } int alert_manager::set_alert_queue_size_limit(int queue_size_limit_) { - std::lock_guard lock(m_mutex); + std::lock_guard lock(m_mutex); std::swap(m_queue_size_limit, queue_size_limit_); return queue_size_limit_; @@ -149,7 +133,7 @@ namespace libtorrent { dropped_alerts_t alert_manager::dropped_alerts() { - std::unique_lock lock(m_mutex); + std::unique_lock lock(m_mutex); dropped_alerts_t const ret = m_dropped; m_dropped.reset(); return ret; diff --git a/test/test_alert_manager.cpp b/test/test_alert_manager.cpp index 6cc2ac8a9..94b967997 100644 --- a/test/test_alert_manager.cpp +++ b/test/test_alert_manager.cpp @@ -277,3 +277,33 @@ TORRENT_TEST(dropped_alerts) // it should have been cleared now though TEST_CHECK(mgr.dropped_alerts().none()); } + +#ifndef TORRENT_DISABLE_EXTENSIONS +struct post_plugin : lt::plugin +{ + explicit post_plugin(alert_manager& m) : mgr(m) {} + void on_alert(alert const*) + { + if (++depth > 10) return; + mgr.emplace_alert(torrent_handle(), piece_index_t{0}); + } + + alert_manager& mgr; + int depth = 0; +}; + +// make sure the alert manager supports alerts being posted while executing a +// plugin handler +TORRENT_TEST(recursive_alerts) +{ + alert_manager mgr(100, alert::all_categories); + auto pl = std::make_shared(mgr); + mgr.add_extension(pl); + + mgr.emplace_alert(torrent_handle(), piece_index_t{0}); + + TEST_EQUAL(pl->depth, 11); +} + +#endif // TORRENT_DISABLE_EXTENSIONS +