Pause session till all torrents are fully started #9007

sledgehammer999 · 2018-05-30T00:16:07Z

There were many user reports that complained that qbt started downloading over their data when they had forgotten to plugin the drive the torrents were in. Or forgotten to mount the partition/share etc.

I was confused because I personally had implemented a measure against this. I was even more confused because I couldn't reproduce it. Until some days ago. After a lot of trial and error it turns out that during startup libtorrent produces a ton of alerts. A lot more than 1000 at a time. And these alerts ended up throwing out of the queue the fastresume_rejected_alert alerts, thus my counter measure was never activated. A longer discussion here: arvidn/libtorrent#3045

Naturally as I had delved into this I discovered other inefficiencies. There's a timing issue where the torrent might start downloading and writing data to disk before we have the chance to pause it.
I opted to have the session paused untill all torrents were loaded and handled.

To my experience, the 3 alerts that interest us are received in this order:
add_torrent_alert -> fastresume_rejected_alert -> torrent_added_alert

About the "Temp solution" commit. This is temporary fix in order to publish the PR and have feedback from you. Relevant bug report: arvidn/libtorrent#3071

IMO, it's better to see each commit by itself.

Caveats:

I didn't thoroughly test the PR. Please help by doing runs yourselves too.
The RC_1_0 code path wasn't even tested if it compiles. I will do it as final step before merge.

wesleybl · 2018-05-30T00:37:58Z

@sledgehammer999 if I remove the disk where the torrent was downloaded and start the qbittorrent what will happen with the percentage completed? Will keep 100% but will indicate error?

glassez · 2018-05-30T05:07:02Z

@sledgehammer999, well, great job!
Although, it requires more changes.

We need to return the correct components loading sequence (startupTorrents should again become a part of the initialization of the session, and not be called from outside).
Torrent-manipulation actions (e.g. add torrent, delete torrent) should be unavailable (or maybe queued) until session is fully initialized (i.e. resumed all torrents).
We no need more two signals (torrentAdded/torrentNew) since it will indicate the same event.
Some GUI components should be disabled until session is fully initialized (or unavailable in some other way, e.g. we can show no GUI or show progress dialog).

glassez · 2018-05-30T05:08:49Z

src/gui/categoryfiltermodel.cpp

@@ -181,6 +181,12 @@ CategoryFilterModel::CategoryFilterModel(QObject *parent)
    connect(session, &Session::categoryRemoved, this, &CategoryFilterModel::categoryRemoved);
    connect(session, &Session::torrentCategoryChanged, this, &CategoryFilterModel::torrentCategoryChanged);
    connect(session, &Session::subcategoriesSupportChanged, this, &CategoryFilterModel::subcategoriesSupportChanged);
+    connect(session, &Session::finishedStartingUp, this,
+            [this, session]()


I would move it on the previous line in all similar cases of this PR.

Do you mean the lambda? Have it as a one liner?

On 2nd thought, you probably mean only the capture clause and parameter list.

On 2nd thought, you probably mean only the capture clause and parameter list.

Yes.

glassez · 2018-05-30T05:09:42Z

src/base/bittorrent/session.cpp

@@ -3808,11 +3786,14 @@ void Session::startUpTorrents()
        QByteArray data;
    } TorrentResumeData;

+    m_nativeSession->pause();


It would be better if we can create session in paused state.

glassez · 2018-05-30T05:11:45Z

src/base/bittorrent/session.cpp

@@ -4009,6 +3990,12 @@ void Session::handleAlert(libt::alert *a)
        case libt::add_torrent_alert::alert_type:
            handleAddTorrentAlert(static_cast<libt::add_torrent_alert*>(a));
            break;
+        case libt::torrent_added_alert::alert_type:
+            if (m_startupInProgress)


Please don't break the current style. Just add handleTorrentAddedAlert() method.

glassez · 2018-05-30T05:12:10Z

src/base/bittorrent/session.cpp

+            if (m_startupInProgress)
+                --m_startedCount;
+            if (m_startedCount == 0 && m_startupInProgress)
+                handleStartUpFinish();


handleStartUpFinished

glassez · 2018-05-30T05:18:56Z

src/base/bittorrent/session.h

@@ -538,6 +538,7 @@ namespace BitTorrent
        void subcategoriesSupportChanged();
        void tagAdded(const QString &tag);
        void tagRemoved(const QString &tag);
