From 3a4e115c528b8be568da039cd5717d6e09c19063 Mon Sep 17 00:00:00 2001 From: Yat Ho Date: Sat, 3 Jan 2026 03:04:53 +0800 Subject: [PATCH] fix: wishlist edge case when got bad piece in unaligned torrents (#8047) * test: add failing test case * fix: got bad piece in unaligned torrents --- libtransmission/peer-mgr-wishlist.cc | 10 +- .../libtransmission/peer-mgr-wishlist-test.cc | 264 ++++++++++++------ 2 files changed, 191 insertions(+), 83 deletions(-) diff --git a/libtransmission/peer-mgr-wishlist.cc b/libtransmission/peer-mgr-wishlist.cc index c7765cd08..3051022d5 100644 --- a/libtransmission/peer-mgr-wishlist.cc +++ b/libtransmission/peer-mgr-wishlist.cc @@ -270,30 +270,29 @@ private: } TR_ASSERT(std::empty(iter->unrequested)); + iter->block_span = iter->raw_block_span; if (piece > 0U) { if (auto const prev = find_by_piece(piece - 1U); prev != std::end(candidates_)) { - iter->block_span.begin = std::max(iter->raw_block_span.begin, prev->block_span.end); + iter->block_span.begin = std::max(iter->block_span.begin, prev->block_span.end); TR_ASSERT(iter->block_span.begin == prev->block_span.end); for (tr_block_index_t i = iter->block_span.begin; i > iter->raw_block_span.begin; --i) { prev->unrequested.insert(i - 1U); } - resort_piece(prev); } } if (piece < mediator_.piece_count() - 1U) { if (auto const next = find_by_piece(piece + 1U); next != std::end(candidates_)) { - iter->block_span.end = std::min(iter->raw_block_span.end, next->block_span.begin); + iter->block_span.end = std::min(iter->block_span.end, next->block_span.begin); TR_ASSERT(iter->block_span.end == next->block_span.begin); for (tr_block_index_t i = iter->raw_block_span.end; i > iter->block_span.end; --i) { next->unrequested.insert(i - 1U); } - resort_piece(next); } } @@ -301,7 +300,8 @@ private: { iter->unrequested.insert(i - 1U); } - resort_piece(iter); + + std::sort(std::begin(candidates_), std::end(candidates_)); } // --- diff --git a/tests/libtransmission/peer-mgr-wishlist-test.cc b/tests/libtransmission/peer-mgr-wishlist-test.cc index d295676c3..ce616f3c7 100644 --- a/tests/libtransmission/peer-mgr-wishlist-test.cc +++ b/tests/libtransmission/peer-mgr-wishlist-test.cc @@ -30,7 +30,6 @@ protected: mutable std::set client_has_block_; mutable std::set client_has_piece_; mutable std::set client_wants_piece_; - tr_piece_index_t piece_count_ = 0; bool is_sequential_download_ = false; tr_piece_index_t sequential_download_from_piece_ = 0; @@ -78,7 +77,7 @@ protected: [[nodiscard]] tr_piece_index_t piece_count() const override { - return piece_count_; + return std::size(block_span_); } [[nodiscard]] tr_priority_t priority(tr_piece_index_t piece) const override @@ -206,7 +205,6 @@ TEST_F(PeerMgrWishlistTest, doesNotRequestPiecesThatAreNotWanted) auto mediator = MockMediator{ *this }; // setup: three pieces, all missing - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 250 }; @@ -231,7 +229,6 @@ TEST_F(PeerMgrWishlistTest, doesNotRequestPiecesThatClientHas) auto mediator = MockMediator{ *this }; // setup: three pieces - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 250 }; @@ -262,7 +259,6 @@ TEST_F(PeerMgrWishlistTest, onlyRequestBlocksThePeerHas) auto mediator = MockMediator{ *this }; // setup: three pieces, all missing - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 250 }; @@ -303,7 +299,6 @@ TEST_F(PeerMgrWishlistTest, doesNotRequestSameBlockTwice) auto mediator = MockMediator{ *this }; // setup: three pieces, all missing - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 250 }; @@ -345,7 +340,6 @@ TEST_F(PeerMgrWishlistTest, sequentialDownload) auto mediator = MockMediator{ *this }; // setup: three pieces, all missing - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 250 }; @@ -410,7 +404,6 @@ TEST_F(PeerMgrWishlistTest, sequentialDownloadFromPiece) auto mediator = MockMediator{ *this }; // setup: four pieces, all missing - mediator.piece_count_ = 4; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; @@ -423,7 +416,7 @@ TEST_F(PeerMgrWishlistTest, sequentialDownloadFromPiece) mediator.piece_replication_[3] = 1; // and we want all pieces - for (tr_piece_index_t i = 0; i < 4; ++i) + for (tr_piece_index_t i = 0; i < mediator.piece_count(); ++i) { mediator.client_wants_piece_.insert(i); } @@ -464,7 +457,6 @@ TEST_F(PeerMgrWishlistTest, doesNotRequestTooManyBlocks) auto mediator = MockMediator{ *this }; // setup: three pieces, all missing - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 250 }; @@ -475,7 +467,7 @@ TEST_F(PeerMgrWishlistTest, doesNotRequestTooManyBlocks) mediator.piece_replication_[2] = 1; // and we want everything - for (tr_piece_index_t i = 0; i < 3; ++i) + for (tr_piece_index_t i = 0; i < mediator.piece_count(); ++i) { mediator.client_wants_piece_.insert(i); } @@ -499,7 +491,6 @@ TEST_F(PeerMgrWishlistTest, prefersHighPriorityPieces) auto mediator = MockMediator{ *this }; // setup: three pieces, all missing - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; @@ -510,7 +501,7 @@ TEST_F(PeerMgrWishlistTest, prefersHighPriorityPieces) mediator.piece_replication_[2] = 1; // and we want everything - for (tr_piece_index_t i = 0; i < 3; ++i) + for (tr_piece_index_t i = 0; i < mediator.piece_count(); ++i) { mediator.client_wants_piece_.insert(i); } @@ -549,7 +540,6 @@ TEST_F(PeerMgrWishlistTest, prefersNearlyCompletePieces) auto mediator = MockMediator{ *this }; // setup: three pieces, same size - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; @@ -560,7 +550,7 @@ TEST_F(PeerMgrWishlistTest, prefersNearlyCompletePieces) mediator.piece_replication_[2] = 1; // and we want everything - for (tr_piece_index_t i = 0; i < 3; ++i) + for (tr_piece_index_t i = 0; i < mediator.piece_count(); ++i) { mediator.client_wants_piece_.insert(i); } @@ -626,13 +616,12 @@ TEST_F(PeerMgrWishlistTest, prefersRarerPieces) auto mediator = MockMediator{ *this }; // setup: three pieces, all missing - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; // and we want everything - for (tr_piece_index_t i = 0; i < 3; ++i) + for (tr_piece_index_t i = 0; i < mediator.piece_count(); ++i) { mediator.client_wants_piece_.insert(i); } @@ -689,13 +678,12 @@ TEST_F(PeerMgrWishlistTest, peerDisconnectDecrementsReplication) auto mediator = MockMediator{ *this }; // setup: three pieces, all missing - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; // and we want everything - for (tr_piece_index_t i = 0; i < 3; ++i) + for (tr_piece_index_t i = 0; i < mediator.piece_count(); ++i) { mediator.client_wants_piece_.insert(i); } @@ -763,13 +751,12 @@ TEST_F(PeerMgrWishlistTest, gotBadPieceResetsPiece) auto mediator = MockMediator{ *this }; // setup: three pieces, all missing - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; // and we want everything - for (tr_piece_index_t i = 0; i < 3; ++i) + for (tr_piece_index_t i = 0; i < mediator.piece_count(); ++i) { mediator.client_wants_piece_.insert(i); } @@ -823,13 +810,12 @@ TEST_F(PeerMgrWishlistTest, gotBitfieldIncrementsReplication) auto mediator = MockMediator{ *this }; // setup: three pieces, all missing - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; // and we want everything - for (tr_piece_index_t i = 0; i < 3; ++i) + for (tr_piece_index_t i = 0; i < mediator.piece_count(); ++i) { mediator.client_wants_piece_.insert(i); } @@ -898,7 +884,6 @@ TEST_F(PeerMgrWishlistTest, sentRequestsResortsPiece) auto mediator = MockMediator{ *this }; // setup: three pieces, all missing - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; @@ -909,7 +894,7 @@ TEST_F(PeerMgrWishlistTest, sentRequestsResortsPiece) mediator.piece_replication_[2] = 1; // and we want everything - for (tr_piece_index_t i = 0; i < 3; ++i) + for (tr_piece_index_t i = 0; i < mediator.piece_count(); ++i) { mediator.client_wants_piece_.insert(i); } @@ -967,7 +952,6 @@ TEST_F(PeerMgrWishlistTest, gotBlockResortsPiece) auto mediator = MockMediator{ *this }; // setup: three pieces, all missing - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; @@ -978,7 +962,7 @@ TEST_F(PeerMgrWishlistTest, gotBlockResortsPiece) mediator.piece_replication_[2] = 1; // and we want everything - for (tr_piece_index_t i = 0; i < 3; ++i) + for (tr_piece_index_t i = 0; i < mediator.piece_count(); ++i) { mediator.client_wants_piece_.insert(i); } @@ -1036,13 +1020,12 @@ TEST_F(PeerMgrWishlistTest, gotHaveIncrementsReplication) auto mediator = MockMediator{ *this }; // setup: three pieces, all missing - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; // and we want everything - for (tr_piece_index_t i = 0; i < 3; ++i) + for (tr_piece_index_t i = 0; i < mediator.piece_count(); ++i) { mediator.client_wants_piece_.insert(i); } @@ -1108,7 +1091,6 @@ TEST_F(PeerMgrWishlistTest, gotChokeResetsRequestedBlocks) auto mediator = MockMediator{ *this }; // setup: three pieces, all missing - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; @@ -1119,7 +1101,7 @@ TEST_F(PeerMgrWishlistTest, gotChokeResetsRequestedBlocks) mediator.piece_replication_[2] = 2; // and we want everything - for (tr_piece_index_t i = 0; i < 3; ++i) + for (tr_piece_index_t i = 0; i < mediator.piece_count(); ++i) { mediator.client_wants_piece_.insert(i); } @@ -1166,13 +1148,12 @@ TEST_F(PeerMgrWishlistTest, gotHaveAllDoesNotAffectOrder) auto mediator = MockMediator{ *this }; // setup: three pieces, all missing - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; // and we want everything - for (tr_piece_index_t i = 0; i < 3; ++i) + for (tr_piece_index_t i = 0; i < mediator.piece_count(); ++i) { mediator.client_wants_piece_.insert(i); } @@ -1238,7 +1219,6 @@ TEST_F(PeerMgrWishlistTest, gotRejectResetsBlock) auto mediator = MockMediator{ *this }; // setup: three pieces, all missing - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; @@ -1249,7 +1229,7 @@ TEST_F(PeerMgrWishlistTest, gotRejectResetsBlock) mediator.piece_replication_[2] = 2; // and we want everything - for (tr_piece_index_t i = 0; i < 3; ++i) + for (tr_piece_index_t i = 0; i < mediator.piece_count(); ++i) { mediator.client_wants_piece_.insert(i); } @@ -1302,7 +1282,6 @@ TEST_F(PeerMgrWishlistTest, gotRejectResortsPiece) auto mediator = MockMediator{ *this }; // setup: two pieces, all missing - mediator.piece_count_ = 2; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; @@ -1353,7 +1332,6 @@ TEST_F(PeerMgrWishlistTest, sentCancelResetsBlocks) auto mediator = MockMediator{ *this }; // setup: three pieces, all missing - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; @@ -1364,7 +1342,7 @@ TEST_F(PeerMgrWishlistTest, sentCancelResetsBlocks) mediator.piece_replication_[2] = 2; // and we want everything - for (tr_piece_index_t i = 0; i < 3; ++i) + for (tr_piece_index_t i = 0; i < mediator.piece_count(); ++i) { mediator.client_wants_piece_.insert(i); } @@ -1417,7 +1395,6 @@ TEST_F(PeerMgrWishlistTest, doesNotRequestBlockAfterBlockCompleted) auto mediator = MockMediator{ *this }; // setup: three pieces, all missing - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; @@ -1428,7 +1405,7 @@ TEST_F(PeerMgrWishlistTest, doesNotRequestBlockAfterBlockCompleted) mediator.piece_replication_[2] = 2; // and we want everything - for (tr_piece_index_t i = 0; i < 3; ++i) + for (tr_piece_index_t i = 0; i < mediator.piece_count(); ++i) { mediator.client_wants_piece_.insert(i); } @@ -1467,7 +1444,6 @@ TEST_F(PeerMgrWishlistTest, doesNotRequestPieceAfterPieceCompleted) auto mediator = MockMediator{ *this }; // setup: three pieces, piece 0 is nearly complete - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; @@ -1478,7 +1454,7 @@ TEST_F(PeerMgrWishlistTest, doesNotRequestPieceAfterPieceCompleted) mediator.piece_replication_[2] = 1; // and we want everything - for (tr_piece_index_t i = 0; i < 3; ++i) + for (tr_piece_index_t i = 0; i < mediator.piece_count(); ++i) { mediator.client_wants_piece_.insert(i); } @@ -1488,6 +1464,12 @@ TEST_F(PeerMgrWishlistTest, doesNotRequestPieceAfterPieceCompleted) auto wishlist = Wishlist{ mediator }; // we just completed piece 0 + sent_request_.emit(nullptr, nullptr, mediator.block_span_[0]); + for (auto [block, end] = mediator.block_span_[0]; block < end; ++block) + { + mediator.client_has_block_.insert(block); + got_block_.emit(nullptr, block); + } mediator.client_has_piece_.insert(0); piece_completed_.emit(nullptr, 0); @@ -1511,7 +1493,6 @@ TEST_F(PeerMgrWishlistTest, settingPriorityResortsCandidates) auto mediator = MockMediator{ *this }; // setup: three pieces, all missing - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; @@ -1522,7 +1503,7 @@ TEST_F(PeerMgrWishlistTest, settingPriorityResortsCandidates) mediator.piece_replication_[2] = 1; // and we want everything - for (tr_piece_index_t i = 0; i < 3; ++i) + for (tr_piece_index_t i = 0; i < mediator.piece_count(); ++i) { mediator.client_wants_piece_.insert(i); } @@ -1566,7 +1547,6 @@ TEST_F(PeerMgrWishlistTest, settingSequentialDownloadResortsCandidates) auto mediator = MockMediator{ *this }; // setup: three pieces, all missing - mediator.piece_count_ = 3; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; @@ -1577,7 +1557,7 @@ TEST_F(PeerMgrWishlistTest, settingSequentialDownloadResortsCandidates) mediator.piece_replication_[2] = 1; // and we want everything - for (tr_piece_index_t i = 0; i < 3; ++i) + for (tr_piece_index_t i = 0; i < mediator.piece_count(); ++i) { mediator.client_wants_piece_.insert(i); } @@ -1636,7 +1616,6 @@ TEST_F(PeerMgrWishlistTest, sequentialDownloadFromPieceResortsCandidates) auto mediator = MockMediator{ *this }; // setup: four pieces, all missing - mediator.piece_count_ = 4; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; @@ -1649,7 +1628,7 @@ TEST_F(PeerMgrWishlistTest, sequentialDownloadFromPieceResortsCandidates) mediator.piece_replication_[3] = 1; // and we want all pieces - for (tr_piece_index_t i = 0; i < 4; ++i) + for (tr_piece_index_t i = 0; i < mediator.piece_count(); ++i) { mediator.client_wants_piece_.insert(i); } @@ -1699,7 +1678,6 @@ TEST_F(PeerMgrWishlistTest, setFileWantedUpdatesCandidateListAdd) auto mediator = MockMediator{ *this }; // setup: four pieces, all missing - mediator.piece_count_ = 4; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; @@ -1756,7 +1734,6 @@ TEST_F(PeerMgrWishlistTest, setFileWantedUpdatesCandidateListAddHad) 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 }; @@ -1816,7 +1793,6 @@ TEST_F(PeerMgrWishlistTest, setFileWantedUpdatesCandidateListRemove) auto mediator = MockMediator{ *this }; // setup: four pieces, all missing - mediator.piece_count_ = 4; mediator.block_span_[0] = { 0, 100 }; mediator.block_span_[1] = { 100, 200 }; mediator.block_span_[2] = { 200, 300 }; @@ -1875,7 +1851,6 @@ TEST_F(PeerMgrWishlistTest, unalignedTorrent) // setup: 4 pieces, (100 / 3 * 16) KiB each, all missing // N.B. only the boundary of piece 2 and 3 is aligned - mediator.piece_count_ = 4; mediator.block_span_[0] = { 0, 34 }; mediator.block_span_[1] = { 33, 67 }; mediator.block_span_[2] = { 66, 100 }; @@ -1923,7 +1898,7 @@ TEST_F(PeerMgrWishlistTest, unalignedTorrent) } } -TEST_F(PeerMgrWishlistTest, unalignedTorrentPartiallyComplete) +TEST_F(PeerMgrWishlistTest, unalignedTorrentPartiallyCompletedPiece) { auto const get_spans = [this](size_t n_wanted) { @@ -1931,7 +1906,6 @@ TEST_F(PeerMgrWishlistTest, unalignedTorrentPartiallyComplete) // setup: 4 pieces, (100 / 3 * 16) KiB each // N.B. only the boundary of piece 2 and 3 is aligned - mediator.piece_count_ = 4; mediator.block_span_[0] = { 0, 34 }; mediator.block_span_[1] = { 33, 67 }; mediator.block_span_[2] = { 66, 100 }; @@ -1995,7 +1969,7 @@ TEST_F(PeerMgrWishlistTest, unalignedTorrentPartiallyComplete) } } -TEST_F(PeerMgrWishlistTest, unalignedTorrentGotBadPiece) +TEST_F(PeerMgrWishlistTest, unalignedTorrentPartiallyCompleted) { auto const get_spans = [this](size_t n_wanted) { @@ -2003,21 +1977,15 @@ TEST_F(PeerMgrWishlistTest, unalignedTorrentGotBadPiece) // setup: 4 pieces, (100 / 3 * 16) KiB each // N.B. only the boundary of piece 2 and 3 is aligned - mediator.piece_count_ = 4; mediator.block_span_[0] = { 0, 34 }; mediator.block_span_[1] = { 33, 67 }; mediator.block_span_[2] = { 66, 100 }; mediator.block_span_[3] = { 100, 134 }; - // We have pieces 0, 2, 3, and sent out requests for - // every block in piece 1 + // We have pieces 0, 2, 3 mediator.client_has_piece_.insert(0); mediator.client_has_piece_.insert(2); mediator.client_has_piece_.insert(3); - for (tr_block_index_t block = 0; block < 134; ++block) - { - mediator.client_has_block_.insert(block); - } // peer has all pieces mediator.piece_replication_[0] = 1; @@ -2034,7 +2002,142 @@ TEST_F(PeerMgrWishlistTest, unalignedTorrentGotBadPiece) // allow the wishlist to build its cache auto wishlist = Wishlist{ mediator }; + return wishlist.next(n_wanted, PeerHasAllPieces); + }; + + // 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{ 134 }; + auto const spans = get_spans(134); + + // We should get [33, 67), not any shrunken spans like [34, 67) + EXPECT_EQ(std::size(spans), 1); + for (auto const& [begin, end] : spans) + { + requested.set_span(begin, end); + } + EXPECT_EQ(34U, requested.count()); + EXPECT_EQ(34U, requested.count(33, 67)); + } +} + +TEST_F(PeerMgrWishlistTest, unalignedTorrentGotBadPiece) +{ + auto const get_spans = [this](size_t n_wanted) + { + auto mediator = MockMediator{ *this }; + + // setup: 4 pieces, (100 / 3 * 16) KiB each + // N.B. only the boundary of piece 2 and 3 is aligned + mediator.block_span_[0] = { 0, 34 }; + mediator.block_span_[1] = { 33, 67 }; + mediator.block_span_[2] = { 66, 100 }; + mediator.block_span_[3] = { 100, 134 }; + + // 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 want all 4 pieces + mediator.client_wants_piece_.insert(0); + mediator.client_wants_piece_.insert(1); + mediator.client_wants_piece_.insert(2); + mediator.client_wants_piece_.insert(3); + + // allow the wishlist to build its cache + auto wishlist = Wishlist{ mediator }; + + // requested all blocks and "download" piece 1, + // as well as parts of piece 0 and piece 2 that + // is next to piece 1 + sent_request_.emit(nullptr, nullptr, { 0, 134 }); + for (auto block = mediator.block_span_[0].end - 10; block < mediator.block_span_[1].end + 10; ++block) + { + mediator.client_has_block_.insert(block); + got_block_.emit(nullptr, block); + } + // piece 1 turned out to be corrupt, needs to be re-downloaded + for (auto [block, end] = mediator.block_span_[1]; block < end; ++block) + { + mediator.client_has_block_.erase(block); + } + got_bad_piece_.emit(nullptr, 1); + + return wishlist.next(n_wanted, PeerHasAllPieces); + }; + + // 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{ 134 }; + auto const spans = get_spans(134); + + // We should get [33, 67), not any shrunken spans like [34, 67) + EXPECT_EQ(std::size(spans), 1); + for (auto const& [begin, end] : spans) + { + requested.set_span(begin, end); + } + EXPECT_EQ(34U, requested.count()); + EXPECT_EQ(34U, requested.count(33, 67)); + } +} + +TEST_F(PeerMgrWishlistTest, unalignedTorrentGotBadPieceSurroundingCompleted) +{ + auto const get_spans = [this](size_t n_wanted) + { + auto mediator = MockMediator{ *this }; + + // setup: 4 pieces, (100 / 3 * 16) KiB each + // N.B. only the boundary of piece 2 and 3 is aligned + mediator.block_span_[0] = { 0, 34 }; + mediator.block_span_[1] = { 33, 67 }; + mediator.block_span_[2] = { 66, 100 }; + mediator.block_span_[3] = { 100, 134 }; + + // 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 want all 4 pieces + mediator.client_wants_piece_.insert(0); + mediator.client_wants_piece_.insert(1); + mediator.client_wants_piece_.insert(2); + mediator.client_wants_piece_.insert(3); + + // allow the wishlist to build its cache + auto wishlist = Wishlist{ mediator }; + + // pieces 0, 2 completed normally, piece 3 has pending requests + sent_request_.emit(nullptr, nullptr, { 0, 134 }); + for (tr_block_index_t block = 0; block < 120; ++block) + { + mediator.client_has_block_.insert(block); + got_block_.emit(nullptr, block); + } + mediator.client_has_piece_.insert(0); + piece_completed_.emit(nullptr, 0); + mediator.client_has_piece_.insert(2); + piece_completed_.emit(nullptr, 2); + + // piece 1 turned out to be corrupt, needs to be re-downloaded + for (auto [block, end] = mediator.block_span_[1]; block < end; ++block) + { + mediator.client_has_block_.erase(block); + } got_bad_piece_.emit(nullptr, 1); return wishlist.next(n_wanted, PeerHasAllPieces); @@ -2068,21 +2171,11 @@ TEST_F(PeerMgrWishlistTest, unalignedTorrentGot2ConsectutiveBadPieces) // setup: 4 pieces, (100 / 3 * 16) KiB each // N.B. only the boundary of piece 2 and 3 is aligned - mediator.piece_count_ = 4; mediator.block_span_[0] = { 0, 34 }; mediator.block_span_[1] = { 33, 67 }; mediator.block_span_[2] = { 66, 100 }; mediator.block_span_[3] = { 100, 134 }; - // We have pieces 0, 3, and sent out requests for - // every block in pieces 1, 2 - mediator.client_has_piece_.insert(0); - mediator.client_has_piece_.insert(3); - for (tr_block_index_t block = 0; block < 134; ++block) - { - mediator.client_has_block_.insert(block); - } - // peer has all pieces mediator.piece_replication_[0] = 1; mediator.piece_replication_[1] = 1; @@ -2098,8 +2191,28 @@ TEST_F(PeerMgrWishlistTest, unalignedTorrentGot2ConsectutiveBadPieces) // allow the wishlist to build its cache auto wishlist = Wishlist{ mediator }; - // piece 1 turned out to be corrupt, needs to be re-downloaded + // pieces 0, 3 completed normally + sent_request_.emit(nullptr, nullptr, { 0, 134 }); + for (tr_block_index_t block = 0; block < 134; ++block) + { + mediator.client_has_block_.insert(block); + got_block_.emit(nullptr, block); + } + mediator.client_has_piece_.insert(0); + piece_completed_.emit(nullptr, 0); + mediator.client_has_piece_.insert(3); + piece_completed_.emit(nullptr, 3); + + // pieces 1, 2 turned out to be corrupt, need to be re-downloaded + for (auto [block, end] = mediator.block_span_[1]; block < end; ++block) + { + mediator.client_has_block_.erase(block); + } got_bad_piece_.emit(nullptr, 1); + for (auto [block, end] = mediator.block_span_[2]; block < end; ++block) + { + mediator.client_has_block_.erase(block); + } got_bad_piece_.emit(nullptr, 2); return wishlist.next(n_wanted, PeerHasAllPieces); @@ -2138,7 +2251,6 @@ TEST_F(PeerMgrWishlistTest, unalignedTorrentPartiallyWanted) // setup: 4 pieces, (100 / 3 * 16) KiB each, all missing // N.B. only the boundary of piece 2 and 3 is aligned - mediator.piece_count_ = 4; mediator.block_span_[0] = { 0, 34 }; mediator.block_span_[1] = { 33, 67 }; mediator.block_span_[2] = { 66, 100 }; @@ -2192,7 +2304,6 @@ TEST_F(PeerMgrWishlistTest, unalignedTorrentDeselectedPiece) // setup: 4 pieces, (100 / 3 * 16) KiB each, all missing // N.B. only the boundary of piece 2 and 3 is aligned - mediator.piece_count_ = 4; mediator.block_span_[0] = { 0, 34 }; mediator.block_span_[1] = { 33, 67 }; mediator.block_span_[2] = { 66, 100 }; @@ -2253,7 +2364,6 @@ TEST_F(PeerMgrWishlistTest, unalignedTorrentDeselected2ConsecutivePieces) // setup: 4 pieces, (100 / 3 * 16) KiB each, all missing // N.B. only the boundary of piece 2 and 3 is aligned - mediator.piece_count_ = 4; mediator.block_span_[0] = { 0, 34 }; mediator.block_span_[1] = { 33, 67 }; mediator.block_span_[2] = { 66, 100 }; @@ -2317,7 +2427,6 @@ TEST_F(PeerMgrWishlistTest, unalignedTorrentSelectedPiece) // setup: 4 pieces, (100 / 3 * 16) KiB each, all missing // N.B. only the boundary of piece 2 and 3 is aligned - mediator.piece_count_ = 4; mediator.block_span_[0] = { 0, 34 }; mediator.block_span_[1] = { 33, 67 }; mediator.block_span_[2] = { 66, 100 }; @@ -2375,7 +2484,6 @@ TEST_F(PeerMgrWishlistTest, unalignedTorrentSelected2ConsecutivePieces) // setup: 4 pieces, (100 / 3 * 16) KiB each, all missing // N.B. only the boundary of piece 2 and 3 is aligned - mediator.piece_count_ = 4; mediator.block_span_[0] = { 0, 34 }; mediator.block_span_[1] = { 33, 67 }; mediator.block_span_[2] = { 66, 100 };