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

@@ -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_));
     }
 
     // ---

tests/libtransmission/peer-mgr-wishlist-test.cc

@@ -30,7 +30,6 @@ protected:
         mutable std::set<tr_block_index_t> client_has_block_;
         mutable std::set<tr_piece_index_t> client_has_piece_;
         mutable std::set<tr_piece_index_t> 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
+        // We have pieces 0, 2, 3
-        // every block in piece 1
         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 };