+        void finishedStartingUp();


startupFinished

Chocobo1 · 2018-05-30T06:19:27Z

src/gui/categoryfiltermodel.cpp

+    connect(session, &Session::finishedStartingUp, this,
+            [this, session]()
+    {
+        for (const auto &handle : session->torrents().values())


add qAsConst()?
or use iterator to loop, this avoids redundant traverse over the container, although I like the use of values() :P

Sure. I am still confused about those macros. Eventually I'll get the hang of them.

Chocobo1 · 2018-05-30T06:41:53Z

Torrent-manipulation actions (e.g. add torrent, delete torrent) should be unavailable (or maybe queued) until session is fully initialized (i.e. resumed all torrents).

I agree with the idea, making it unavailable seems simpler.

Some GUI components should be disabled until session is fully initialized (or unavailable in some other way, e.g. we can show no GUI or show progress dialog).

Since this is an interactive program, displaying no GUI would be a bad choice.
If feasible, I would choose to ready the mainwindow as early as possible (modern browsers startup time is impressive!) and let user know program is still initializing: could be some text in statusbar, or display the torrent jobs initially as "not yet loaded" and transition to other ready state (paused or downloading) when it is loaded.

glassez · 2018-05-30T06:55:24Z

or display the torrent jobs initially as "not yet loaded" and transition to other ready state (paused or downloading) when it is loaded.

Session doesn't emit signals for resumed torrents in changed code, only one startupFinished signal, and I really like this idea.

glassez · 2018-05-30T07:42:31Z

I agree with the idea, making it unavailable seems simpler.

We still need queuing at least for "add torrent" action since the application can start with torrents specified in command line. Of course, we can implement it outside the Session.

sledgehammer999 · 2018-05-31T02:30:23Z

I addressed some of the comments in my 3rd commit

I pushed 2 new commits. Now the initial loading of the torrents is done in a separate thread. However, for me, the mainwindow still takes quite a few seconds to draw its contents. The initial blank/white window is drawn quickly. The widgets are taking quite a few seconds.
The 2 new commits were written rather hastily with not enough testing. So rip them apart in the review.

Chocobo1 · 2018-05-31T04:59:23Z

Now the initial loading of the torrents is done in a separate thread. However, for me, the mainwindow still takes quite a few seconds to draw its contents.

Haven't tried this PR yet, (for you) does it still take seconds to draw the GUI even when there are no torrents?

wesleybl · 2018-05-31T14:28:57Z

@sledgehammer999 I would like to test this PR in Windows. If I do:

./configure
make && make install
qbittorrent

on Linux and copying the files to Windows will work?

glassez · 2018-08-11T16:51:50Z

@sledgehammer999, unfortunately I can't say that I approve this PR in its current form. It's more like a rough workaround (in terms of its design).
However, as I see it, it takes away from you the resources that are already missing for full maintainence of the project. So you are free to merge it if you think its purpose is achieved (from a user point of view, closing our eyes to its implementation) in the hope that we will polish and improve it in the future.
But I would make upcoming release ASAP (without this PR) and then merge it immediately so we would be able to test it thoroughly and fix/improve it before the next release.

P.S. Have you ever thought of having an additional release manager? This could help to make releases on time. It is a very hateful situation, when hotfixes "rot" in the master for several months while the bugtracker is filled with duplicate issues!

ghost · 2018-09-01T12:20:26Z

I have this problem too. Looks like add_torrent with flag_override_resume_data can help solve this problem.

glassez · 2018-12-29T18:08:26Z

@sledgehammer999, you still want to do something about it?

glassez · 2019-08-07T17:51:29Z

@sledgehammer999, #9007 (comment)

githubuserthatsright · 2019-08-12T15:22:04Z

Just so that I understand - this potential fix, or for that matter maybe any other, has been around for a bit over a year and more than just a few version updates and we've still got this as a problem?

glassez · 2019-08-12T15:47:14Z

Just so that I understand - this potential fix, or for that matter maybe any other, has been around for a bit over a year and more than just a few version updates and we've still got this as a problem?

The problems it was supposed to solve (at least most of them) are now solved in a different way.

githubuserthatsright · 2019-08-12T16:16:39Z

So moving or deleting data of a formerly completed torrrent will in no case any longer cause a redownload?

glassez · 2019-08-12T17:40:13Z

So moving or deleting data of a formerly completed torrrent will in no case any longer cause a redownload?

This should be fixed now.

glassez · 2020-06-03T13:15:01Z

The goal was achieved in a different way.

sledgehammer999 added the Core label May 30, 2018

sledgehammer999 added this to the 4.1.2 milestone May 30, 2018

sledgehammer999 requested review from glassez and Chocobo1 May 30, 2018 00:16

glassez reviewed May 30, 2018

View reviewed changes

Chocobo1 reviewed May 30, 2018

View reviewed changes

sledgehammer999 force-pushed the defer_loading branch from 6ea2f57 to bfe97ba Compare May 31, 2018 02:27

sledgehammer999 force-pushed the defer_loading branch from bfe97ba to fff13be Compare May 31, 2018 14:12

sledgehammer999 force-pushed the defer_loading branch from fff13be to 2ca6cd2 Compare May 31, 2018 14:32

sledgehammer999 modified the milestones: 4.1.2, 4.1.3 Aug 12, 2018

This was referenced Aug 21, 2018

Decrease probability of missing important alerts #9383

Merged

If you disconnect harddrive with torrent, it will be stopped at 0% #9389

Closed

ghost mentioned this pull request Sep 1, 2018

Keep paused torrent paused when restart. #9429

Closed

sledgehammer999 modified the milestones: 4.1.3, 4.1.4 Sep 18, 2018

thalieht mentioned this pull request Sep 19, 2018

Feature request #9534

Closed

sledgehammer999 modified the milestones: 4.1.4, 4.1.5 Nov 19, 2018

sledgehammer999 modified the milestones: 4.1.5, 4.1.6 Dec 24, 2018

sledgehammer999 modified the milestones: 4.1.6, 4.1.7 May 5, 2019

N46AN mentioned this pull request Jun 18, 2019

Stopped task restarts downloading after its file being moved elsewhere #10815

Closed

sledgehammer999 modified the milestones: 4.1.7, 4.1.8 Aug 4, 2019

sledgehammer999 modified the milestones: 4.1.8, 4.1.10 Oct 27, 2019

Seeker2 mentioned this pull request Jan 4, 2020

Torrents getting deleted #11334

Closed

glassez closed this Jun 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pause session till all torrents are fully started #9007

Pause session till all torrents are fully started #9007

sledgehammer999 commented May 30, 2018

wesleybl commented May 30, 2018

glassez commented May 30, 2018

glassez May 30, 2018

sledgehammer999 Jul 15, 2018

sledgehammer999 Jul 15, 2018

glassez Jul 16, 2018

glassez May 30, 2018

glassez May 30, 2018

glassez May 30, 2018

glassez May 30, 2018

Chocobo1 May 30, 2018

sledgehammer999 Jul 15, 2018

Chocobo1 commented May 30, 2018

glassez commented May 30, 2018

glassez commented May 30, 2018

sledgehammer999 commented May 31, 2018

Chocobo1 commented May 31, 2018

wesleybl commented May 31, 2018

glassez commented Aug 11, 2018

ghost commented Sep 1, 2018

glassez commented Dec 29, 2018

glassez commented Aug 7, 2019

githubuserthatsright commented Aug 12, 2019

glassez commented Aug 12, 2019

githubuserthatsright commented Aug 12, 2019

glassez commented Aug 12, 2019

glassez commented Jun 3, 2020

Pause session till all torrents are fully started #9007

Pause session till all torrents are fully started #9007

Conversation

sledgehammer999 commented May 30, 2018

wesleybl commented May 30, 2018

glassez commented May 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chocobo1 commented May 30, 2018

glassez commented May 30, 2018

glassez commented May 30, 2018

sledgehammer999 commented May 31, 2018

Chocobo1 commented May 31, 2018

wesleybl commented May 31, 2018

glassez commented Aug 11, 2018

ghost commented Sep 1, 2018

glassez commented Dec 29, 2018

glassez commented Aug 7, 2019

githubuserthatsright commented Aug 12, 2019

glassez commented Aug 12, 2019

githubuserthatsright commented Aug 12, 2019

glassez commented Aug 12, 2019

glassez commented Jun 3, 2020