Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix received metadata handling #13736

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/base/bittorrent/nativetorrentextension.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,11 @@ bool NativeTorrentExtension::on_pause()
// and other extensions to be also invoked.
return false;
}

void NativeTorrentExtension::on_state(const lt::torrent_status::state_t state)
{
if (m_state == lt::torrent_status::downloading_metadata)
m_torrentHandle.set_flags(lt::torrent_flags::stop_when_ready);

m_state = state;
}
2 changes: 2 additions & 0 deletions src/base/bittorrent/nativetorrentextension.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ class NativeTorrentExtension final : public lt::torrent_plugin

private:
bool on_pause() override;
void on_state(lt::torrent_status::state_t state) override;

lt::torrent_handle m_torrentHandle;
lt::torrent_status::state_t m_state = lt::torrent_status::checking_resume_data;
};
65 changes: 28 additions & 37 deletions src/base/bittorrent/session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -593,11 +593,11 @@ void Session::setAppendExtensionEnabled(const bool enabled)
{
if (isAppendExtensionEnabled() != enabled)
{
m_isAppendExtensionEnabled = enabled;

// append or remove .!qB extension for incomplete files
for (TorrentHandleImpl *const torrent : asConst(m_torrents))
torrent->handleAppendExtensionToggled();

m_isAppendExtensionEnabled = enabled;
}
}

Expand Down Expand Up @@ -1892,12 +1892,12 @@ bool Session::deleteTorrent(const InfoHash &hash, const DeleteOption deleteOptio
return true;
}

bool Session::cancelLoadMetadata(const InfoHash &hash)
bool Session::cancelDownloadMetadata(const InfoHash &hash)
{
const auto loadedMetadataIter = m_loadedMetadata.find(hash);
if (loadedMetadataIter == m_loadedMetadata.end()) return false;
const auto downloadedMetadataIter = m_downloadedMetadata.find(hash);
if (downloadedMetadataIter == m_downloadedMetadata.end()) return false;

m_loadedMetadata.erase(loadedMetadataIter);
m_downloadedMetadata.erase(downloadedMetadataIter);
--m_extraLimit;
adjustLimits();
m_nativeSession->remove_torrent(m_nativeSession->find_torrent(hash), lt::session::delete_files);
Expand Down Expand Up @@ -1951,7 +1951,7 @@ void Session::decreaseTorrentsQueuePos(const QVector<InfoHash> &hashes)
torrentQueue.pop();
}

for (auto i = m_loadedMetadata.cbegin(); i != m_loadedMetadata.cend(); ++i)
for (auto i = m_downloadedMetadata.cbegin(); i != m_downloadedMetadata.cend(); ++i)
torrentQueuePositionBottom(m_nativeSession->find_torrent(*i));

saveTorrentsQueue();
Expand Down Expand Up @@ -2004,7 +2004,7 @@ void Session::bottomTorrentsQueuePos(const QVector<InfoHash> &hashes)
torrentQueue.pop();
}

for (auto i = m_loadedMetadata.cbegin(); i != m_loadedMetadata.cend(); ++i)
for (auto i = m_downloadedMetadata.cbegin(); i != m_downloadedMetadata.cend(); ++i)
torrentQueuePositionBottom(m_nativeSession->find_torrent(*i));

