Skip to content

Adds onFinished callback for SessionImpl::updateTrackersFromURL #23510

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.

Sign up for GitHub

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

Closed
vafada wants to merge 1 commit into from
Closed

Adds onFinished callback for SessionImpl::updateTrackersFromURL #23510

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 21 additions & 6 deletions src/base/bittorrent/sessionimpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -683,16 +683,25 @@ SessionImpl::SessionImpl(QObject *parent)
// start embedded tracker
enableTracker(isTrackerEnabled());

prepareStartup();

m_updateTrackersFromURLTimer = new QTimer(this);
m_updateTrackersFromURLTimer->setInterval(24h);
connect(m_updateTrackersFromURLTimer, &QTimer::timeout, this, &SessionImpl::updateTrackersFromURL);
if (isAddTrackersFromURLEnabled())
connect(m_updateTrackersFromURLTimer, &QTimer::timeout, this, [this]()
{
updateTrackersFromURL();
});

if (isAddTrackersFromURLEnabled())
{
updateTrackersFromURL([this]()
{
prepareStartup();
});
Comment on lines +695 to +698
Copy link
Member

@glassez glassez Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't call it a good idea to perform tasks sequentially that are ideal candidates for parallel execution. It seems that the issue requires a slightly more advanced solution. (Of course, if it's worth the effort. #23508 doesn't seem to have a large enough coverage so we could consider it as a known limitation rather than a bug.)

Copy link
Member

@glassez glassez Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have an idea. I’ll give it a try when the chance arises.

Copy link

@Obegg Obegg Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to bother, is there any update on this?

Copy link
Member

@glassez glassez Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to bother, is there any update on this?

I have an idea. I’ll give it a try when the chance arises.

There still wasn't time to do it.

Copy link
Member

@glassez glassez Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, wouldn't it be an acceptable compromise to just keep a list of downloaded trackers stored between sessions? Then it would always be available at startup, although it would probably be outdated, but it is unlikely that it is updated so often that this would be a problem.

Copy link
Contributor Author

@vafada vafada Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@glassez do you suggest storing the list of tracker urls in qBittorrent.conf ? If yes, maybe a can take a stab at this

Copy link
Member

@glassez glassez Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@glassez do you suggest storing the list of tracker urls in qBittorrent.conf ?

Well, certainly not in qBittorrent.conf. In a separate txt file in the qBittorrent data folder.

Copy link
Contributor Author

@vafada vafada Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#23657

m_updateTrackersFromURLTimer->start();
}
else
{
prepareStartup();
}
}

SessionImpl::~SessionImpl()
Expand Down Expand Up @@ -4024,26 +4033,32 @@ void SessionImpl::setAdditionalTrackersFromURL(const QString &trackers)
}
}

void SessionImpl::updateTrackersFromURL()
void SessionImpl::updateTrackersFromURL(const std::function<void()> &onFinished)
{
const QString url = additionalTrackersURL();
if (url.isEmpty())
{
setAdditionalTrackersFromURL({});
if (onFinished)
onFinished();
}
else
{
Net::DownloadManager::instance()->download(Net::DownloadRequest(url)
, Preferences::instance()->useProxyForGeneralPurposes(), this, [this](const Net::DownloadResult &result)
, Preferences::instance()->useProxyForGeneralPurposes(), this, [this, onFinished](const Net::DownloadResult &result)
{
if (result.status == Net::DownloadStatus::Success)
{
setAdditionalTrackersFromURL(QString::fromUtf8(result.data));
LogMsg(tr("Tracker list updated"), Log::INFO);
if (onFinished)
onFinished();
return;
}

LogMsg(tr("Failed to update tracker list. Reason: \"%1\"").arg(result.errorString), Log::WARNING);
if (onFinished)
onFinished();
});
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/base/bittorrent/sessionimpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,7 @@ namespace BitTorrent
void handleRemovedTorrent(const TorrentID &torrentID, const QString &partfileRemoveError = {});

void setAdditionalTrackersFromURL(const QString &trackers);
void updateTrackersFromURL();
void updateTrackersFromURL(const std::function<void()> &onFinished = {});

CachedSettingValue<QString> m_DHTBootstrapNodes;
CachedSettingValue<bool> m_isDHTEnabled;
Expand Down
Loading