fix: magnet links are always paused when added (#4856)

libtransmission/peer-mgr.cc

@@ -43,6 +43,7 @@
 #include "session.h"
 #include "timer.h"
 #include "torrent.h"
+#include "torrent-magnet.h"
 #include "tr-assert.h"
 #include "tr-utp.h"
 #include "utils.h"
@@ -2391,8 +2392,11 @@ void tr_peerMgr::bandwidthPulse()
     session->top_bandwidth_.allocate(Msec);
 
     // torrent upkeep
-    auto& torrents = session->torrents();
+    for (auto* const tor : session->torrents())
-    std::for_each(std::begin(torrents), std::end(torrents), [](auto* tor) { tor->do_idle_work(); });
+    {
+        tor->do_idle_work();
+        tr_torrentMagnetDoIdleWork(tor);
+    }
 
     /* pump the queues */
     queuePulse(session, TR_UP);

libtransmission/resume.cc

@@ -712,7 +712,7 @@ auto loadFromFile(tr_torrent* tor, tr_resume::fields_t fields_to_load, bool* did
 
     if (auto val = bool{}; (fields_to_load & tr_resume::Run) != 0 && tr_variantDictFindBool(&top, TR_KEY_paused, &val))
     {
-        tor->isRunning = !val;
+        tor->start_when_stable = !val;
         fields_loaded |= tr_resume::Run;
     }
 
@@ -831,7 +831,7 @@ auto setFromCtor(tr_torrent* tor, tr_resume::fields_t fields, tr_ctor const* cto
     {
         if (auto is_paused = bool{}; tr_ctorGetPaused(ctor, mode, &is_paused))
         {
-            tor->isRunning = !is_paused;
+            tor->start_when_stable = !is_paused;
             ret |= tr_resume::Run;
         }
     }
@@ -907,7 +907,7 @@ void save(tr_torrent* tor)
     tr_variantDictAddInt(&top, TR_KEY_uploaded, tor->uploadedPrev + tor->uploadedCur);
     tr_variantDictAddInt(&top, TR_KEY_max_peers, tor->peerLimit());
     tr_variantDictAddInt(&top, TR_KEY_bandwidth_priority, tor->getPriority());
-    tr_variantDictAddBool(&top, TR_KEY_paused, !tor->isRunning && !tor->isQueued());
+    tr_variantDictAddBool(&top, TR_KEY_paused, !tor->start_when_stable);
     savePeers(&top, tor);
 
     if (tor->hasMetainfo())

libtransmission/torrent-magnet.cc

@@ -176,13 +176,6 @@ bool tr_torrentUseMetainfoFromFile(
         delete tor->incompleteMetadata;
         tor->incompleteMetadata = nullptr;
     }
-    tor->isStopping = true;
-    tor->magnetVerify = true;
-    if (tor->session->shouldPauseAddedTorrents())
-    {
-        tor->startAfterVerify = false;
-    }
-    tor->markEdited();
 
     return true;
 }
@@ -310,13 +303,6 @@ void on_have_all_metainfo(tr_torrent* tor, tr_incomplete_metadata* m)
     {
         delete tor->incompleteMetadata;
         tor->incompleteMetadata = nullptr;
-        tor->isStopping = true;
-        tor->magnetVerify = true;
-        if (tor->session->shouldPauseAddedTorrents() && !tor->magnetStartAfterVerify)
-        {
-            tor->startAfterVerify = false;
-        }
-        tor->markEdited();
     }
     else /* drat. */
     {
@@ -340,6 +326,19 @@ void on_have_all_metainfo(tr_torrent* tor, tr_incomplete_metadata* m)
 } // namespace set_metadata_piece_helpers
 } // namespace
 
+void tr_torrentMagnetDoIdleWork(tr_torrent* const tor)
+{
+    using namespace set_metadata_piece_helpers;
+
+    TR_ASSERT(tr_isTorrent(tor));
+
+    if (auto* const m = tor->incompleteMetadata; m != nullptr && std::empty(m->pieces_needed))
+    {
+        tr_logAddDebugTor(tor, fmt::format("we now have all the metainfo!"));
+        on_have_all_metainfo(tor, m);
+    }
+}
+
 void tr_torrentSetMetadataPiece(tr_torrent* tor, int piece, void const* data, size_t len)
 {
     using namespace set_metadata_piece_helpers;
@@ -384,13 +383,6 @@ void tr_torrentSetMetadataPiece(tr_torrent* tor, int piece, void const* data, si
 
     needed.erase(iter);
     tr_logAddDebugTor(tor, fmt::format("saving metainfo piece {}... {} remain", piece, std::size(needed)));
-
-    // are we done?
-    if (std::empty(needed))
-    {
-        tr_logAddDebugTor(tor, fmt::format("metainfo piece {} was the last one", piece));
-        on_have_all_metainfo(tor, m);
-    }
 }
 
 // ---

libtransmission/torrent-magnet.h

@@ -33,6 +33,8 @@ bool tr_torrentSetMetadataSizeHint(tr_torrent* tor, int64_t metadata_size);
 
 double tr_torrentGetMetadataPercent(tr_torrent const* tor);
 
+void tr_torrentMagnetDoIdleWork(tr_torrent* tor);
+
 bool tr_torrentUseMetainfoFromFile(
     tr_torrent* tor,
     tr_torrent_metainfo const* metainfo,

libtransmission/torrent.cc

@@ -830,6 +830,8 @@ void torrentStart(tr_torrent* tor, torrent_start_opts opts)
 {
     using namespace start_stop_helpers;
 
+    auto const lock = tor->unique_lock();
+
     switch (tor->activity())
     {
     case TR_STATUS_SEED:
@@ -849,7 +851,6 @@ void torrentStart(tr_torrent* tor, torrent_start_opts opts)
     case TR_STATUS_CHECK_WAIT:
         /* verifying right now... wait until that's done so
          * we'll know what completeness to use/announce */
-        tor->startAfterVerify = true;
         return;
 
     case TR_STATUS_STOPPED:
@@ -868,9 +869,6 @@ void torrentStart(tr_torrent* tor, torrent_start_opts opts)
         return;
     }
 
-    /* otherwise, start it now... */
-    auto const lock = tor->unique_lock();
-
     /* allow finished torrents to be resumed */
     if (tr_torrentIsSeedRatioDone(tor))
     {
@@ -895,6 +893,9 @@ void torrentStop(tr_torrent* const tor)
     TR_ASSERT(tor->session->amInSessionThread());
     auto const lock = tor->unique_lock();
 
+    tor->isRunning = false;
+    tor->isStopping = false;
+
     if (!tor->session->isClosing())
     {
         tr_logAddInfoTor(tor, _("Pausing torrent"));
@@ -913,16 +914,6 @@ void torrentStop(tr_torrent* const tor)
     }
 
     torrentSetQueued(tor, false);
-
-    if (tor->magnetVerify)
-    {
-        tor->magnetVerify = false;
-        tr_logAddTraceTor(tor, "Magnet Verify");
-        tor->refreshCurrentDir();
-        tr_torrentVerify(tor);
-
-        callScriptIfEnabled(tor, TR_SCRIPT_ON_TORRENT_ADDED);
-    }
 }
 } // namespace
 
@@ -935,8 +926,7 @@ void tr_torrentStop(tr_torrent* tor)
 
     auto const lock = tor->unique_lock();
 
-    tor->isRunning = false;
+    tor->start_when_stable = false;
-    tor->isStopping = false;
     tor->setDirty();
     tor->session->runInSessionThread(torrentStop, tor);
 }
@@ -965,7 +955,6 @@ void tr_torrentFreeInSessionThread(tr_torrent* tor)
         tr_logAddInfoTor(tor, _("Removing torrent"));
     }
 
-    tor->magnetVerify = false;
     torrentStop(tor);
 
     if (tor->isDeleting)
@@ -975,7 +964,6 @@ void tr_torrentFreeInSessionThread(tr_torrent* tor)
         tr_torrent_metainfo::removeFile(tor->session->resumeDir(), tor->name(), tor->infoHashString(), ".resume"sv);
     }
 
