fix: crash when a torrent autopauses after encountering an error (#4021)

libtransmission/cache.cc

@@ -133,7 +133,7 @@ Cache::Cache(tr_torrents& torrents, int64_t max_bytes)
 ****
 ***/
 
-void Cache::writeBlock(tr_torrent_id_t tor_id, tr_block_index_t block, std::unique_ptr<std::vector<uint8_t>>& writeme)
+int Cache::writeBlock(tr_torrent_id_t tor_id, tr_block_index_t block, std::unique_ptr<std::vector<uint8_t>>& writeme)
 {
     auto const key = Key{ tor_id, block };
     auto iter = std::lower_bound(std::begin(blocks_), std::end(blocks_), key, CompareCacheBlockByKey{});
@@ -150,7 +150,7 @@ void Cache::writeBlock(tr_torrent_id_t tor_id, tr_block_index_t block, std::uniq
     ++cache_writes_;
     cache_write_bytes_ += std::size(*iter->buf);
 
-    (void)cacheTrim();
+    return cacheTrim();
 }
 
 Cache::CIter Cache::getBlock(tr_torrent const* torrent, tr_block_info::Location loc) noexcept

libtransmission/cache.h

@@ -35,7 +35,9 @@ public:
         return max_bytes_;
     }
 
-    void writeBlock(tr_torrent_id_t tor, tr_block_index_t block, std::unique_ptr<std::vector<uint8_t>>& writeme);
+    // @return any error code from cacheTrim()
+    int writeBlock(tr_torrent_id_t tor, tr_block_index_t block, std::unique_ptr<std::vector<uint8_t>>& writeme);
+
     int readBlock(tr_torrent* torrent, tr_block_info::Location loc, uint32_t len, uint8_t* setme);
     int prefetchBlock(tr_torrent* torrent, tr_block_info::Location loc, uint32_t len);
     int flushTorrent(tr_torrent const* torrent);

libtransmission/peer-io.cc

@@ -119,59 +119,61 @@ static void canReadWrapper(tr_peerIo* io_in)
     tr_session const* const session = io->session;
 
     /* try to consume the input buffer */
-    if (io->canRead != nullptr)
+    if (io->canRead == nullptr)
     {
-        auto const lock = session->unique_lock();
+        return;
+    }
 
-        auto const now = tr_time_msec();
+    auto const lock = session->unique_lock();
-        auto done = bool{ false };
-        auto err = bool{ false };
 
-        while (!done && !err)
+    auto const now = tr_time_msec();
+    auto done = bool{ false };
+    auto err = bool{ false };
+
+    while (!done && !err)
+    {
+        size_t piece = 0;
+        size_t const old_len = io->readBufferSize();
+        int const ret = io->canRead(io.get(), io->userData, &piece);
+        size_t const used = old_len - io->readBufferSize();
+        unsigned int const overhead = guessPacketOverhead(used);
+
+        if (piece != 0 || piece != used)
         {
-            size_t piece = 0;
+            if (piece != 0)
-            size_t const old_len = io->readBufferSize();
-            int const ret = io->canRead(io.get(), io->userData, &piece);
-            size_t const used = old_len - io->readBufferSize();
-            unsigned int const overhead = guessPacketOverhead(used);
-
-            if (piece != 0 || piece != used)
             {
-                if (piece != 0)
+                io->bandwidth().notifyBandwidthConsumed(TR_DOWN, piece, true, now);
-                {
-                    io->bandwidth().notifyBandwidthConsumed(TR_DOWN, piece, true, now);
-                }
-
-                if (used != piece)
-                {
-                    io->bandwidth().notifyBandwidthConsumed(TR_DOWN, used - piece, false, now);
-                }
             }
 
-            if (overhead > 0)
+            if (used != piece)
             {
-                io->bandwidth().notifyBandwidthConsumed(TR_UP, overhead, false, now);
+                io->bandwidth().notifyBandwidthConsumed(TR_DOWN, used - piece, false, now);
+            }
+        }
+
+        if (overhead > 0)
+        {
+            io->bandwidth().notifyBandwidthConsumed(TR_UP, overhead, false, now);
+        }
+
+        switch (ret)
+        {
+        case READ_NOW:
+            if (io->readBufferSize() != 0)
+            {
+                continue;
             }
 
-            switch (ret)
+            done = true;
-            {
+            break;
-            case READ_NOW:
-                if (io->readBufferSize() != 0)
-                {
-                    continue;
-                }
 
-                done = true;
+        case READ_LATER:
-                break;
+            done = true;
+            break;
 
-            case READ_LATER:
+        case READ_ERR:
-                done = true;
+            err = true;
-                break;
+            break;
-
-            case READ_ERR:
-                err = true;
-                break;
-            }
         }
     }
 }

libtransmission/peer-msgs.cc

@@ -1527,12 +1527,7 @@ static ReadState readBtPiece(tr_peerMsgsImpl* msgs, size_t inlen, size_t* setme_
         return READ_LATER;
     }
 
-    // pass the block along...
+    return clientGotBlock(msgs, block_buf, block) != 0 ? READ_ERR : READ_NOW;
-    int const err = clientGotBlock(msgs, block_buf, block);
-    msgs->incoming.block_buf.erase(block);
-
-    // cleanup
-    return err != 0 ? READ_ERR : READ_NOW;
 }
 
 static ReadState readBtMessage(tr_peerMsgsImpl* msgs, size_t inlen)
@@ -1838,9 +1833,17 @@ static int clientGotBlock(
         return 0;
     }
 
-    msgs->session->cache->writeBlock(tor->id(), block, block_data);
+    // NB: if writeBlock() fails the torrent may be paused.
+    // If this happens, `msgs` will be a dangling pointer and must no longer be used.
+    if (auto const err = msgs->session->cache->writeBlock(tor->id(), block, block_data); err != 0)
+    {
+        return err;
+    }
+
     msgs->blame.set(loc.piece);
+    msgs->incoming.block_buf.erase(block);
     msgs->publish(tr_peer_event::GotBlock(tor->blockInfo(), block));
+
     return 0;
 }
 
@@ -1903,8 +1906,6 @@ static ReadState canRead(tr_peerIo* io, void* vmsgs, size_t* piece)
         }
     }
 
-    logtrace(msgs, fmt::format(FMT_STRING("canRead: ret is {:d}"), static_cast<int>(ret)));
-
     return ret;
 }