From c9eabdb94e3f2eeb9b01efc471fa21404ed06e8d Mon Sep 17 00:00:00 2001 From: Szabolcs Fruvald Date: Thu, 16 Jul 2026 19:46:05 +0200 Subject: [PATCH] Fix tracker announce stalls with active scrapes --- src/net/curl_socket.cc | 2 ++ src/net/curl_stack.cc | 31 +++++++++++++----- src/net/curl_stack.h | 2 ++ src/tracker/tracker_controller.cc | 2 +- test/torrent/test_tracker_controller.cc | 43 +++++++++++++++++++++++++ test/torrent/test_tracker_controller.h | 7 ++++ 6 files changed, 78 insertions(+), 9 deletions(-) diff --git a/src/net/curl_socket.cc b/src/net/curl_socket.cc index 871439283c..d290e0ac60 100644 --- a/src/net/curl_socket.cc +++ b/src/net/curl_socket.cc @@ -349,6 +349,8 @@ CurlSocket::handle_action(int ev_bitmask) { // while (stack->process_done_handle()) ; // Do nothing. + + stack->ensure_timeout_scheduled(); } void diff --git a/src/net/curl_stack.cc b/src/net/curl_stack.cc index cc9fec14c6..3643e9d6ae 100644 --- a/src/net/curl_stack.cc +++ b/src/net/curl_stack.cc @@ -161,6 +161,8 @@ CurlStack::start_get(const std::shared_ptr& curl_get) { curl_get->activate_unsafe(); } + + ensure_timeout_scheduled(); } void @@ -233,16 +235,29 @@ CurlStack::receive_timeout() { return; } - if (!m_task_timeout.is_scheduled()) { - // Sometimes libcurl forgets to reset the timeout. Try to poll the value in that case, or use 10 - // seconds max. - long timeout_ms; - curl_multi_timeout(m_handle, &timeout_ms); + ensure_timeout_scheduled(); +} - auto timeout = std::max(std::chrono::milliseconds(timeout_ms), 10s); +// The multi-socket API only enforces CURLOPT_TIMEOUT when libcurl is driven by socket activity or +// the timer task. If the timer is not scheduled while transfers are active (e.g. libcurl cleared +// it, or a connection through a dead proxy never produces socket events), a hung transfer would +// never time out. Guarantee the stack is polled at least every 10 seconds while non-empty. +void +CurlStack::ensure_timeout_scheduled() { + assert(std::this_thread::get_id() == m_thread->thread_id()); - torrent::this_thread::scheduler()->wait_for_ceil_seconds(&m_task_timeout, timeout); - } + if (base_type::empty() || m_task_timeout.is_scheduled()) + return; + + long timeout_ms{}; + curl_multi_timeout(m_handle, &timeout_ms); + + std::chrono::microseconds timeout = 10s; + + if (timeout_ms >= 0) + timeout = std::min(std::chrono::milliseconds(timeout_ms), timeout); + + torrent::this_thread::scheduler()->wait_for_ceil_seconds(&m_task_timeout, timeout); } // TODO: Check if curl_get is still active. diff --git a/src/net/curl_stack.h b/src/net/curl_stack.h index a468e653d3..198e64024a 100644 --- a/src/net/curl_stack.h +++ b/src/net/curl_stack.h @@ -63,6 +63,8 @@ class align_cacheline CurlStack : private std::vector> bool process_done_handle(); + void ensure_timeout_scheduled(); + system::Thread* thread() const { return m_thread; } protected: diff --git a/src/tracker/tracker_controller.cc b/src/tracker/tracker_controller.cc index 657a6fa6e1..5e9654c463 100644 --- a/src/tracker/tracker_controller.cc +++ b/src/tracker/tracker_controller.cc @@ -548,7 +548,7 @@ TrackerController::receive_success(const tracker::Tracker& tracker, TrackerContr if ((m_flags & flag_requesting)) update_timeout(30); - else if (!m_tracker_list->has_active()) { + else if (!m_tracker_list->has_active_not_scrape()) { std::chrono::seconds normal_interval; tracker.lock_and_call_state([&](const tracker::TrackerState& state) { diff --git a/test/torrent/test_tracker_controller.cc b/test/torrent/test_tracker_controller.cc index 32bdda33b3..623acd5503 100644 --- a/test/torrent/test_tracker_controller.cc +++ b/test/torrent/test_tracker_controller.cc @@ -708,6 +708,49 @@ TestTrackerController::test_new_peers() { CPPUNIT_ASSERT(tracker_0_0.state().latest_new_peers() == 20); } +// Regression test for the stale-announce bug: +// If an announce succeeds while a scrape is still active on another tracker, the +// controller must still schedule the next announce timeout. Before the fix it +// used has_active() which counted scrapes as active, so it skipped scheduling +// and the next announce was never started once the scrape finished. +void +TestTrackerController::test_success_with_active_scrape_reschedules_timeout() { + TEST_MULTI3_BEGIN(); + + auto tracker_0_0_worker = TrackerTest::test_worker(tracker_0_0); + auto tracker_1_0_worker = TrackerTest::test_worker(tracker_1_0); + + // Send an announce on tracker_0_0 and a scrape on tracker_1_0. Use the + // worker directly for the scrape so the test does not depend on cached time + // or scrape intervals. + tracker_controller.send_update_event(); + + tracker_1_0_worker->set_scrapable(); + tracker_1_0_worker->send_scrape(torrent::tracker::TrackerParams{}); + + std::this_thread::sleep_for(100ms); + + CPPUNIT_ASSERT(tracker_0_0.is_requesting()); + CPPUNIT_ASSERT(tracker_1_0.is_requesting()); + CPPUNIT_ASSERT(!tracker_1_0.is_requesting_not_scrape()); + + // Announce succeeds while the scrape is still in progress. + CPPUNIT_ASSERT(tracker_0_0_worker->trigger_success()); + + // The next announce timeout must be queued despite the active scrape. + // trigger_success() resets the tracker's normal interval to the default, + // so just verify the timeout is set and non-zero. + CPPUNIT_ASSERT(tracker_controller.is_timeout_queued()); + CPPUNIT_ASSERT(tracker_controller.seconds_to_next_timeout() > 0); + CPPUNIT_ASSERT(tracker_controller.seconds_to_next_timeout() <= + tracker_0_0.state().normal_interval().count() + 1); + + // Let the scrape finish. + CPPUNIT_ASSERT(tracker_1_0_worker->trigger_scrape()); + + TEST_MULTIPLE_END(1, 0); +} + // Add new function for finding the first tracker that will time out, // e.g. both with failure mode and normal rerequesting. diff --git a/test/torrent/test_tracker_controller.h b/test/torrent/test_tracker_controller.h index 1752997718..aff3570452 100644 --- a/test/torrent/test_tracker_controller.h +++ b/test/torrent/test_tracker_controller.h @@ -31,6 +31,7 @@ class TestTrackerController : public TestFixtureWithMainAndTrackerThread { CPPUNIT_TEST(test_timeout_lacking_usable); CPPUNIT_TEST(test_disable_tracker); CPPUNIT_TEST(test_new_peers); + CPPUNIT_TEST(test_success_with_active_scrape_reschedules_timeout); CPPUNIT_TEST_SUITE_END(); @@ -63,6 +64,12 @@ class TestTrackerController : public TestFixtureWithMainAndTrackerThread { void test_timeout_lacking_usable(); void test_disable_tracker(); void test_new_peers(); + + // Regression: an announce success while another tracker is scraping must still + // schedule the next announce. Before the fix, the controller waited for *all* + // activity (including scrapes) to finish, then receive_scrape() never + // rescheduled the announce timeout. + void test_success_with_active_scrape_reschedules_timeout(); }; #define TRACKER_CONTROLLER_SETUP() \