-    tor->isRunning = false;
     freeTorrent(tor);
 }
 
@@ -1036,6 +1024,34 @@ void torrentInitFromInfoDict(tr_torrent* tor)
     tor->checked_pieces_ = tr_bitfield{ size_t(tor->pieceCount()) };
 }
 
+void on_metainfo_completed(tr_torrent* tor)
+{
+    // we can look for files now that we know what files are in the torrent
+    tor->refreshCurrentDir();
+
+    callScriptIfEnabled(tor, TR_SCRIPT_ON_TORRENT_ADDED);
+
+    if (tor->session->shouldFullyVerifyAddedTorrents() || !isNewTorrentASeed(tor))
+    {
+        tr_torrentVerify(tor);
+    }
+    else
+    {
+        tor->completion.setHasAll();
+        tor->doneDate = tor->addedDate;
+        tor->recheckCompleteness();
+
+        if (tor->start_when_stable)
+        {
+            torrentStart(tor, {});
+        }
+        else if (tor->isRunning)
+        {
+            tr_torrentStop(tor);
+        }
+    }
+}
+
 void torrentInit(tr_torrent* tor, tr_ctor const* ctor)
 {
     tr_session* session = tr_ctorGetSession(ctor);
@@ -1081,7 +1097,7 @@ void torrentInit(tr_torrent* tor, tr_ctor const* ctor)
     tor->anyDate = now;
 
     tr_resume::fields_t loaded = {};
-    if (tor->hasMetainfo())
+
     {
         // tr_resume::load() calls a lot of tr_torrentSetFoo() methods
         // that set things as dirty, but... these settings being loaded are
@@ -1106,9 +1122,6 @@ void torrentInit(tr_torrent* tor, tr_ctor const* ctor)
 
     tor->refreshCurrentDir();
 
-    bool const do_start = tor->isRunning;
-    tor->isRunning = false;
-
     if ((loaded & tr_resume::Speedlimit) == 0)
     {
         tor->useSpeedLimit(TR_UP, false);
@@ -1174,37 +1187,14 @@ void torrentInit(tr_torrent* tor, tr_ctor const* ctor)
 
     tor->torrent_announcer = session->announcer_->addTorrent(tor, &tr_torrent::onTrackerResponse);
 
-    if (is_new_torrent)
+    if (auto const has_metainfo = tor->hasMetainfo(); is_new_torrent && has_metainfo)
     {
-        if (tor->hasMetainfo())
+        on_metainfo_completed(tor);
-        {
-            callScriptIfEnabled(tor, TR_SCRIPT_ON_TORRENT_ADDED);
-        }
-
-        if (!tor->hasMetainfo() && !do_start)
-        {
-            auto opts = torrent_start_opts{};
-            opts.bypass_queue = true;
-            opts.has_local_data = has_local_data;
-            torrentStart(tor, opts);
-        }
-        else if (!session->shouldFullyVerifyAddedTorrents() && isNewTorrentASeed(tor))
-        {
-            tor->completion.setHasAll();
-            tor->doneDate = tor->addedDate;
-            tor->recheckCompleteness();
-        }
-        else
-        {
-            tor->startAfterVerify = do_start;
-            tr_torrentVerify(tor);
-        }
     }
-    else if (do_start)
+    else if (tor->start_when_stable || !has_metainfo)
     {
-        // if checked_pieces_ got populated from the loading the resume
-        // file above, then torrentStart doesn't need to check again
         auto opts = torrent_start_opts{};
+        opts.bypass_queue = !has_metainfo; // to fetch metainfo from peers
         opts.has_local_data = has_local_data;
         torrentStart(tor, opts);
     }
@@ -1216,16 +1206,20 @@ void torrentInit(tr_torrent* tor, tr_ctor const* ctor)
 } // namespace torrent_init_helpers
 } // namespace
 
-void tr_torrent::setMetainfo(tr_torrent_metainfo const& tm)
+void tr_torrent::setMetainfo(tr_torrent_metainfo tm)
 {
     using namespace torrent_init_helpers;
 
-    metainfo_ = tm;
+    TR_ASSERT(!hasMetainfo());
+    metainfo_ = std::move(tm);
 
     torrentInitFromInfoDict(this);
     tr_peerMgrOnTorrentGotMetainfo(this);
     session->onMetadataCompleted(this);
     this->setDirty();
+    this->markEdited();
+
+    on_metainfo_completed(this);
 }
 
 tr_torrent* tr_torrentNew(tr_ctor* ctor, tr_torrent** setme_duplicate_of)
@@ -1763,33 +1757,35 @@ void tr_torrentAmountFinished(tr_torrent const* tor, float* tabs, int n_tabs)
 
 // --- Start/Stop Callback
 
+namespace
+{
+void tr_torrentStartImpl(tr_torrent* tor, bool bypass_queue)
+{
+    if (!tr_isTorrent(tor))
+    {
+        return;
+    }
+
+    tor->start_when_stable = true;
+    auto opts = torrent_start_opts{};
+    opts.bypass_queue = bypass_queue;
+    torrentStart(tor, opts);
+}
+} // namespace
+
 void tr_torrentStart(tr_torrent* tor)
 {
-    if (tr_isTorrent(tor))
+    tr_torrentStartImpl(tor, false);
-    {
-        tor->startAfterVerify = true;
-        torrentStart(tor, {});
-    }
 }
 
 void tr_torrentStartNow(tr_torrent* tor)
 {
-    if (tr_isTorrent(tor))
+    tr_torrentStartImpl(tor, true);
-    {
-        auto opts = torrent_start_opts{};
-        opts.bypass_queue = true;
-        torrentStart(tor, opts);
-    }
 }
 
 void tr_torrentStartMagnet(tr_torrent* tor)
 {
-    if (tr_isTorrent(tor))
+    tr_torrentStart(tor);
-    {
-        tor->magnetStartAfterVerify = true;
-        tor->startAfterVerify = true;
-        torrentStart(tor, {});
-    }
 }
 
 // ---
@@ -1809,10 +1805,8 @@ void onVerifyDoneThreadFunc(tr_torrent* const tor)
 
     tor->recheckCompleteness();
 
-    if (tor->startAfterVerify)
+    if (tor->start_when_stable)
     {
-        tor->startAfterVerify = false;
-
         auto opts = torrent_start_opts{};
         opts.has_local_data = !tor->checked_pieces_.hasNone();
         torrentStart(tor, opts);
@@ -1832,20 +1826,18 @@ void verifyTorrent(tr_torrent* const tor)
     /* if the torrent's already being verified, stop it */
     tor->session->verifyRemove(tor);
 
-    bool const start_after = (tor->isRunning || tor->startAfterVerify) && !tor->isStopping;
+    if (!tor->hasMetainfo())
+    {
+        return;
+    }
 
     if (tor->isRunning)
     {
-        tr_torrentStop(tor);
+        torrentStop(tor);
     }
 
-    if (setLocalErrorIfFilesDisappeared(tor))
+    if (!setLocalErrorIfFilesDisappeared(tor))
     {
-        tor->startAfterVerify = false;
-    }
-    else
-    {
-        tor->startAfterVerify = start_after;
         tor->session->verifyAdd(tor);
     }
 }

libtransmission/torrent.h

@@ -109,7 +109,8 @@ public:
     // but more refactoring is needed before that can happen
     // because much of tr_torrent's impl is in the non-member C bindings
 
-    void setMetainfo(tr_torrent_metainfo const& tm);
+    // Used to add metainfo to a magnet torrent.
+    void setMetainfo(tr_torrent_metainfo tm);
 
     [[nodiscard]] auto unique_lock() const
     {
@@ -903,10 +904,10 @@ public:
     bool is_queued = false;
     bool isRunning = false;
     bool isStopping = false;
-    bool startAfterVerify = false;
-    bool magnetStartAfterVerify = false;
 
-    bool magnetVerify = false;
+    // start the torrent after all the startup scaffolding is done,
+    // e.g. fetching metadata from peers and/or verifying the torrent
+    bool start_when_stable = false;
 
 private:
     [[nodiscard]] constexpr bool isPieceTransferAllowed(tr_direction direction) const noexcept

libtransmission/transmission.h

@@ -601,7 +601,7 @@ void tr_sessionSetAntiBruteForceEnabled(tr_session* session, bool enabled);
 /** @brief Like `tr_torrentStart()`, but resumes right away regardless of the queues. */
 void tr_torrentStartNow(tr_torrent* tor);
 
-/** @brief Like tr_torrentStart(), but sets magnetStartAfterVerify to true. */
+/** @brief DEPRECATED. Equivalent to tr_torrentStart(). Use that instead. */
 void tr_torrentStartMagnet(tr_torrent*);
 
 /** @brief Return the queued torrent's position in the queue it's in. [0...n) */

macosx/Torrent.mm

@@ -280,7 +280,7 @@ bool trashDataFile(char const* filename, void* /*user_data*/, tr_error** error)
 {
     if ([self alertForRemainingDiskSpace])
     {
-        tr_torrentStartMagnet(self.fHandle);
+        tr_torrentStart(self.fHandle);
         [self update];
 
         //capture, specifically, stop-seeding settings changing to unlimited