Safely close database connections and delete backends

Also fix NewClosure leak caused by disconnected object signals

src/collection/collection.cpp

@@ -21,6 +21,7 @@
 #include "config.h"
 
 #include <stdbool.h>
+#include <unistd.h>
 
 #include <QObject>
 #include <QThread>
@@ -50,7 +51,10 @@ SCollection::SCollection(Application *app, QObject *parent)
       backend_(nullptr),
       model_(nullptr),
       watcher_(nullptr),
-      watcher_thread_(nullptr) {
+      watcher_thread_(nullptr),
+      original_thread_(nullptr) {
+
+  original_thread_ = thread();
 
   backend_ = new CollectionBackend();
   backend()->moveToThread(app->database()->thread());
@@ -64,11 +68,17 @@ SCollection::SCollection(Application *app, QObject *parent)
 }
 
 SCollection::~SCollection() {
-  watcher_->Stop();
-  watcher_->deleteLater();
-  watcher_thread_->exit();
-  watcher_thread_->wait(5000 /* five seconds */);
+
+  if (watcher_) {
+    watcher_->Stop();
+    watcher_->deleteLater();
+  }
+  if (watcher_thread_) {
+    watcher_thread_->exit();
+    watcher_thread_->wait(5000 /* five seconds */);
+  }
   backend_->deleteLater();
+
 }
 
 void SCollection::Init() {
@@ -98,6 +108,29 @@ void SCollection::Init() {
 
   // This will start the watcher checking for updates
   backend_->LoadDirectoriesAsync();
+
+}
+
+void SCollection::Exit() {
+
+  wait_for_exit_ << backend_ << watcher_;
+
+  disconnect(backend_, 0, watcher_, 0);
+  disconnect(watcher_, 0, backend_, 0);
+
+  connect(backend_, SIGNAL(ExitFinished()), this, SLOT(ExitReceived()));
+  connect(watcher_, SIGNAL(ExitFinished()), this, SLOT(ExitReceived()));
+  backend_->ExitAsync();
+  watcher_->ExitAsync();
+
+}
+
+void SCollection::ExitReceived() {
+
+  disconnect(sender(), 0, this, 0);
+  wait_for_exit_.removeAll(sender());
+  if (wait_for_exit_.isEmpty()) emit ExitFinished();
+
 }
 
 void SCollection::IncrementalScan() { watcher_->IncrementalScanAsync(); }

src/collection/collection.h

@@ -49,6 +49,7 @@ class SCollection : public QObject {
   static const char *kFtsTable;
 
   void Init();
+  void Exit();
 
   CollectionBackend *backend() const { return backend_; }
   CollectionModel *model() const { return model_; }
@@ -70,12 +71,17 @@ class SCollection : public QObject {
   void Rescan(const SongList &songs);
 
  private slots:
+  void ExitReceived();
+
   void IncrementalScan();
 
   void CurrentSongChanged(const Song &song);
   void SongsStatisticsChanged(const SongList& songs);
   void Stopped();
 
+ signals:
+  void ExitFinished();
+
  private:
   Application *app_;
   CollectionBackend *backend_;
@@ -83,9 +89,12 @@ class SCollection : public QObject {
 
   CollectionWatcher *watcher_;
   Thread *watcher_thread_;
+  QThread *original_thread_;
 
   // DB schema versions which should trigger a full collection rescan (each of those with a short reason why).
   QHash<int, QString> full_rescan_revisions_;
+
+  QList<QObject*> wait_for_exit_;
 };
 
 #endif

src/collection/collectionbackend.cpp

@@ -20,8 +20,12 @@
 
 #include "config.h"
 
+#include <assert.h>
+
 #include <QtGlobal>
 #include <QObject>
+#include <QApplication>
+#include <QThread>
 #include <QMutex>
 #include <QSet>
 #include <QMap>
@@ -52,7 +56,14 @@ const char *CollectionBackend::kSettingsGroup = "Collection";
 
 CollectionBackend::CollectionBackend(QObject *parent) :
     CollectionBackendInterface(parent),
-    db_(nullptr) {}
+    db_(nullptr),
+    original_thread_(nullptr) {
+
+  original_thread_ = thread();
+
+}
+
+CollectionBackend::~CollectionBackend() {}
 
 void CollectionBackend::Init(Database *db, const Song::Source source, const QString &songs_table, const QString &dirs_table, const QString &subdirs_table, const QString &fts_table) {
   db_ = db;
@@ -63,6 +74,29 @@ void CollectionBackend::Init(Database *db, const Song::Source source, const QStr
   fts_table_ = fts_table;
 }
 
+void CollectionBackend::Close() {
+
+  if (db_) {
+    QMutexLocker l(db_->Mutex());
+    db_->Close();
+  }
+
+}
+
+void CollectionBackend::ExitAsync() {
+  metaObject()->invokeMethod(this, "Exit", Qt::QueuedConnection);
+}
+
+void CollectionBackend::Exit() {
+
+  assert(QThread::currentThread() == thread());
+
+  Close();
+  moveToThread(original_thread_);
+  emit ExitFinished();
+
+}
+
 void CollectionBackend::LoadDirectoriesAsync() {
   metaObject()->invokeMethod(this, "LoadDirectories", Qt::QueuedConnection);
 }

src/collection/collectionbackend.h

@@ -41,6 +41,7 @@
 #include "collectionquery.h"
 #include "directory.h"
 
+class QThread;
 class Database;
 
 class CollectionBackendInterface : public QObject {
@@ -123,7 +124,12 @@ class CollectionBackend : public CollectionBackendInterface {
   static const char *kSettingsGroup;
 
   Q_INVOKABLE CollectionBackend(QObject *parent = nullptr);
+  ~CollectionBackend();
+
   void Init(Database *db, const Song::Source source, const QString &songs_table, const QString &dirs_table, const QString &subdirs_table, const QString &fts_table);
+  void Close();
+
+  void ExitAsync();
 
   Database *db() const { return db_; }
 
@@ -183,6 +189,7 @@ class CollectionBackend : public CollectionBackendInterface {
   SongList GetSongsBySongId(const QStringList &song_ids);
 
  public slots:
+  void Exit();
   void LoadDirectories();
   void UpdateTotalSongCount();
   void UpdateTotalArtistCount();
@@ -200,7 +207,7 @@ class CollectionBackend : public CollectionBackendInterface {
   void ResetStatistics(int id);
   void SongPathChanged(const Song &song, const QFileInfo &new_file);
 
-signals:
+ signals:
   void DirectoryDiscovered(const Directory &dir, const SubdirectoryList &subdirs);
   void DirectoryDeleted(const Directory &dir);
 
@@ -214,6 +221,8 @@ signals:
   void TotalArtistCountUpdated(int total);
   void TotalAlbumCountUpdated(int total);
 
+  void ExitFinished();
+
  private:
   struct CompilationInfo {
     CompilationInfo() : has_compilation_detected(false), has_not_compilation_detected(false) {}
@@ -243,6 +252,7 @@ signals:
   QString dirs_table_;
   QString subdirs_table_;
   QString fts_table_;
+  QThread *original_thread_;
 
 };
 

src/collection/collectionmodel.cpp

@@ -129,7 +129,10 @@ CollectionModel::CollectionModel(CollectionBackend *backend, Application *app, Q
 
 }
 
-CollectionModel::~CollectionModel() { delete root_; }
+CollectionModel::~CollectionModel() {
+  backend_->Close();
+  delete root_;
+}
 
 void CollectionModel::set_pretty_covers(bool use_pretty_covers) {
 
@@ -757,6 +760,11 @@ CollectionModel::QueryResult CollectionModel::RunQuery(CollectionItem *parent) {
   while (q.Next()) {
     result.rows << SqlRow(q);
   }
+
+  if (QThread::currentThread() != thread() && QThread::currentThread() != backend_->thread()) {
+    backend_->Close();
+  }
+
   return result;
 
 }

src/collection/collectionwatcher.cpp

@@ -20,7 +20,11 @@
 
 #include "config.h"
 
+#include <assert.h>
+
 #include <QObject>
+#include <QApplication>
+#include <QThread>
 #include <QIODevice>
 #include <QDir>
 #include <QDirIterator>
@@ -79,7 +83,10 @@ CollectionWatcher::CollectionWatcher(Song::Source source, QObject *parent)
       rescan_timer_(new QTimer(this)),
       rescan_paused_(false),
       total_watches_(0),
-      cue_parser_(new CueParser(backend_, this)) {
+      cue_parser_(new CueParser(backend_, this)),
+      original_thread_(nullptr) {
+
+  original_thread_ = thread();
 
   rescan_timer_->setInterval(1000);
   rescan_timer_->setSingleShot(true);
@@ -93,6 +100,21 @@ CollectionWatcher::CollectionWatcher(Song::Source source, QObject *parent)
   connect(rescan_timer_, SIGNAL(timeout()), SLOT(RescanPathsNow()));
 }
 
+void CollectionWatcher::ExitAsync() {
+  metaObject()->invokeMethod(this, "Exit", Qt::QueuedConnection);
+}
+
+void CollectionWatcher::Exit() {
+
+  assert(QThread::currentThread() == thread());
+
+  Stop();
+  if (backend_) backend_->Close();
+  moveToThread(original_thread_);
+  emit ExitFinished();
+
+}
+
 CollectionWatcher::ScanTransaction::ScanTransaction(CollectionWatcher *watcher, const int dir, const bool incremental, const bool ignores_mtime, const bool prevent_delete)
     : progress_(0),
       progress_max_(0),

src/collection/collectionwatcher.h

@@ -60,6 +60,8 @@ class CollectionWatcher : public QObject {
 
   void Stop() { stop_requested_ = true; }
 
+  void ExitAsync();
+
 signals:
   void NewOrUpdatedSongs(const SongList &songs);
   void SongsMTimeUpdated(const SongList &songs);
@@ -68,6 +70,7 @@ signals:
   void SubdirsDiscovered(const SubdirectoryList &subdirs);
   void SubdirsMTimeUpdated(const SubdirectoryList &subdirs);
   void CompilationsNeedUpdating();
+  void ExitFinished();
 
   void ScanStarted(int task_id);
 
@@ -142,6 +145,7 @@ signals:
   };
 
  private slots:
+  void Exit();
   void DirectoryChanged(const QString &path);
   void IncrementalScanNow();
   void FullScanNow();
@@ -204,6 +208,8 @@ signals:
 
   SongList song_rescan_queue_; // Set by ui thread
 
+  QThread *original_thread_;
+
 };
 
 inline QString CollectionWatcher::NoExtensionPart(const QString& fileName) {