Ensure thread is stopped before deleting QThread

PR #18037.

src/base/CMakeLists.txt

@@ -101,6 +101,7 @@ add_library(qbt_base STATIC
     utils/password.h
     utils/random.h
     utils/string.h
+    utils/thread.h
     utils/version.h
     version.h
 
@@ -183,6 +184,7 @@ add_library(qbt_base STATIC
     utils/password.cpp
     utils/random.cpp
     utils/string.cpp
+    utils/thread.cpp
 )
 
 target_link_libraries(qbt_base

src/base/base.pri

@@ -101,6 +101,7 @@ HEADERS += \
     $$PWD/utils/password.h \
     $$PWD/utils/random.h \
     $$PWD/utils/string.h \
+    $$PWD/utils/thread.h \
     $$PWD/utils/version.h \
     $$PWD/version.h
 
@@ -182,4 +183,5 @@ SOURCES += \
     $$PWD/utils/net.cpp \
     $$PWD/utils/password.cpp \
     $$PWD/utils/random.cpp \
-    $$PWD/utils/string.cpp
+    $$PWD/utils/string.cpp \
+    $$PWD/utils/thread.cpp

src/base/bittorrent/bencoderesumedatastorage.cpp

@@ -1,6 +1,6 @@
 /*
  * Bittorrent Client using Qt and libtorrent.
- * Copyright (C) 2015, 2018  Vladimir Golovnev <glassez@yandex.ru>
+ * Copyright (C) 2015-2022  Vladimir Golovnev <glassez@yandex.ru>
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -91,7 +91,7 @@ namespace
 
 BitTorrent::BencodeResumeDataStorage::BencodeResumeDataStorage(const Path &path, QObject *parent)
     : ResumeDataStorage(path, parent)
-    , m_ioThread {new QThread(this)}
+    , m_ioThread {new QThread}
     , m_asyncWorker {new Worker(path)}
 {
     Q_ASSERT(path.isAbsolute());
@@ -117,17 +117,11 @@ BitTorrent::BencodeResumeDataStorage::BencodeResumeDataStorage(const Path &path,
 
     qDebug() << "Registered torrents count: " << m_registeredTorrents.size();
 
-    m_asyncWorker->moveToThread(m_ioThread);
+    m_asyncWorker->moveToThread(m_ioThread.get());
-    connect(m_ioThread, &QThread::finished, m_asyncWorker, &QObject::deleteLater);
+    connect(m_ioThread.get(), &QThread::finished, m_asyncWorker, &QObject::deleteLater);
     m_ioThread->start();
 }
 
-BitTorrent::BencodeResumeDataStorage::~BencodeResumeDataStorage()
-{
-    m_ioThread->quit();
-    m_ioThread->wait();
-}
-
 QVector<BitTorrent::TorrentID> BitTorrent::BencodeResumeDataStorage::registeredTorrents() const
 {
     return m_registeredTorrents;

src/base/bittorrent/bencoderesumedatastorage.h

@@ -1,6 +1,6 @@
 /*
  * Bittorrent Client using Qt and libtorrent.
- * Copyright (C) 2015, 2018  Vladimir Golovnev <glassez@yandex.ru>
+ * Copyright (C) 2015-2022  Vladimir Golovnev <glassez@yandex.ru>
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -32,6 +32,8 @@
 #include <QVector>
 
 #include "base/pathfwd.h"
+#include "base/utils/thread.h"
+
 #include "resumedatastorage.h"
 
 class QByteArray;
@@ -46,7 +48,6 @@ namespace BitTorrent
 
     public:
         explicit BencodeResumeDataStorage(const Path &path, QObject *parent = nullptr);
-        ~BencodeResumeDataStorage() override;
 
         QVector<TorrentID> registeredTorrents() const override;
         LoadResumeDataResult load(const TorrentID &id) const override;
@@ -60,7 +61,7 @@ namespace BitTorrent
         LoadResumeDataResult loadTorrentResumeData(const QByteArray &data, const QByteArray &metadata) const;
 
         QVector<TorrentID> m_registeredTorrents;
-        QThread *m_ioThread = nullptr;
+        Utils::Thread::UniquePtr m_ioThread;
 
         class Worker;
         Worker *m_asyncWorker = nullptr;

src/base/bittorrent/dbresumedatastorage.cpp

@@ -1,6 +1,6 @@
 /*
  * Bittorrent Client using Qt and libtorrent.
- * Copyright (C) 2021  Vladimir Golovnev <glassez@yandex.ru>
+ * Copyright (C) 2021-2022  Vladimir Golovnev <glassez@yandex.ru>
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -256,7 +256,7 @@ namespace BitTorrent
 
 BitTorrent::DBResumeDataStorage::DBResumeDataStorage(const Path &dbPath, QObject *parent)
     : ResumeDataStorage(dbPath, parent)
-    , m_ioThread {new QThread(this)}
+    , m_ioThread {new QThread}
 {
     const bool needCreateDB = !dbPath.exists();
 
@@ -277,8 +277,8 @@ BitTorrent::DBResumeDataStorage::DBResumeDataStorage(const Path &dbPath, QObject
     }
 
     m_asyncWorker = new Worker(dbPath, u"ResumeDataStorageWorker"_qs, m_dbLock);
-    m_asyncWorker->moveToThread(m_ioThread);
+    m_asyncWorker->moveToThread(m_ioThread.get());
-    connect(m_ioThread, &QThread::finished, m_asyncWorker, &QObject::deleteLater);
+    connect(m_ioThread.get(), &QThread::finished, m_asyncWorker, &QObject::deleteLater);
     m_ioThread->start();
 
     RuntimeError *errPtr = nullptr;
@@ -302,9 +302,6 @@ BitTorrent::DBResumeDataStorage::~DBResumeDataStorage()
 {
     QMetaObject::invokeMethod(m_asyncWorker, &Worker::closeDatabase);
     QSqlDatabase::removeDatabase(DB_CONNECTION_NAME);
-
-    m_ioThread->quit();
-    m_ioThread->wait();
 }
 
 QVector<BitTorrent::TorrentID> BitTorrent::DBResumeDataStorage::registeredTorrents() const

src/base/bittorrent/dbresumedatastorage.h

@@ -1,6 +1,6 @@
 /*
  * Bittorrent Client using Qt and libtorrent.
- * Copyright (C) 2021  Vladimir Golovnev <glassez@yandex.ru>
+ * Copyright (C) 2021-2022  Vladimir Golovnev <glassez@yandex.ru>
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -31,6 +31,7 @@
 #include <QReadWriteLock>
 
 #include "base/pathfwd.h"
+#include "base/utils/thread.h"
 #include "resumedatastorage.h"
 
 class QThread;
@@ -60,7 +61,7 @@ namespace BitTorrent
         void updateDB(int fromVersion) const;
         void enableWALMode() const;
 
-        QThread *m_ioThread = nullptr;
+        Utils::Thread::UniquePtr m_ioThread;
 
         class Worker;
         Worker *m_asyncWorker = nullptr;

src/base/bittorrent/sessionimpl.cpp

@@ -529,7 +529,7 @@ SessionImpl::SessionImpl(QObject *parent)
     , m_resumeDataStorageType(BITTORRENT_SESSION_KEY(u"ResumeDataStorageType"_qs), ResumeDataStorageType::Legacy)
     , m_seedingLimitTimer {new QTimer(this)}
     , m_resumeDataTimer {new QTimer(this)}
-    , m_ioThread {new QThread(this)}
+    , m_ioThread {new QThread}
     , m_asyncWorker {new QThreadPool(this)}
     , m_recentErroredTorrentsTimer {new QTimer(this)}
 #if (QT_VERSION < QT_VERSION_CHECK(6, 0, 0))
@@ -584,8 +584,8 @@ SessionImpl::SessionImpl(QObject *parent)
 #endif
 
     m_fileSearcher = new FileSearcher;
-    m_fileSearcher->moveToThread(m_ioThread);
+    m_fileSearcher->moveToThread(m_ioThread.get());
-    connect(m_ioThread, &QThread::finished, m_fileSearcher, &QObject::deleteLater);
+    connect(m_ioThread.get(), &QThread::finished, m_fileSearcher, &QObject::deleteLater);
     connect(m_fileSearcher, &FileSearcher::searchFinished, this, &SessionImpl::fileSearchFinished);
 
     m_ioThread->start();
@@ -619,9 +619,6 @@ SessionImpl::~SessionImpl()
 
     saveStatistics();
 
-    m_asyncWorker->clear();
-    m_asyncWorker->waitForDone();
-
     // We must delete FilterParserThread
     // before we delete lt::session
     delete m_filterParser;
@@ -630,11 +627,11 @@ SessionImpl::~SessionImpl()
     // we delete lt::session
     delete Net::PortForwarder::instance();
 
-    qDebug("Deleting the session");
+    m_asyncWorker->clear();
-    delete m_nativeSession;
+    m_asyncWorker->waitForDone();
 
-    m_ioThread->quit();
+    qDebug("Deleting libtorrent session...");
-    m_ioThread->wait();
+    delete m_nativeSession;
 }
 
 bool SessionImpl::isDHTEnabled() const

src/base/bittorrent/sessionimpl.h

@@ -47,6 +47,7 @@
 #include "base/path.h"
 #include "base/settingvalue.h"
 #include "base/types.h"
+#include "base/utils/thread.h"
 #include "addtorrentparams.h"
 #include "cachestatus.h"
 #include "categoryoptions.h"
@@ -709,7 +710,7 @@ namespace BitTorrent
         // Tracker
         QPointer<Tracker> m_tracker;
 
-        QThread *m_ioThread = nullptr;
+        Utils::Thread::UniquePtr m_ioThread;
         QThreadPool *m_asyncWorker = nullptr;
         ResumeDataStorage *m_resumeDataStorage = nullptr;
         FileSearcher *m_fileSearcher = nullptr;

src/base/rss/rss_autodownloader.cpp

@@ -102,15 +102,15 @@ AutoDownloader::AutoDownloader()
     , m_storeSmartEpisodeFilter(u"RSS/AutoDownloader/SmartEpisodeFilter"_qs)
     , m_storeDownloadRepacks(u"RSS/AutoDownloader/DownloadRepacks"_qs)
     , m_processingTimer(new QTimer(this))
-    , m_ioThread(new QThread(this))
+    , m_ioThread(new QThread)
 {
     Q_ASSERT(!m_instance); // only one instance is allowed
     m_instance = this;
 
     m_fileStorage = new AsyncFileStorage(specialFolderLocation(SpecialFolder::Config) / Path(CONF_FOLDER_NAME));
 
-    m_fileStorage->moveToThread(m_ioThread);
+    m_fileStorage->moveToThread(m_ioThread.get());
-    connect(m_ioThread, &QThread::finished, m_fileStorage, &AsyncFileStorage::deleteLater);
+    connect(m_ioThread.get(), &QThread::finished, m_fileStorage, &AsyncFileStorage::deleteLater);
     connect(m_fileStorage, &AsyncFileStorage::failed, [](const Path &fileName, const QString &errorString)
     {
         LogMsg(tr("Couldn't save RSS AutoDownloader data in %1. Error: %2")
@@ -155,9 +155,6 @@ AutoDownloader::AutoDownloader()
 AutoDownloader::~AutoDownloader()
 {
     store();
-
-    m_ioThread->quit();
-    m_ioThread->wait();
 }
 
 AutoDownloader *AutoDownloader::instance()

src/base/rss/rss_autodownloader.h

@@ -38,6 +38,7 @@
 
 #include "base/exceptions.h"
 #include "base/settingvalue.h"
+#include "base/utils/thread.h"
 
 class QThread;
 class QTimer;
@@ -137,7 +138,7 @@ namespace RSS
         SettingValue<bool> m_storeDownloadRepacks;
 
         QTimer *m_processingTimer = nullptr;
-        QThread *m_ioThread = nullptr;
+        Utils::Thread::UniquePtr m_ioThread;
         AsyncFileStorage *m_fileStorage = nullptr;
         QHash<QString, AutoDownloadRule> m_rules;
         QList<QSharedPointer<ProcessingJob>> m_processingQueue;

src/base/rss/rss_session.cpp

@@ -62,14 +62,14 @@ Session::Session()
     : m_storeProcessingEnabled(u"RSS/Session/EnableProcessing"_qs)
     , m_storeRefreshInterval(u"RSS/Session/RefreshInterval"_qs, 30)
     , m_storeMaxArticlesPerFeed(u"RSS/Session/MaxArticlesPerFeed"_qs, 50)
-    , m_workingThread(new QThread(this))
+    , m_workingThread(new QThread)
 {
     Q_ASSERT(!m_instance); // only one instance is allowed
     m_instance = this;
 
     m_confFileStorage = new AsyncFileStorage(specialFolderLocation(SpecialFolder::Config) / Path(CONF_FOLDER_NAME));
-    m_confFileStorage->moveToThread(m_workingThread);
+    m_confFileStorage->moveToThread(m_workingThread.get());
-    connect(m_workingThread, &QThread::finished, m_confFileStorage, &AsyncFileStorage::deleteLater);
+    connect(m_workingThread.get(), &QThread::finished, m_confFileStorage, &AsyncFileStorage::deleteLater);
     connect(m_confFileStorage, &AsyncFileStorage::failed, [](const Path &fileName, const QString &errorString)
     {
         LogMsg(tr("Couldn't save RSS session configuration. File: \"%1\". Error: \"%2\"")
@@ -77,8 +77,8 @@ Session::Session()
     });
 
     m_dataFileStorage = new AsyncFileStorage(specialFolderLocation(SpecialFolder::Data) / Path(DATA_FOLDER_NAME));
-    m_dataFileStorage->moveToThread(m_workingThread);
+    m_dataFileStorage->moveToThread(m_workingThread.get());
-    connect(m_workingThread, &QThread::finished, m_dataFileStorage, &AsyncFileStorage::deleteLater);
+    connect(m_workingThread.get(), &QThread::finished, m_dataFileStorage, &AsyncFileStorage::deleteLater);
     connect(m_dataFileStorage, &AsyncFileStorage::failed, [](const Path &fileName, const QString &errorString)
     {
         LogMsg(tr("Couldn't save RSS session data. File: \"%1\". Error: \"%2\"")
@@ -126,11 +126,6 @@ Session::~Session()
     //store();
     delete m_itemsByPath[u""_qs]; // deleting root folder
 
-    // some items may add I/O operation at destruction
-    // stop working thread after deleting root folder
-    m_workingThread->quit();
-    m_workingThread->wait();
-
     qDebug() << "RSS Session deleted.";
 }
 
@@ -487,7 +482,7 @@ void Session::setRefreshInterval(const int refreshInterval)
 
 QThread *Session::workingThread() const
 {
-    return m_workingThread;
+    return m_workingThread.get();
 }
 
 void Session::handleItemAboutToBeDestroyed(Item *item)

src/base/rss/rss_session.h

@@ -75,6 +75,7 @@
 
 #include "base/3rdparty/expected.hpp"
 #include "base/settingvalue.h"
+#include "base/utils/thread.h"
 
 class QThread;
 
@@ -158,7 +159,7 @@ namespace RSS
         CachedSettingValue<bool> m_storeProcessingEnabled;
         CachedSettingValue<int> m_storeRefreshInterval;
         CachedSettingValue<int> m_storeMaxArticlesPerFeed;
-        QThread *m_workingThread = nullptr;
+        Utils::Thread::UniquePtr m_workingThread;
         AsyncFileStorage *m_confFileStorage = nullptr;
         AsyncFileStorage *m_dataFileStorage = nullptr;
         QTimer m_refreshTimer;

src/base/torrentfileswatcher.cpp

@@ -254,7 +254,7 @@ TorrentFilesWatcher *TorrentFilesWatcher::instance()
 
 TorrentFilesWatcher::TorrentFilesWatcher(QObject *parent)
     : QObject {parent}
-    , m_ioThread {new QThread(this)}
+    , m_ioThread {new QThread}
 {
     const auto *btSession = BitTorrent::Session::instance();
     if (btSession->isRestored())
@@ -265,12 +265,6 @@ TorrentFilesWatcher::TorrentFilesWatcher(QObject *parent)
     load();
 }
 
-TorrentFilesWatcher::~TorrentFilesWatcher()
-{
-    m_ioThread->quit();
-    m_ioThread->wait();
-}
-
 void TorrentFilesWatcher::initWorker()
 {
     Q_ASSERT(!m_asyncWorker);
@@ -280,8 +274,8 @@ void TorrentFilesWatcher::initWorker()
     connect(m_asyncWorker, &TorrentFilesWatcher::Worker::magnetFound, this, &TorrentFilesWatcher::onMagnetFound);
     connect(m_asyncWorker, &TorrentFilesWatcher::Worker::torrentFound, this, &TorrentFilesWatcher::onTorrentFound);
 
-    m_asyncWorker->moveToThread(m_ioThread);
+    m_asyncWorker->moveToThread(m_ioThread.get());
-    connect(m_ioThread, &QThread::finished, m_asyncWorker, &QObject::deleteLater);
+    connect(m_ioThread.get(), &QThread::finished, m_asyncWorker, &QObject::deleteLater);
     m_ioThread->start();
 
     for (auto it = m_watchedFolders.cbegin(); it != m_watchedFolders.cend(); ++it)

src/base/torrentfileswatcher.h

@@ -33,6 +33,7 @@
 
 #include "base/bittorrent/addtorrentparams.h"
 #include "base/path.h"
+#include "base/utils/thread.h"
 
 class QThread;
 
@@ -76,7 +77,6 @@ private slots:
 
 private:
     explicit TorrentFilesWatcher(QObject *parent = nullptr);
-    ~TorrentFilesWatcher() override;
 
     void initWorker();
     void load();
@@ -89,7 +89,7 @@ private:
 
     QHash<Path, WatchedFolderOptions> m_watchedFolders;
 
-    QThread *m_ioThread = nullptr;
+    Utils::Thread::UniquePtr m_ioThread;
 
     class Worker;
     Worker *m_asyncWorker = nullptr;

src/base/utils/thread.cpp

@@ -0,0 +1,38 @@
+/*
+ * Bittorrent Client using Qt and libtorrent.
+ * Copyright (C) 2022  Vladimir Golovnev <glassez@yandex.ru>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ * In addition, as a special exception, the copyright holders give permission to
+ * link this program with the OpenSSL project's "OpenSSL" library (or with
+ * modified versions of it that use the same license as the "OpenSSL" library),
+ * and distribute the linked executables. You must obey the GNU General Public
+ * License in all respects for all of the code used other than "OpenSSL".  If you
+ * modify file(s), you may extend this exception to your version of the file(s),
+ * but you are not obligated to do so. If you do not wish to do so, delete this
+ * exception statement from your version.
+ */
+
+#include "thread.h"
+
+#include <QThread>
+
+void Utils::Thread::GracefulDeleter::operator()(QThread *thread) const
+{
+    thread->quit();
+    thread->wait();
+    delete thread;
+}

src/base/utils/thread.h

@@ -0,0 +1,43 @@
+/*
+ * Bittorrent Client using Qt and libtorrent.
+ * Copyright (C) 2022  Vladimir Golovnev <glassez@yandex.ru>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ * In addition, as a special exception, the copyright holders give permission to
+ * link this program with the OpenSSL project's "OpenSSL" library (or with
+ * modified versions of it that use the same license as the "OpenSSL" library),
+ * and distribute the linked executables. You must obey the GNU General Public
+ * License in all respects for all of the code used other than "OpenSSL".  If you
+ * modify file(s), you may extend this exception to your version of the file(s),
+ * but you are not obligated to do so. If you do not wish to do so, delete this
+ * exception statement from your version.
+ */
+
+#pragma once
+
+#include <memory>
+
+class QThread;
+
+namespace Utils::Thread
+{
+    struct GracefulDeleter
+    {
+        void operator()(QThread *thread) const;
+    };
+
+    using UniquePtr = std::unique_ptr<QThread, GracefulDeleter>;
+}

src/webui/api/synccontroller.cpp

@@ -372,12 +372,12 @@ namespace
 
 SyncController::SyncController(IApplication *app, QObject *parent)
     : APIController(app, parent)
+    , m_freeDiskSpaceChecker {new FreeDiskSpaceChecker}
+    , m_freeDiskSpaceThread {new QThread}
 {
-    m_freeDiskSpaceThread = new QThread(this);
+    m_freeDiskSpaceChecker->moveToThread(m_freeDiskSpaceThread.get());
-    m_freeDiskSpaceChecker = new FreeDiskSpaceChecker();
-    m_freeDiskSpaceChecker->moveToThread(m_freeDiskSpaceThread);
 
-    connect(m_freeDiskSpaceThread, &QThread::finished, m_freeDiskSpaceChecker, &QObject::deleteLater);
+    connect(m_freeDiskSpaceThread.get(), &QThread::finished, m_freeDiskSpaceChecker, &QObject::deleteLater);
     connect(m_freeDiskSpaceChecker, &FreeDiskSpaceChecker::checked, this, &SyncController::freeDiskSpaceSizeUpdated);
 
     m_freeDiskSpaceThread->start();
@@ -385,12 +385,6 @@ SyncController::SyncController(IApplication *app, QObject *parent)
     m_freeDiskSpaceElapsedTimer.start();
 }
 
-SyncController::~SyncController()
-{
-    m_freeDiskSpaceThread->quit();
-    m_freeDiskSpaceThread->wait();
-}
-
 // The function returns the changed data from the server to synchronize with the web client.
 // Return value is map in JSON format.
 // Map contain the key:

src/webui/api/synccontroller.h

@@ -31,6 +31,7 @@
 #include <QElapsedTimer>
 #include <QVariantMap>
 
+#include "base/utils/thread.h"
 #include "apicontroller.h"
 
 class QThread;
@@ -46,7 +47,6 @@ public:
     using APIController::APIController;
 
     explicit SyncController(IApplication *app, QObject *parent = nullptr);
-    ~SyncController() override;
 
 private slots:
     void maindataAction();
@@ -59,7 +59,7 @@ private:
 
     qint64 m_freeDiskSpace = 0;
     FreeDiskSpaceChecker *m_freeDiskSpaceChecker = nullptr;
-    QThread *m_freeDiskSpaceThread = nullptr;
+    Utils::Thread::UniquePtr m_freeDiskSpaceThread;
     QElapsedTimer m_freeDiskSpaceElapsedTimer;
 
     QVariantMap m_lastMaindataResponse;