-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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.
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
Adds onFinished callback for SessionImpl::updateTrackersFromURL #23510
Conversation
| updateTrackersFromURL([this]() | ||
| { | ||
| prepareStartup(); | ||
| }); |
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes #23508.
This PR adds
onFinishedparam callback forSessionImpl::updateTrackersFromURLfunction.The
onFinishedcall back is called whensetAdditionalTrackersFromURLis called or when the download of the tracker list failed.This fixes the async issue when qBittorent is launched via CLI with the following conditions:
--skip-dialog=trueAutomatically append trackers from URL to new downloads:setIn
master, on app launch, an async task is started to download the url set inAdditionalTrackersURL. Meanwhile, the application would process the CLI params and immediately start the download of the torrent file or magnet url specified in the CLI param even before the download task finishes. This bug makes theAutomatically append trackers from URL to new downloads:feature not work properly if the torrent download was triggered via CLI and if add torrent dialog is skipped.With this PR, if the
AdditionalTrackersURLis set, we wait for the download task to finish before starting up the session.caveats
AdditionalTrackersURLis invalid, it would take 30 seconds for the app to startup since the default timeout for the http request is 30 secondsqBittorrent/src/base/net/downloadmanager.cpp
Line 336 in 46cc3a3
if the CLI param has
--skip-dialog=trueand has aurlparam, but thesessionimplclass has no access to CLI params