diff --git a/libtransmission/peer-mgr-wishlist.cc b/libtransmission/peer-mgr-wishlist.cc index ffadda443..325389ff7 100644 --- a/libtransmission/peer-mgr-wishlist.cc +++ b/libtransmission/peer-mgr-wishlist.cc @@ -334,7 +334,9 @@ private: for (tr_piece_index_t piece = 0U, idx_c = 0U; piece < n_pieces; ++piece) { auto const existing_candidate = idx_c < n_old_c && piece == candidates_[idx_c].piece; - if (mediator_.client_wants_piece(piece)) + auto const client_wants_piece = mediator_.client_wants_piece(piece); + auto const client_has_piece = mediator_.client_has_piece(piece); + if (client_wants_piece && !client_has_piece) { if (existing_candidate) { diff --git a/tests/libtransmission/peer-mgr-wishlist-test.cc b/tests/libtransmission/peer-mgr-wishlist-test.cc index e15a56f0e..0e48359e2 100644 --- a/tests/libtransmission/peer-mgr-wishlist-test.cc +++ b/tests/libtransmission/peer-mgr-wishlist-test.cc @@ -1627,6 +1627,66 @@ TEST_F(PeerMgrWishlistTest, setFileWantedUpdatesCandidateListAdd) // So test several times to shake out any randomness static auto constexpr NumRuns = 1000; for (int run = 0; run < NumRuns; ++run) + { + auto requested = tr_bitfield{ 400 }; + auto const spans = get_spans(400); + for (auto const& [begin, end] : spans) + { + requested.set_span(begin, end); + } + EXPECT_EQ(400U, requested.count()); + EXPECT_NE(0U, requested.count(0, 100)); + EXPECT_NE(0U, requested.count(100, 200)); + EXPECT_NE(0U, requested.count(200, 300)); + EXPECT_NE(0U, requested.count(300, 400)); + } +} + +TEST_F(PeerMgrWishlistTest, setFileWantedUpdatesCandidateListAddHad) +{ + auto const get_spans = [this](size_t n_wanted) + { + auto mediator = MockMediator{ *this }; + + // setup: four pieces + mediator.piece_count_ = 4; + mediator.block_span_[0] = { 0, 100 }; + mediator.block_span_[1] = { 100, 200 }; + mediator.block_span_[2] = { 200, 300 }; + mediator.block_span_[3] = { 300, 400 }; + + // we have pieces 2, 3 + mediator.client_has_piece_.insert(2); + mediator.client_has_piece_.insert(3); + + // peer has all pieces + mediator.piece_replication_[0] = 1; + mediator.piece_replication_[1] = 1; + mediator.piece_replication_[2] = 1; + mediator.piece_replication_[3] = 1; + + // we initially wanted the first 2 pieces only + mediator.client_wants_piece_.insert(0); + mediator.client_wants_piece_.insert(1); + + // allow the wishlist to build its cache + auto wishlist = Wishlist{ mediator }; + + // now we want piece 2 and piece 3 + mediator.client_wants_piece_.insert(2); + mediator.client_wants_piece_.insert(3); + files_wanted_changed_.emit(nullptr, nullptr, 0, true); + + // the candidate list should remain unchanged + return wishlist.next(n_wanted, PeerHasAllPieces); + }; + + // We should only request pieces 0, 1 here. + // NB: when all other things are equal in the wishlist, pieces are + // picked at random so this test -could- pass even if there's a bug. + // So test several times to shake out any randomness + static auto constexpr NumRuns = 1000; + for (int run = 0; run < NumRuns; ++run) { auto requested = tr_bitfield{ 400 }; auto const spans = get_spans(350); @@ -1634,11 +1694,11 @@ TEST_F(PeerMgrWishlistTest, setFileWantedUpdatesCandidateListAdd) { requested.set_span(begin, end); } - EXPECT_EQ(350U, requested.count()); + EXPECT_EQ(200U, requested.count()); EXPECT_NE(0U, requested.count(0, 100)); EXPECT_NE(0U, requested.count(100, 200)); - EXPECT_NE(0U, requested.count(200, 300)); - EXPECT_NE(0U, requested.count(300, 400)); + EXPECT_EQ(0U, requested.count(200, 300)); + EXPECT_EQ(0U, requested.count(300, 400)); } } @@ -1687,7 +1747,7 @@ TEST_F(PeerMgrWishlistTest, setFileWantedUpdatesCandidateListRemove) for (int run = 0; run < NumRuns; ++run) { auto requested = tr_bitfield{ 400 }; - auto const spans = get_spans(350); + auto const spans = get_spans(400); for (auto const& [begin, end] : spans) { requested.set_span(begin, end);