From 7ce83599b42073bcf4ca8fd5906fd097061e93be Mon Sep 17 00:00:00 2001
From: Chocobo1 <Chocobo1@users.noreply.github.com>
Date: Sun, 26 Apr 2020 09:42:08 +0800
Subject: [PATCH] Improve behavior of deleting torrents in transfer list

Avoids accessing invalid pointers.
Closes #12584.
---
 src/gui/transferlistwidget.cpp | 44 +++++++++++++++++++---------------
 src/gui/transferlistwidget.h   |  1 +
 2 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/src/gui/transferlistwidget.cpp b/src/gui/transferlistwidget.cpp
index b4006b399..441b3151d 100644
--- a/src/gui/transferlistwidget.cpp
+++ b/src/gui/transferlistwidget.cpp
@@ -501,6 +501,17 @@ QVector<BitTorrent::TorrentHandle *> TransferListWidget::getSelectedTorrents() c
     return torrents;
 }
 
+QVector<BitTorrent::TorrentHandle *> TransferListWidget::getVisibleTorrents() const
+{
+    const int visibleTorrentsCount = m_sortFilterModel->rowCount();
+
+    QVector<BitTorrent::TorrentHandle *> torrents;
+    torrents.reserve(visibleTorrentsCount);
+    for (int i = 0; i < visibleTorrentsCount; ++i)
+        torrents << m_listModel->torrentHandle(mapToSource(m_sortFilterModel->index(i, 0)));
+    return torrents;
+}
+
 void TransferListWidget::setSelectedTorrentsLocation()
 {
     const QVector<BitTorrent::TorrentHandle *> torrents = getSelectedTorrents();
@@ -547,11 +558,8 @@ void TransferListWidget::forceStartSelectedTorrents()
 
 void TransferListWidget::startVisibleTorrents()
 {
-    for (int i = 0; i < m_sortFilterModel->rowCount(); ++i) {
-        BitTorrent::TorrentHandle *const torrent = m_listModel->torrentHandle(mapToSource(m_sortFilterModel->index(i, 0)));
-        if (torrent)
-            torrent->resume();
-    }
+    for (BitTorrent::TorrentHandle *const torrent : asConst(getVisibleTorrents()))
+        torrent->resume();
 }
 
 void TransferListWidget::pauseSelectedTorrents()
@@ -562,11 +570,8 @@ void TransferListWidget::pauseSelectedTorrents()
 
 void TransferListWidget::pauseVisibleTorrents()
 {
-    for (int i = 0; i < m_sortFilterModel->rowCount(); ++i) {
-        BitTorrent::TorrentHandle *const torrent = m_listModel->torrentHandle(mapToSource(m_sortFilterModel->index(i, 0)));
-        if (torrent)
-            torrent->pause();
-    }
+    for (BitTorrent::TorrentHandle *const torrent : asConst(getVisibleTorrents()))
+        torrent->pause();
 }
 
 void TransferListWidget::softDeleteSelectedTorrents()
@@ -589,9 +594,11 @@ void TransferListWidget::deleteSelectedTorrents(const bool deleteLocalFiles)
     if (Preferences::instance()->confirmTorrentDeletion()) {
         auto *dialog = new DeletionConfirmationDialog(this, torrents.size(), torrents[0]->name(), deleteLocalFiles);
         dialog->setAttribute(Qt::WA_DeleteOnClose);
-        connect(dialog, &DeletionConfirmationDialog::accepted, this, [dialog, torrents]()
+        connect(dialog, &DeletionConfirmationDialog::accepted, this, [this, dialog]()
         {
-            removeTorrents(torrents, dialog->isDeleteFileSelected());
+            // Some torrents might be removed when waiting for user input, so refetch the torrent list
+            // NOTE: this will only work when dialog is modal
+            removeTorrents(getSelectedTorrents(), dialog->isDeleteFileSelected());
         });
         dialog->open();
     }
@@ -602,18 +609,17 @@ void TransferListWidget::deleteSelectedTorrents(const bool deleteLocalFiles)
 
 void TransferListWidget::deleteVisibleTorrents()
 {
-    if (m_sortFilterModel->rowCount() <= 0) return;
-
-    QVector<BitTorrent::TorrentHandle *> torrents;
-    for (int i = 0; i < m_sortFilterModel->rowCount(); ++i)
-        torrents << m_listModel->torrentHandle(mapToSource(m_sortFilterModel->index(i, 0)));
+    const QVector<BitTorrent::TorrentHandle *> torrents = getVisibleTorrents();
+    if (torrents.empty()) return;
 
     if (Preferences::instance()->confirmTorrentDeletion()) {
         auto *dialog = new DeletionConfirmationDialog(this, torrents.size(), torrents[0]->name(), false);
         dialog->setAttribute(Qt::WA_DeleteOnClose);
-        connect(dialog, &DeletionConfirmationDialog::accepted, this, [dialog, torrents]()
+        connect(dialog, &DeletionConfirmationDialog::accepted, this, [this, dialog]()
         {
-            removeTorrents(torrents, dialog->isDeleteFileSelected());
+            // Some torrents might be removed when waiting for user input, so refetch the torrent list
+            // NOTE: this will only work when dialog is modal
+            removeTorrents(getVisibleTorrents(), dialog->isDeleteFileSelected());
         });
         dialog->open();
     }
diff --git a/src/gui/transferlistwidget.h b/src/gui/transferlistwidget.h
index 889ae0d1c..166c8fcd9 100644
--- a/src/gui/transferlistwidget.h
+++ b/src/gui/transferlistwidget.h
@@ -141,6 +141,7 @@ private:
     void confirmRemoveAllTagsForSelection();
     QStringList askTagsForSelection(const QString &dialogTitle);
     void applyToSelectedTorrents(const std::function<void (BitTorrent::TorrentHandle *const)> &fn);
+    QVector<BitTorrent::TorrentHandle *> getVisibleTorrents() const;
 
     // supposed to be used with qss only
     QColor unknownStateForeground() const;