From a466ff5057f845bcfd0dfebc44044f29eb2cf541 Mon Sep 17 00:00:00 2001 From: "Vladimir Golovnev (Glassez)" Date: Fri, 28 Jun 2019 21:24:39 +0300 Subject: [PATCH 1/4] Fix torrent checking issues Start all torrents auto-managed to prevent simultaneous checking of multiple torrents. Handle checking state of paused torrent to prevent it from being resumed when qBittorrent is closed until checking isn't complete. --- src/base/bittorrent/session.cpp | 80 ++++++++++++++++++--------- src/base/bittorrent/session.h | 3 +- src/base/bittorrent/torrenthandle.cpp | 74 ++++++++++++++++--------- src/base/bittorrent/torrenthandle.h | 11 ++-- 4 files changed, 110 insertions(+), 58 deletions(-) diff --git a/src/base/bittorrent/session.cpp b/src/base/bittorrent/session.cpp index 0b6162f0b..0a5778f11 100644 --- a/src/base/bittorrent/session.cpp +++ b/src/base/bittorrent/session.cpp @@ -2148,25 +2148,19 @@ bool Session::addTorrent_impl(CreateTorrentParams params, const MagnetUri &magne hash = magnetUri.hash(); if (m_loadedMetadata.contains(hash)) { - // Adding preloaded torrent - m_loadedMetadata.remove(hash); - libt::torrent_handle handle = m_nativeSession->find_torrent(hash); - --m_extraLimit; + /// Adding preloaded torrent... + m_addingTorrents.insert(hash, params); + + lt::torrent_handle handle = m_nativeSession->find_torrent(hash); + // We need to pause it first to create TorrentHandle within the same + // underlying state as in other cases. try { handle.auto_managed(false); - // Preloaded torrent is in "Upload mode" so we need to disable it - // otherwise the torrent never be downloaded (until application restart) - handle.set_upload_mode(false); handle.pause(); } catch (std::exception &) {} - adjustLimits(); - - // use common 2nd step of torrent addition - m_addingTorrents.insert(hash, params); - createTorrentHandle(handle); return true; } @@ -2225,9 +2219,12 @@ bool Session::addTorrent_impl(CreateTorrentParams params, const MagnetUri &magne else p.storage_mode = libt::storage_mode_sparse; - p.flags |= libt::add_torrent_params::flag_paused; // Start in pause - p.flags &= ~libt::add_torrent_params::flag_auto_managed; // Because it is added in paused state p.flags &= ~libt::add_torrent_params::flag_duplicate_is_error; // Already checked + // Make sure the torrent will be initially checked and then paused + // to perform some service jobs on it. We will start it if needed. + p.flags |= lt::add_torrent_params::flag_paused; + p.flags |= lt::add_torrent_params::flag_auto_managed; + p.flags |= lt::add_torrent_params::flag_stop_when_ready; // Seeding mode // Skip checking and directly start seeding (new in libtorrent v0.15) @@ -2239,7 +2236,15 @@ bool Session::addTorrent_impl(CreateTorrentParams params, const MagnetUri &magne if (!fromMagnetUri) { if (params.restored) { // Set torrent fast resume data - p.resume_data = {fastresumeData.constData(), fastresumeData.constData() + fastresumeData.size()}; + // Make sure the torrent will be initially checked and then paused + // to perform some service jobs on it. We will start it if needed. + // (Workaround to easily support libtorrent-1.1) + QByteArray patchedFastresumeData = fastresumeData; + patchedFastresumeData.replace("6:pausedi0e", "6:pausedi1e"); + patchedFastresumeData.replace("12:auto_managedi0e", "12:auto_managedi1e"); + + p.resume_data = std::vector {patchedFastresumeData.constData() + , (patchedFastresumeData.constData() + patchedFastresumeData.size())}; p.flags |= libt::add_torrent_params::flag_use_resume_save_path; } else { @@ -2247,12 +2252,6 @@ bool Session::addTorrent_impl(CreateTorrentParams params, const MagnetUri &magne } } - if (params.restored && !params.paused) { - // Make sure the torrent will restored in "paused" state - // Then we will start it if needed - p.flags |= libt::add_torrent_params::flag_stop_when_ready; - } - // Limits p.max_connections = maxConnectionsPerTorrent(); p.max_uploads = maxUploadsPerTorrent(); @@ -4148,10 +4147,7 @@ void Session::handleAlert(libt::alert *a) case libt::tracker_warning_alert::alert_type: case libt::fastresume_rejected_alert::alert_type: case libt::torrent_checked_alert::alert_type: - dispatchTorrentAlert(a); - break; case libt::metadata_received_alert::alert_type: - handleMetadataReceivedAlert(static_cast(a)); dispatchTorrentAlert(a); break; case libt::state_update_alert::alert_type: @@ -4211,8 +4207,19 @@ void Session::handleAlert(libt::alert *a) void Session::dispatchTorrentAlert(libt::alert *a) { TorrentHandle *const torrent = m_torrents.value(static_cast(a)->handle.info_hash()); - if (torrent) + if (torrent) { torrent->handleAlert(a); + return; + } + + switch (a->type()) { + case libt::torrent_paused_alert::alert_type: + handleTorrentPausedAlert(static_cast(a)); + break; + case libt::metadata_received_alert::alert_type: + handleMetadataReceivedAlert(static_cast(a)); + break; + } } void Session::createTorrentHandle(const libt::torrent_handle &nativeHandle) @@ -4326,7 +4333,7 @@ void Session::handleTorrentDeleteFailedAlert(libt::torrent_delete_failed_alert * } } -void Session::handleMetadataReceivedAlert(libt::metadata_received_alert *p) +void Session::handleMetadataReceivedAlert(const libt::metadata_received_alert *p) { InfoHash hash = p->handle.info_hash(); @@ -4338,6 +4345,27 @@ void Session::handleMetadataReceivedAlert(libt::metadata_received_alert *p) } } +void Session::handleTorrentPausedAlert(const libtorrent::torrent_paused_alert *p) +{ + const InfoHash hash {p->handle.info_hash()}; + + if (m_loadedMetadata.contains(hash)) { + // Adding preloaded torrent + m_loadedMetadata.remove(hash); + lt::torrent_handle handle = p->handle; + --m_extraLimit; + + // Preloaded torrent is in "Upload mode" so we need to disable it + // otherwise the torrent never be downloaded (until application restart) + handle.set_upload_mode(false); + handle.auto_managed(true); + + adjustLimits(); + // use common 2nd step of torrent addition + createTorrentHandle(handle); + } +} + void Session::handleFileErrorAlert(libt::file_error_alert *p) { TorrentHandle *const torrent = m_torrents.value(p->handle.info_hash()); diff --git a/src/base/bittorrent/session.h b/src/base/bittorrent/session.h index 3b863b737..8888c73fb 100644 --- a/src/base/bittorrent/session.h +++ b/src/base/bittorrent/session.h @@ -614,7 +614,8 @@ namespace BitTorrent void dispatchTorrentAlert(libtorrent::alert *a); void handleAddTorrentAlert(libtorrent::add_torrent_alert *p); void handleStateUpdateAlert(libtorrent::state_update_alert *p); - void handleMetadataReceivedAlert(libtorrent::metadata_received_alert *p); + void handleMetadataReceivedAlert(const libtorrent::metadata_received_alert *p); + void handleTorrentPausedAlert(const libtorrent::torrent_paused_alert *p); void handleFileErrorAlert(libtorrent::file_error_alert *p); void handleTorrentRemovedAlert(libtorrent::torrent_removed_alert *p); void handleTorrentDeletedAlert(libtorrent::torrent_deleted_alert *p); diff --git a/src/base/bittorrent/torrenthandle.cpp b/src/base/bittorrent/torrenthandle.cpp index 5d4667649..ef29b92a3 100644 --- a/src/base/bittorrent/torrenthandle.cpp +++ b/src/base/bittorrent/torrenthandle.cpp @@ -192,6 +192,7 @@ TorrentHandle::TorrentHandle(Session *session, const libtorrent::torrent_handle , m_hasRootFolder(params.hasRootFolder) , m_needsToSetFirstLastPiecePriority(false) , m_needsToStartForced(params.forced) + , m_pauseWhenReady(params.paused) { if (m_useAutoTMM) m_savePath = Utils::Fs::toNativePath(m_session->categorySavePath(m_category)); @@ -217,18 +218,18 @@ TorrentHandle::TorrentHandle(Session *session, const libtorrent::torrent_handle m_hasRootFolder = false; } - // "started" means "all initialization has completed and torrent has started regular processing". - // When torrent added/restored in "paused" state it become "started" immediately after construction. - // When it is added/restored in "resumed" state, it become "started" after it is really resumed - // (i.e. after receiving "torrent resumed" alert). - if (params.paused) { - m_startupState = Started; - } - else if (!params.restored || !hasMetadata()) { - // Resume torrent because it was added in "resumed" state - // but it's actually paused during initialization - m_startupState = Starting; - resume(params.forced); + if (!hasMetadata()) { + // There is nothing to prepare + if (!m_pauseWhenReady) { + // Resume torrent because it was added in "resumed" state + // but it's actually paused during initialization. + m_startupState = Starting; + resume(m_needsToStartForced); + } + else { + m_startupState = Started; + m_pauseWhenReady = false; + } } } @@ -1275,7 +1276,8 @@ void TorrentHandle::forceRecheck() if (isPaused()) { m_nativeHandle.stop_when_ready(true); - resume_impl(false); + m_nativeHandle.auto_managed(true); + m_pauseWhenReady = true; } } @@ -1554,17 +1556,22 @@ void TorrentHandle::handleTorrentCheckedAlert(const libtorrent::torrent_checked_ Q_UNUSED(p); qDebug("\"%s\" have just finished checking", qUtf8Printable(name())); - if (m_startupState == NotStarted) { - if (!m_hasMissingFiles) { - // Resume torrent because it was added in "resumed" state - // but it's actually paused during initialization. - m_startupState = Starting; - resume(m_needsToStartForced); + if (m_startupState == Preparing) { + if (!m_pauseWhenReady) { + if (!m_hasMissingFiles) { + // Resume torrent because it was added in "resumed" state + // but it's actually paused during initialization. + m_startupState = Starting; + resume(m_needsToStartForced); + } + else { + // Torrent that has missing files is paused. + m_startupState = Started; + } } else { - // Torrent that has missing files is marked as "started" - // but it remains paused. m_startupState = Started; + m_pauseWhenReady = false; } } @@ -1588,10 +1595,10 @@ void TorrentHandle::handleTorrentFinishedAlert(const libtorrent::torrent_finishe Q_UNUSED(p); qDebug("Got a torrent finished alert for \"%s\"", qUtf8Printable(name())); qDebug("Torrent has seed status: %s", m_hasSeedStatus ? "yes" : "no"); + m_hasMissingFiles = false; if (m_hasSeedStatus) return; updateStatus(); - m_hasMissingFiles = false; m_hasSeedStatus = true; adjustActualSavePath(); @@ -1615,9 +1622,14 @@ void TorrentHandle::handleTorrentPausedAlert(const libtorrent::torrent_paused_al Q_UNUSED(p); if (m_startupState == Started) { - updateStatus(); - m_speedMonitor.reset(); - m_session->handleTorrentPaused(this); + if (!m_pauseWhenReady) { + updateStatus(); + m_speedMonitor.reset(); + m_session->handleTorrentPaused(this); + } + else { + m_pauseWhenReady = false; + } } } @@ -1639,8 +1651,8 @@ void TorrentHandle::handleSaveResumeDataAlert(const libtorrent::save_resume_data libtorrent::entry &resumeData = useDummyResumeData ? dummyEntry : *(p->resume_data); if (useDummyResumeData) { resumeData["qBt-magnetUri"] = toMagnetUri().toStdString(); - resumeData["qBt-paused"] = isPaused(); - resumeData["qBt-forced"] = isForced(); + resumeData["paused"] = isPaused(); + resumeData["auto_managed"] = m_nativeStatus.auto_managed; // Both firstLastPiecePriority and sequential need to be stored in the // resume data if there is no metadata, otherwise they won't be // restored if qBittorrent quits before the metadata are retrieved: @@ -1662,6 +1674,14 @@ void TorrentHandle::handleSaveResumeDataAlert(const libtorrent::save_resume_data resumeData["qBt-queuePosition"] = (nativeHandle().queue_position() + 1); // qBt starts queue at 1 resumeData["qBt-hasRootFolder"] = m_hasRootFolder; + if (m_pauseWhenReady) { + // We need to redefine these values when torrent starting/rechecking + // in "paused" state since native values can be logically wrong + // (torrent can be not paused and auto_managed when it is checking). + resumeData["paused"] = true; + resumeData["auto_managed"] = false; + } + m_session->handleTorrentResumeDataReady(this, resumeData); } diff --git a/src/base/bittorrent/torrenthandle.h b/src/base/bittorrent/torrenthandle.h index c9d9d9a67..3c002c52b 100644 --- a/src/base/bittorrent/torrenthandle.h +++ b/src/base/bittorrent/torrenthandle.h @@ -476,12 +476,15 @@ namespace BitTorrent enum StartupState { - NotStarted, - Starting, - Started + Preparing, // torrent is preparing to start regular processing + Starting, // torrent is prepared and starting to perform regular processing + Started // torrent is performing regular processing }; + StartupState m_startupState = Preparing; + // Handle torrent state when it starts performing some service job + // being in Paused state so it might be unpaused internally and then paused again + bool m_pauseWhenReady; - StartupState m_startupState = NotStarted; bool m_unchecked = false; }; } From cdcc7a210bc3d9d5f0f350c5392ab60ffec4382b Mon Sep 17 00:00:00 2001 From: "Vladimir Golovnev (Glassez)" Date: Wed, 10 Jul 2019 19:54:10 +0300 Subject: [PATCH 2/4] Avoid race conditions when adding torrent --- src/base/bittorrent/session.cpp | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/base/bittorrent/session.cpp b/src/base/bittorrent/session.cpp index 0a5778f11..67e228c15 100644 --- a/src/base/bittorrent/session.cpp +++ b/src/base/bittorrent/session.cpp @@ -2147,20 +2147,26 @@ bool Session::addTorrent_impl(CreateTorrentParams params, const MagnetUri &magne if (fromMagnetUri) { hash = magnetUri.hash(); - if (m_loadedMetadata.contains(hash)) { - /// Adding preloaded torrent... - m_addingTorrents.insert(hash, params); + const auto it = m_loadedMetadata.constFind(hash); + if (it != m_loadedMetadata.constEnd()) { + // Adding preloaded torrent... + const TorrentInfo metadata = it.value(); + if (metadata.isValid()) { + // Metadata is received and torrent_handle is being deleted + // so we can't reuse it. Just add torrent using its metadata. + return addTorrent_impl(params + , MagnetUri {}, metadata, fastresumeData); + } + // Reuse existing torrent_handle lt::torrent_handle handle = m_nativeSession->find_torrent(hash); // We need to pause it first to create TorrentHandle within the same // underlying state as in other cases. + handle.auto_managed(false); + handle.pause(); - try { - handle.auto_managed(false); - handle.pause(); - } - catch (std::exception &) {} - + m_loadedMetadata.remove(hash); + m_addingTorrents.insert(hash, params); return true; } @@ -4349,16 +4355,14 @@ void Session::handleTorrentPausedAlert(const libtorrent::torrent_paused_alert *p { const InfoHash hash {p->handle.info_hash()}; - if (m_loadedMetadata.contains(hash)) { + if (m_addingTorrents.contains(hash)) { // Adding preloaded torrent - m_loadedMetadata.remove(hash); lt::torrent_handle handle = p->handle; --m_extraLimit; // Preloaded torrent is in "Upload mode" so we need to disable it // otherwise the torrent never be downloaded (until application restart) handle.set_upload_mode(false); - handle.auto_managed(true); adjustLimits(); // use common 2nd step of torrent addition From 600791329147067dc84497e21a7d614d727719b1 Mon Sep 17 00:00:00 2001 From: "Vladimir Golovnev (Glassez)" Date: Sun, 14 Jul 2019 11:29:49 +0300 Subject: [PATCH 3/4] Ignore some actions on uninitialized torrents Some actions can lead to an inconsistent state if applied to an uninitialized torrent, so we just ignore them. --- src/base/bittorrent/torrenthandle.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/base/bittorrent/torrenthandle.cpp b/src/base/bittorrent/torrenthandle.cpp index ef29b92a3..d7a99a8b5 100644 --- a/src/base/bittorrent/torrenthandle.cpp +++ b/src/base/bittorrent/torrenthandle.cpp @@ -224,7 +224,7 @@ TorrentHandle::TorrentHandle(Session *session, const libtorrent::torrent_handle // Resume torrent because it was added in "resumed" state // but it's actually paused during initialization. m_startupState = Starting; - resume(m_needsToStartForced); + resume_impl(m_needsToStartForced); } else { m_startupState = Started; @@ -1231,6 +1231,8 @@ bool TorrentHandle::setCategory(const QString &category) void TorrentHandle::move(QString path) { + if (m_startupState != Started) return; + m_useAutoTMM = false; m_session->handleTorrentSavingModeChanged(this); @@ -1269,6 +1271,7 @@ void TorrentHandle::forceDHTAnnounce() void TorrentHandle::forceRecheck() { + if (m_startupState != Started) return; if (!hasMetadata()) return; m_nativeHandle.force_recheck(); @@ -1346,6 +1349,7 @@ void TorrentHandle::toggleFirstLastPiecePriority() void TorrentHandle::pause() { + if (m_startupState != Started) return; if (isPaused()) return; m_nativeHandle.auto_managed(false); @@ -1360,6 +1364,8 @@ void TorrentHandle::pause() void TorrentHandle::resume(bool forced) { + if (m_startupState != Started) return; + resume_impl(forced); } @@ -1408,6 +1414,8 @@ void TorrentHandle::setTrackerLogin(const QString &username, const QString &pass void TorrentHandle::renameFile(int index, const QString &name) { + if (m_startupState != Started) return; + m_oldPath[LTFileIndex {index}].push_back(filePath(index)); ++m_renameCount; qDebug() << Q_FUNC_INFO << index << name; @@ -1562,7 +1570,7 @@ void TorrentHandle::handleTorrentCheckedAlert(const libtorrent::torrent_checked_ // Resume torrent because it was added in "resumed" state // but it's actually paused during initialization. m_startupState = Starting; - resume(m_needsToStartForced); + resume_impl(m_needsToStartForced); } else { // Torrent that has missing files is paused. From 988f7e2ef81661a1f2defcdbbabbdc6274006298 Mon Sep 17 00:00:00 2001 From: "Vladimir Golovnev (Glassez)" Date: Sun, 14 Jul 2019 11:50:44 +0300 Subject: [PATCH 4/4] Don't break torrent checking --- src/base/bittorrent/torrenthandle.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/base/bittorrent/torrenthandle.cpp b/src/base/bittorrent/torrenthandle.cpp index d7a99a8b5..47a140578 100644 --- a/src/base/bittorrent/torrenthandle.cpp +++ b/src/base/bittorrent/torrenthandle.cpp @@ -1350,6 +1350,12 @@ void TorrentHandle::toggleFirstLastPiecePriority() void TorrentHandle::pause() { if (m_startupState != Started) return; + if (m_pauseWhenReady) return; + if (isChecking()) { + m_pauseWhenReady = true; + return; + } + if (isPaused()) return; m_nativeHandle.auto_managed(false); @@ -1366,6 +1372,7 @@ void TorrentHandle::resume(bool forced) { if (m_startupState != Started) return; + m_pauseWhenReady = false; resume_impl(forced); }