saveTorrentsQueue();
Expand Down Expand Up @@ -2058,21 +2058,6 @@ bool Session::addTorrent(const MagnetUri &magnetUri, const AddTorrentParams &par
{
if (!magnetUri.isValid()) return false;

const InfoHash hash = magnetUri.hash();

const auto it = m_loadedMetadata.constFind(hash);
if (it != m_loadedMetadata.constEnd())
{
// It looks illogical that we don't just use an existing handle,
// but as previous experience has shown, it actually creates unnecessary
// problems and unwanted behavior due to the fact that it was originally
// added with parameters other than those provided by the user.
m_loadedMetadata.erase(it);
--m_extraLimit;
adjustLimits();
m_nativeSession->remove_torrent(m_nativeSession->find_torrent(hash), lt::session::delete_files);
}

return addTorrent_impl(params, magnetUri);
}

Expand Down Expand Up @@ -2113,7 +2098,7 @@ LoadTorrentParams Session::initLoadTorrentParams(const AddTorrentParams &addTorr

const QString category = addTorrentParams.category;
if (!category.isEmpty() && !m_categories.contains(category) && !addCategory(category))
loadTorrentParams.category = "";
loadTorrentParams.category = "";
else
loadTorrentParams.category = addTorrentParams.category;

Expand All @@ -2126,9 +2111,15 @@ bool Session::addTorrent_impl(const AddTorrentParams &addTorrentParams, const Ma
const bool hasMetadata = metadata.isValid();
const InfoHash hash = (hasMetadata ? metadata.hash() : magnetUri.hash());

// It looks illogical that we don't just use an existing handle,
// but as previous experience has shown, it actually creates unnecessary
// problems and unwanted behavior due to the fact that it was originally
// added with parameters other than those provided by the user.
cancelDownloadMetadata(hash);

// We should not add the torrent if it is already
// processed or is pending to add to session
if (m_loadingTorrents.contains(hash) || m_loadedMetadata.contains(hash))
if (m_loadingTorrents.contains(hash))
return false;

TorrentHandleImpl *const torrent = m_torrents.value(hash);
Expand Down Expand Up @@ -2276,9 +2267,9 @@ bool Session::findIncompleteFiles(TorrentInfo &torrentInfo, QString &savePath) c
return found;
}

// Add a torrent to the BitTorrent session in hidden mode
// and force it to load its metadata
bool Session::loadMetadata(const MagnetUri &magnetUri)
// Add a torrent to libtorrent session in hidden mode
// and force it to download its metadata
bool Session::downloadMetadata(const MagnetUri &magnetUri)
{
if (!magnetUri.isValid()) return false;

Expand All @@ -2289,7 +2280,7 @@ bool Session::loadMetadata(const MagnetUri &magnetUri)
// processed or adding to session
if (m_torrents.contains(hash)) return false;
if (m_loadingTorrents.contains(hash)) return false;
if (m_loadedMetadata.contains(hash)) return false;
if (m_downloadedMetadata.contains(hash)) return false;

qDebug("Adding torrent to preload metadata...");
qDebug(" -> Hash: %s", qUtf8Printable(hash));
Expand Down Expand Up @@ -2322,13 +2313,13 @@ bool Session::loadMetadata(const MagnetUri &magnetUri)
p.storage = customStorageConstructor;
#endif

// Adding torrent to BitTorrent session
// Adding torrent to libtorrent session
lt::error_code ec;
lt::torrent_handle h = m_nativeSession->add_torrent(p, ec);
if (ec) return false;

// waiting for metadata...
m_loadedMetadata.insert(h.info_hash());
m_downloadedMetadata.insert(h.info_hash());
++m_extraLimit;
adjustLimits();

Expand Down Expand Up @@ -3775,7 +3766,7 @@ bool Session::isKnownTorrent(const InfoHash &hash) const
{
return (m_torrents.contains(hash)
|| m_loadingTorrents.contains(hash)
|| m_loadedMetadata.contains(hash));
|| m_downloadedMetadata.contains(hash));
}

void Session::updateSeedingLimitTimer()
Expand Down Expand Up @@ -4590,7 +4581,7 @@ void Session::createTorrentHandle(const lt::torrent_handle &nativeHandle)

const LoadTorrentParams params = m_loadingTorrents.take(nativeHandle.info_hash());

auto *const torrent = new TorrentHandleImpl {this, nativeHandle, params};
auto *const torrent = new TorrentHandleImpl {this, nativeHandle, m_nativeSession, params};
m_torrents.insert(torrent->hash(), torrent);

const bool hasMetadata = torrent->hasMetadata();
Expand Down Expand Up @@ -4724,18 +4715,18 @@ void Session::handleTorrentDeleteFailedAlert(const lt::torrent_delete_failed_ale
void Session::handleMetadataReceivedAlert(const lt::metadata_received_alert *p)
{
const InfoHash hash {p->handle.info_hash()};
const auto loadedMetadataIter = m_loadedMetadata.find(hash);
const auto downloadedMetadataIter = m_downloadedMetadata.find(hash);

if (loadedMetadataIter != m_loadedMetadata.end())
if (downloadedMetadataIter != m_downloadedMetadata.end())
{
TorrentInfo metadata {p->handle.torrent_file()};

m_loadedMetadata.erase(loadedMetadataIter);
m_downloadedMetadata.erase(downloadedMetadataIter);
--m_extraLimit;
adjustLimits();
m_nativeSession->remove_torrent(p->handle, lt::session::delete_files);

emit metadataLoaded(metadata);
emit metadataDownloaded(metadata);
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/base/bittorrent/session.h
Original file line number Diff line number Diff line change
Expand Up @@ -453,8 +453,8 @@ namespace BitTorrent
bool addTorrent(const MagnetUri &magnetUri, const AddTorrentParams &params = AddTorrentParams());
bool addTorrent(const TorrentInfo &torrentInfo, const AddTorrentParams &params = AddTorrentParams());
bool deleteTorrent(const InfoHash &hash, DeleteOption deleteOption = Torrent);
bool loadMetadata(const MagnetUri &magnetUri);
bool cancelLoadMetadata(const InfoHash &hash);
bool downloadMetadata(const MagnetUri &magnetUri);
bool cancelDownloadMetadata(const InfoHash &hash);

void recursiveTorrentDownload(const InfoHash &hash);
void increaseTorrentsQueuePos(const QVector<InfoHash> &hashes);
Expand Down Expand Up @@ -497,7 +497,7 @@ namespace BitTorrent
void fullDiskError(TorrentHandle *torrent, const QString &msg);
void IPFilterParsed(bool error, int ruleCount);
void loadTorrentFailed(const QString &error);
void metadataLoaded(const TorrentInfo &info);
void metadataDownloaded(const TorrentInfo &info);
void recursiveTorrentDownloadPossible(TorrentHandle *torrent);
void speedLimitModeChanged(bool alternative);
void statsUpdated();
Expand Down Expand Up @@ -764,7 +764,7 @@ namespace BitTorrent
QThread *m_ioThread = nullptr;
ResumeDataSavingManager *m_resumeDataSavingManager = nullptr;

QSet<InfoHash> m_loadedMetadata;
QSet<InfoHash> m_downloadedMetadata;

QHash<InfoHash, TorrentHandleImpl *> m_torrents;
QHash<InfoHash, LoadTorrentParams> m_loadingTorrents;
Expand Down
40 changes: 28 additions & 12 deletions src/base/bittorrent/torrenthandleimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include <libtorrent/alert_types.hpp>
#include <libtorrent/entry.hpp>
#include <libtorrent/magnet_uri.hpp>
#include <libtorrent/session.hpp>
#include <libtorrent/storage_defs.hpp>
#include <libtorrent/time.hpp>
#include <libtorrent/version.hpp>
Expand Down Expand Up @@ -101,10 +102,11 @@ namespace

// TorrentHandleImpl

TorrentHandleImpl::TorrentHandleImpl(Session *session, const lt::torrent_handle &nativeHandle,
const LoadTorrentParams &params)
TorrentHandleImpl::TorrentHandleImpl(Session *session, lt::session *nativeSession
, const lt::torrent_handle &nativeHandle, const LoadTorrentParams &params)
: QObject(session)
, m_session(session)
, m_nativeSession(nativeSession)
, m_nativeHandle(nativeHandle)
, m_name(params.name)
, m_savePath(Utils::Fs::toNativePath(params.savePath))
Expand Down Expand Up @@ -1498,6 +1500,15 @@ void TorrentHandleImpl::handleSaveResumeDataAlert(const lt::save_resume_data_ale
m_ltAddTorrentParams.save_path = Profile::instance()->toPortablePath(
QString::fromStdString(m_ltAddTorrentParams.save_path)).toStdString();

if (m_maintenanceJob == MaintenanceJob::HandleMetadata)
{
m_nativeSession->remove_torrent(m_nativeHandle, lt::session::delete_files);

lt::add_torrent_params p = m_ltAddTorrentParams;
p.ti = m_torrentInfo.nativeInfo();
m_nativeHandle = m_nativeSession->add_torrent(p);
}

auto resumeDataPtr = std::make_shared<lt::entry>(lt::write_resume_data(m_ltAddTorrentParams));
lt::entry &resumeData = *resumeDataPtr;

Expand Down Expand Up @@ -1647,20 +1658,25 @@ void TorrentHandleImpl::handleFileCompletedAlert(const lt::file_completed_alert
void TorrentHandleImpl::handleMetadataReceivedAlert(const lt::metadata_received_alert *p)
{
Q_UNUSED(p);

qDebug("Metadata received for torrent %s.", qUtf8Printable(name()));
updateStatus();
if (m_session->isAppendExtensionEnabled())
manageIncompleteFiles();
if (!m_hasRootFolder)
m_torrentInfo.stripRootFolder();
if (filesCount() == 1)
m_hasRootFolder = false;
m_session->handleTorrentMetadataReceived(this);
m_maintenanceJob = MaintenanceJob::HandleMetadata;

saveResumeData();

// updateStatus();
// if (m_session->isAppendExtensionEnabled())
// manageIncompleteFiles();
// if (!m_hasRootFolder)
// m_torrentInfo.stripRootFolder();
// if (filesCount() == 1)
// m_hasRootFolder = false;
// m_session->handleTorrentMetadataReceived(this);

// If first/last piece priority was specified when adding this torrent,
// we should apply it now that we have metadata:
if (m_hasFirstLastPiecePriority)
applyFirstLastPiecePriority(true);
// if (m_hasFirstLastPiecePriority)
// applyFirstLastPiecePriority(true);
}

void TorrentHandleImpl::handlePerformanceAlert(const lt::performance_alert *p) const
Expand Down
13 changes: 11 additions & 2 deletions src/base/bittorrent/torrenthandleimpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,20 @@ namespace BitTorrent
Overwrite
};

enum class MaintenanceJob
{
None,
HandleMetadata
};

class TorrentHandleImpl final : public QObject, public TorrentHandle
{
Q_DISABLE_COPY(TorrentHandleImpl)
Q_DECLARE_TR_FUNCTIONS(BitTorrent::TorrentHandleImpl)

public:
TorrentHandleImpl(Session *session, const lt::torrent_handle &nativeHandle,
const LoadTorrentParams &params);
TorrentHandleImpl(Session *session, lt::session *nativeSession
, const lt::torrent_handle &nativeHandle, const LoadTorrentParams &params);
~TorrentHandleImpl() override;

bool isValid() const;
Expand Down Expand Up @@ -280,6 +286,7 @@ namespace BitTorrent
void applyFirstLastPiecePriority(bool enabled, const QVector<DownloadPriority> &updatedFilePrio = {});

Session *const m_session;
lt::session *m_nativeSession;
lt::torrent_handle m_nativeHandle;
lt::torrent_status m_nativeStatus;
TorrentState m_state = TorrentState::Unknown;
Expand All @@ -294,6 +301,8 @@ namespace BitTorrent
int m_renameCount = 0;
bool m_storageIsMoving = false;

MaintenanceJob m_maintenanceJob = MaintenanceJob::None;

// Until libtorrent provide an "old_name" field in `file_renamed_alert`
// we will rely on this workaround to remove empty leftover folders
QHash<lt::file_index_t, QVector<QString>> m_oldPath;
Expand Down
8 changes: 4 additions & 4 deletions src/gui/addnewtorrentdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ bool AddNewTorrentDialog::loadMagnet(const BitTorrent::MagnetUri &magnetUri)
return false;
}

connect(BitTorrent::Session::instance(), &BitTorrent::Session::metadataLoaded, this, &AddNewTorrentDialog::updateMetadata);
connect(BitTorrent::Session::instance(), &BitTorrent::Session::metadataDownloaded, this, &AddNewTorrentDialog::updateMetadata);

// Set dialog title
const QString torrentName = magnetUri.name();
Expand All @@ -356,7 +356,7 @@ bool AddNewTorrentDialog::loadMagnet(const BitTorrent::MagnetUri &magnetUri)
setupTreeview();
TMMChanged(m_ui->comboTTM->currentIndex());

BitTorrent::Session::instance()->loadMetadata(magnetUri);
BitTorrent::Session::instance()->downloadMetadata(magnetUri);
setMetadataProgressIndicator(true, tr("Retrieving metadata..."));
m_ui->labelHashData->setText(infoHash);

Expand Down Expand Up @@ -613,7 +613,7 @@ void AddNewTorrentDialog::reject()
if (!m_hasMetadata)
{
setMetadataProgressIndicator(false);
BitTorrent::Session::instance()->cancelLoadMetadata(m_magnetURI.hash());
BitTorrent::Session::instance()->cancelDownloadMetadata(m_magnetURI.hash());
}

QDialog::reject();
Expand All @@ -623,7 +623,7 @@ void AddNewTorrentDialog::updateMetadata(const BitTorrent::TorrentInfo &metadata
{
if (metadata.hash() != m_magnetURI.hash()) return;

disconnect(BitTorrent::Session::instance(), &BitTorrent::Session::metadataLoaded, this, &AddNewTorrentDialog::updateMetadata);
disconnect(BitTorrent::Session::instance(), &BitTorrent::Session::metadataDownloaded, this, &AddNewTorrentDialog::updateMetadata);

if (!metadata.isValid())
{
Expand Down