Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/net/curl_socket.cc
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,8 @@ CurlSocket::handle_action(int ev_bitmask) {
//
while (stack->process_done_handle())
; // Do nothing.

stack->ensure_timeout_scheduled();
}

void
Expand Down
31 changes: 23 additions & 8 deletions src/net/curl_stack.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ CurlStack::start_get(const std::shared_ptr<CurlGet>& curl_get) {

curl_get->activate_unsafe();
}

ensure_timeout_scheduled();
}

void
Expand Down Expand Up @@ -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::microseconds>(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::microseconds>(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.
Expand Down
2 changes: 2 additions & 0 deletions src/net/curl_stack.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ class align_cacheline CurlStack : private std::vector<std::shared_ptr<CurlGet>>

bool process_done_handle();

void ensure_timeout_scheduled();

system::Thread* thread() const { return m_thread; }

protected:
Expand Down
2 changes: 1 addition & 1 deletion src/tracker/tracker_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
43 changes: 43 additions & 0 deletions test/torrent/test_tracker_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
7 changes: 7 additions & 0 deletions test/torrent/test_tracker_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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() \
Expand Down