Refactoring: remove BackgroundThread

src/CMakeLists.txt

@@ -74,7 +74,6 @@ set(SOURCES
   core/appearance.cpp
   core/application.cpp
   core/backgroundstreams.cpp
-  core/backgroundthread.cpp
   core/commandlineoptions.cpp
   core/crashreporting.cpp
   core/database.cpp
@@ -344,7 +343,6 @@ set(HEADERS
 
   core/application.h
   core/backgroundstreams.h
-  core/backgroundthread.h
   core/crashreporting.h
   core/database.h
   core/deletefiles.h

src/core/backgroundthread.cpp

@@ -1,64 +0,0 @@
-/* This file is part of Clementine.
-   Copyright 2010, David Sansome <me@davidsansome.com>
-
-   Clementine 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 3 of the License, or
-   (at your option) any later version.
-
-   Clementine 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 Clementine.  If not, see <http://www.gnu.org/licenses/>.
-*/
-
-#include "backgroundthread.h"
-
-BackgroundThreadBase::BackgroundThreadBase(QObject *parent)
-  : QThread(parent),
-    io_priority_(IOPRIO_CLASS_NONE),
-    cpu_priority_(InheritPriority)
-{
-}
-
-int BackgroundThreadBase::SetIOPriority() {
-#ifdef Q_OS_LINUX
-  return syscall(SYS_ioprio_set, IOPRIO_WHO_PROCESS, gettid(),
-                 4 | io_priority_ << IOPRIO_CLASS_SHIFT);
-#elif defined(Q_OS_DARWIN)
-  return setpriority(PRIO_DARWIN_THREAD, 0,
-                     io_priority_ == IOPRIO_CLASS_IDLE ? PRIO_DARWIN_BG : 0);
-#else
-  return 0;
-#endif
-}
-
-int BackgroundThreadBase::gettid() {
-#ifdef Q_OS_LINUX
-  return syscall(SYS_gettid);
-#else
-  return 0;
-#endif
-}
-
-void BackgroundThreadBase::Start(bool block) {
-  if (!block) {
-    // Just start the thread and return immediately
-    start(cpu_priority_);
-    return;
-  }
-
-  // Lock the mutex so the new thread won't try to wake us up before we start
-  // waiting.
-  QMutexLocker l(&started_wait_condition_mutex_);
-
-  // Start the thread.
-  start(cpu_priority_);
-
-  // Wait for the thread to initalise.
-  started_wait_condition_.wait(l.mutex());
-}
-

src/core/backgroundthread.h

@@ -1,185 +0,0 @@
-/* This file is part of Clementine.
-   Copyright 2010, David Sansome <me@davidsansome.com>
-
-   Clementine 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 3 of the License, or
-   (at your option) any later version.
-
-   Clementine 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 Clementine.  If not, see <http://www.gnu.org/licenses/>.
-*/
-
-#ifndef BACKGROUNDTHREAD_H
-#define BACKGROUNDTHREAD_H
-
-#include <QThread>
-#include <QtDebug>
-#include <QWaitCondition>
-#include <QMutexLocker>
-#include <QCoreApplication>
-
-#include <boost/shared_ptr.hpp>
-
-#ifdef Q_OS_LINUX
-#  include <sys/syscall.h>
-#endif
-#ifdef Q_OS_DARWIN
-#  include <sys/resource.h>
-#endif
-
-// These classes are a bit confusing because they're trying to do so many
-// things:
-//  * Run a worker in a background thread
-//  * ... or maybe run it in the same thread if we're in a test
-//  * Use interfaces throughout, so the implementations can be mocked
-//  * Create concrete implementations of the interfaces when threads start
-//
-// The types you should use throughout your header files are:
-//   BackgroundThread<InterfaceType>
-//   BackgroundThreadFactory<InterfaceType>
-//
-// You should allow callers to set their own factory (which might return mocks
-// of your interface), and default to using a:
-//   BackgroundThreadFactoryImplementation<InterfaceType, DerivedType>
-
-
-// This is the base class.  We need one because moc doesn't like templated
-// classes.  This also deals with anything that doesn't depend on the type of
-// the worker.
-class BackgroundThreadBase : public QThread {
-  Q_OBJECT
- public:
-  BackgroundThreadBase(QObject* parent = 0);
-
-  // Borrowed from schedutils
-  enum IoPriority {
-    IOPRIO_CLASS_NONE = 0,
-    IOPRIO_CLASS_RT,
-    IOPRIO_CLASS_BE,
-    IOPRIO_CLASS_IDLE,
-  };
-
-  void set_io_priority(IoPriority priority) { io_priority_ = priority; }
-  void set_cpu_priority(QThread::Priority priority) { cpu_priority_ = priority; }
-
-  virtual void Start(bool block = false);
-
- signals:
-  void Initialised();
-
- protected:
-  int SetIOPriority();
-  static int gettid();
-
-  enum {
-    IOPRIO_WHO_PROCESS = 1,
-    IOPRIO_WHO_PGRP,
-    IOPRIO_WHO_USER,
-  };
-  static const int IOPRIO_CLASS_SHIFT = 13;
-
-  IoPriority io_priority_;
-  QThread::Priority cpu_priority_;
-
-  QWaitCondition started_wait_condition_;
-  QMutex started_wait_condition_mutex_;
-};
-
-// This is the templated class that stores and returns the worker object.
-template <typename InterfaceType>
-class BackgroundThread : public BackgroundThreadBase {
- public:
-  BackgroundThread(QObject* parent = 0);
-  ~BackgroundThread();
-
-  boost::shared_ptr<InterfaceType> Worker() const { return worker_; }
-
- protected:
-  boost::shared_ptr<InterfaceType> worker_;
-};
-
-// This class actually creates an implementation of the worker object
-template <typename InterfaceType, typename DerivedType>
-class BackgroundThreadImplementation : public BackgroundThread<InterfaceType> {
- public:
-  BackgroundThreadImplementation(QObject* parent = 0);
-
- protected:
-  void run();
-};
-
-
-// This is a pure virtual factory for creating threads.
-template <typename InterfaceType>
-class BackgroundThreadFactory {
- public:
-  virtual ~BackgroundThreadFactory() {}
-  virtual BackgroundThread<InterfaceType>* GetThread(QObject* parent) = 0;
-};
-
-// This implementation of the factory returns a BackgroundThread that creates
-// the right derived types...
-template <typename InterfaceType, typename DerivedType>
-class BackgroundThreadFactoryImplementation : public BackgroundThreadFactory<InterfaceType> {
- public:
-  BackgroundThread<InterfaceType>* GetThread(QObject* parent) {
-    return new BackgroundThreadImplementation<InterfaceType, DerivedType>(parent);
-  }
-};
-
-template <typename InterfaceType>
-BackgroundThread<InterfaceType>::BackgroundThread(QObject *parent)
-  : BackgroundThreadBase(parent)
-{
-}
-
-template <typename InterfaceType>
-BackgroundThread<InterfaceType>::~BackgroundThread() {
-  if (isRunning()) {
-    if (boost::shared_ptr<InterfaceType> w = worker_)
-      w->Stop();
-
-    quit();
-    if (wait(1000))
-      return;
-    terminate();
-    wait(1000);
-  }
-}
-
-template <typename InterfaceType, typename DerivedType>
-BackgroundThreadImplementation<InterfaceType, DerivedType>::
-    BackgroundThreadImplementation(QObject* parent)
-      : BackgroundThread<InterfaceType>(parent)
-{
-}
-
-
-template <typename InterfaceType, typename DerivedType>
-void BackgroundThreadImplementation<InterfaceType, DerivedType>::run() {
-  this->SetIOPriority();
-
-  this->worker_.reset(new DerivedType);
-
-  {
-    // Tell the calling thread that we've initialised the worker.
-    QMutexLocker l(&this->started_wait_condition_mutex_);
-    this->started_wait_condition_.wakeAll();
-  }
-
-  emit this->Initialised();
-  QThread::exec();
-
-  this->worker_.reset();
-}
-
-
-
-
-#endif // BACKGROUNDTHREAD_H

src/core/utilities.cpp

@@ -43,6 +43,13 @@
 #  include <windows.h>
 #endif
 
+#ifdef Q_OS_LINUX
+#  include <sys/syscall.h>
+#endif
+#ifdef Q_OS_DARWIN
+#  include <sys/resource.h>
+#endif
+
 #ifdef Q_OS_DARWIN
 #  include "core/mac_startup.h"
 #  include "CoreServices/CoreServices.h"
@@ -407,6 +414,26 @@ const char* EnumToString(const QMetaObject& meta, const char* name, int value) {
   return result;
 }
 
+int SetThreadIOPriority(IoPriority priority) {
+#ifdef Q_OS_LINUX
+  return syscall(SYS_ioprio_set, IOPRIO_WHO_PROCESS, GetThreadId(),
+                 4 | priority << IOPRIO_CLASS_SHIFT);
+#elif defined(Q_OS_DARWIN)
+  return setpriority(PRIO_DARWIN_THREAD, 0,
+                     priority == IOPRIO_CLASS_IDLE ? PRIO_DARWIN_BG : 0);
+#else
+  return 0;
+#endif
+}
+
+int GetThreadId() {
+#ifdef Q_OS_LINUX
+  return syscall(SYS_gettid);
+#else
+  return 0;
+#endif
+}
+
 }  // namespace Utilities
 
 

src/core/utilities.h

@@ -100,6 +100,23 @@ namespace Utilities {
 
   // Returns the minor version of OS X (ie. 6 for Snow Leopard, 7 for Lion).
   qint32 GetMacVersion();
+
+  // Borrowed from schedutils
+  enum IoPriority {
+    IOPRIO_CLASS_NONE = 0,
+    IOPRIO_CLASS_RT,
+    IOPRIO_CLASS_BE,
+    IOPRIO_CLASS_IDLE,
+  };
+  enum {
+    IOPRIO_WHO_PROCESS = 1,
+    IOPRIO_WHO_PGRP,
+    IOPRIO_WHO_USER,
+  };
+  static const int IOPRIO_CLASS_SHIFT = 13;
+
+  int SetThreadIOPriority(IoPriority priority);
+  int GetThreadId();
 }
 
 class ScopedWCharArray {

src/covers/albumcoverloader.h

@@ -19,7 +19,6 @@
 #define ALBUMCOVERLOADER_H
 
 #include "albumcoverloaderoptions.h"
-#include "core/backgroundthread.h"
 #include "core/song.h"
 
 #include <QImage>

src/devices/filesystemdevice.cpp

@@ -23,6 +23,7 @@
 #include "library/librarymodel.h"
 #include "library/librarywatcher.h"
 
+#include <QThread>
 #include <QtDebug>
 
 FilesystemDevice::FilesystemDevice(
@@ -32,35 +33,34 @@ FilesystemDevice::FilesystemDevice(
     int database_id, bool first_time)
       : FilesystemMusicStorage(url.toLocalFile()),
         ConnectedDevice(url, lister, unique_id, manager, app, database_id, first_time),
-        watcher_(new BackgroundThreadImplementation<LibraryWatcher, LibraryWatcher>(this))
+        watcher_(new LibraryWatcher),
+        watcher_thread_(new QThread(this))
 {
-  // Create the library watcher
-  watcher_->Start(true);
-  watcher_->Worker()->set_device_name(manager->data(manager->index(
-      manager->FindDeviceById(unique_id)), DeviceManager::Role_FriendlyName).toString());
-  watcher_->Worker()->set_backend(backend_);
-  watcher_->Worker()->set_task_manager(app_->task_manager());
+  watcher_->moveToThread(watcher_thread_);
+  watcher_thread_->start(QThread::IdlePriority);
 
-  // To make the connections below less verbose
-  LibraryWatcher* watcher = watcher_->Worker().get();
+  watcher_->set_device_name(manager->data(manager->index(
+      manager->FindDeviceById(unique_id)), DeviceManager::Role_FriendlyName).toString());
+  watcher_->set_backend(backend_);
+  watcher_->set_task_manager(app_->task_manager());
 
   connect(backend_, SIGNAL(DirectoryDiscovered(Directory,SubdirectoryList)),
-          watcher,  SLOT(AddDirectory(Directory,SubdirectoryList)));
+          watcher_, SLOT(AddDirectory(Directory,SubdirectoryList)));
   connect(backend_, SIGNAL(DirectoryDeleted(Directory)),
-          watcher,  SLOT(RemoveDirectory(Directory)));
-  connect(watcher,  SIGNAL(NewOrUpdatedSongs(SongList)),
+          watcher_, SLOT(RemoveDirectory(Directory)));
+  connect(watcher_, SIGNAL(NewOrUpdatedSongs(SongList)),
           backend_, SLOT(AddOrUpdateSongs(SongList)));
-  connect(watcher,  SIGNAL(SongsMTimeUpdated(SongList)),
+  connect(watcher_, SIGNAL(SongsMTimeUpdated(SongList)),
           backend_, SLOT(UpdateMTimesOnly(SongList)));
-  connect(watcher,  SIGNAL(SongsDeleted(SongList)),
+  connect(watcher_, SIGNAL(SongsDeleted(SongList)),
           backend_, SLOT(DeleteSongs(SongList)));
-  connect(watcher,  SIGNAL(SubdirsDiscovered(SubdirectoryList)),
+  connect(watcher_, SIGNAL(SubdirsDiscovered(SubdirectoryList)),
           backend_, SLOT(AddOrUpdateSubdirs(SubdirectoryList)));
-  connect(watcher,  SIGNAL(SubdirsMTimeUpdated(SubdirectoryList)),
+  connect(watcher_, SIGNAL(SubdirsMTimeUpdated(SubdirectoryList)),
           backend_, SLOT(AddOrUpdateSubdirs(SubdirectoryList)));
-  connect(watcher,  SIGNAL(CompilationsNeedUpdating()),
+  connect(watcher_, SIGNAL(CompilationsNeedUpdating()),
           backend_, SLOT(UpdateCompilations()));
-  connect(watcher, SIGNAL(ScanStarted(int)), SIGNAL(TaskStarted(int)));
+  connect(watcher_, SIGNAL(ScanStarted(int)), SIGNAL(TaskStarted(int)));
 }
 
 void FilesystemDevice::Init() {
@@ -69,4 +69,7 @@ void FilesystemDevice::Init() {
 }
 
 FilesystemDevice::~FilesystemDevice() {
+  watcher_->deleteLater();
+  watcher_thread_->exit();
+  watcher_thread_->wait();
 }

src/devices/filesystemdevice.h

@@ -19,7 +19,6 @@
 #define FILESYSTEMDEVICE_H
 
 #include "connecteddevice.h"
-#include "core/backgroundthread.h"
 #include "core/filesystemmusicstorage.h"
 
 class DeviceManager;
@@ -41,7 +40,8 @@ public:
   static QStringList url_schemes() { return QStringList() << "file"; }
 
 private:
-  BackgroundThread<LibraryWatcher>* watcher_;
+  LibraryWatcher* watcher_;
+  QThread* watcher_thread_;
 };
 
 #endif // FILESYSTEMDEVICE_H

src/globalsearch/globalsearch.cpp

@@ -24,6 +24,7 @@
 
 #include <QSettings>
 #include <QStringBuilder>
+#include <QTimerEvent>
 #include <QUrl>
 
 const int GlobalSearch::kDelayedSearchTimeoutMs = 200;

src/globalsearch/groovesharksearchprovider.h

@@ -19,7 +19,6 @@
 #define GROOVESHARKSEARCHPROVIDER_H
 
 #include "searchprovider.h"
-#include "core/backgroundthread.h"
 #include "covers/albumcoverloaderoptions.h"
 
 class AlbumCoverLoader;

src/internet/internetmodel.h

@@ -18,7 +18,6 @@
 #ifndef INTERNETMODEL_H
 #define INTERNETMODEL_H
 
-#include "core/backgroundthread.h"
 #include "core/song.h"
 #include "library/librarymodel.h"
 #include "playlist/playlistitem.h"

src/internet/spotifyblobdownloader.cpp

@@ -22,6 +22,7 @@
 #include "core/network.h"
 #include "core/utilities.h"
 
+#include <QCoreApplication>
 #include <QDir>
 #include <QMessageBox>
 #include <QNetworkReply>

src/library/library.cpp

@@ -25,6 +25,8 @@
 #include "smartplaylists/querygenerator.h"
 #include "smartplaylists/search.h"
 
+#include <QThread>
+
 const char* Library::kSongsTable = "songs";
 const char* Library::kDirsTable = "directories";
 const char* Library::kSubdirsTable = "subdirectories";
@@ -35,8 +37,8 @@ Library::Library(Application* app, QObject *parent)
     app_(app),
     backend_(NULL),
     model_(NULL),
-    watcher_factory_(new BackgroundThreadFactoryImplementation<LibraryWatcher, LibraryWatcher>),
-    watcher_(NULL)
+    watcher_(NULL),
+    watcher_thread_(NULL)
 {
   backend_ = new LibraryBackend;
   backend()->moveToThread(app->database()->thread());
@@ -96,83 +98,65 @@ Library::Library(Application* app, QObject *parent)
   full_rescan_revisions_[26] = tr("CUE sheet support");
 }
 
-void Library::set_watcher_factory(BackgroundThreadFactory<LibraryWatcher>* factory) {
-  watcher_factory_.reset(factory);
+Library::~Library() {
+  watcher_->deleteLater();
+  watcher_thread_->exit();
+  watcher_thread_->wait();
 }
 
 void Library::Init() {
-  watcher_ = watcher_factory_->GetThread(this);
-  connect(watcher_, SIGNAL(Initialised()), SLOT(WatcherInitialised()));
-}
+  watcher_ = new LibraryWatcher;
+  watcher_thread_ = new QThread(this);
 
-void Library::StartThreads() {
-  Q_ASSERT(watcher_);
+  watcher_->moveToThread(watcher_thread_);
+  watcher_thread_->start(QThread::IdlePriority);
 
-  watcher_->set_io_priority(BackgroundThreadBase::IOPRIO_CLASS_IDLE);
-  watcher_->set_cpu_priority(QThread::IdlePriority);
-  watcher_->Start();
-
-  model_->Init();
-}
-
-void Library::WatcherInitialised() {
-  LibraryWatcher* watcher = watcher_->Worker().get();
-
-  watcher->set_backend(backend_);
-  watcher->set_task_manager(app_->task_manager());
+  watcher_->set_backend(backend_);
+  watcher_->set_task_manager(app_->task_manager());
 
   connect(backend_, SIGNAL(DirectoryDiscovered(Directory,SubdirectoryList)),
-          watcher,  SLOT(AddDirectory(Directory,SubdirectoryList)));
+          watcher_, SLOT(AddDirectory(Directory,SubdirectoryList)));
   connect(backend_, SIGNAL(DirectoryDeleted(Directory)),
-          watcher,  SLOT(RemoveDirectory(Directory)));
-  connect(watcher,  SIGNAL(NewOrUpdatedSongs(SongList)),
+          watcher_, SLOT(RemoveDirectory(Directory)));
+  connect(watcher_, SIGNAL(NewOrUpdatedSongs(SongList)),
           backend_, SLOT(AddOrUpdateSongs(SongList)));
-  connect(watcher,  SIGNAL(SongsMTimeUpdated(SongList)),
+  connect(watcher_, SIGNAL(SongsMTimeUpdated(SongList)),
           backend_, SLOT(UpdateMTimesOnly(SongList)));
-  connect(watcher,  SIGNAL(SongsDeleted(SongList)),
+  connect(watcher_, SIGNAL(SongsDeleted(SongList)),
           backend_, SLOT(MarkSongsUnavailable(SongList)));
-  connect(watcher,  SIGNAL(SubdirsDiscovered(SubdirectoryList)),
+  connect(watcher_, SIGNAL(SubdirsDiscovered(SubdirectoryList)),
           backend_, SLOT(AddOrUpdateSubdirs(SubdirectoryList)));
-  connect(watcher,  SIGNAL(SubdirsMTimeUpdated(SubdirectoryList)),
+  connect(watcher_, SIGNAL(SubdirsMTimeUpdated(SubdirectoryList)),
           backend_, SLOT(AddOrUpdateSubdirs(SubdirectoryList)));
-  connect(watcher, SIGNAL(CompilationsNeedUpdating()),
+  connect(watcher_, SIGNAL(CompilationsNeedUpdating()),
           backend_, SLOT(UpdateCompilations()));
 
   // This will start the watcher checking for updates
   backend_->LoadDirectoriesAsync();
 }
 
-void Library::IncrementalScan() {
-  if (!watcher_->Worker())
-    return;
+void Library::StartThreads() {
+  Q_ASSERT(watcher_);
 
-  watcher_->Worker()->IncrementalScanAsync();
+  model_->Init();
+}
+
+void Library::IncrementalScan() {
+  watcher_->IncrementalScanAsync();
 }
 
 void Library::FullScan() {
-  if (!watcher_->Worker())
-    return;
-
-  watcher_->Worker()->FullScanAsync();
+  watcher_->FullScanAsync();
 }
 
 void Library::PauseWatcher() {
-  if (!watcher_->Worker())
-    return;
-
-  watcher_->Worker()->SetRescanPausedAsync(true);
+  watcher_->SetRescanPausedAsync(true);
 }
 
 void Library::ResumeWatcher() {
-  if (!watcher_->Worker())
-    return;
-
-  watcher_->Worker()->SetRescanPausedAsync(false);
+  watcher_->SetRescanPausedAsync(false);
 }
 
 void Library::ReloadSettings() {
-  if (!watcher_->Worker())
-    return;
-
-  watcher_->Worker()->ReloadSettingsAsync();
+  watcher_->ReloadSettingsAsync();
 }

src/library/library.h

@@ -18,8 +18,7 @@
 #ifndef LIBRARY_H
 #define LIBRARY_H
 
-#include "core/backgroundthread.h"
-
+#include <QHash>
 #include <QObject>
 
 #include <boost/scoped_ptr.hpp>
@@ -36,15 +35,13 @@ class Library : public QObject {
 
  public:
   Library(Application* app, QObject* parent);
+  ~Library();
 
   static const char* kSongsTable;
   static const char* kDirsTable;
   static const char* kSubdirsTable;
   static const char* kFtsTable;
 
-  // Useful for tests.  The library takes ownership.
-  void set_watcher_factory(BackgroundThreadFactory<LibraryWatcher>* factory);
-
   void Init();
   void StartThreads();
 
@@ -63,15 +60,14 @@ class Library : public QObject {
 
  private slots:
   void IncrementalScan();
-  void WatcherInitialised();
 
  private:
   Application* app_;
   LibraryBackend* backend_;
   LibraryModel* model_;
 
-  boost::scoped_ptr<BackgroundThreadFactory<LibraryWatcher> > watcher_factory_;
-  BackgroundThread<LibraryWatcher>* watcher_;
+  LibraryWatcher* watcher_;
+  QThread* watcher_thread_;
 
   // DB schema versions which should trigger a full library rescan (each of those with
   // a short reason why).

src/library/librarywatcher.cpp

@@ -22,6 +22,7 @@
 #include "core/logging.h"
 #include "core/tagreaderclient.h"
 #include "core/taskmanager.h"
+#include "core/utilities.h"
 #include "playlistparsers/cueparser.h"
 
 #include <QDateTime>
@@ -59,6 +60,8 @@ LibraryWatcher::LibraryWatcher(QObject* parent)
     total_watches_(0),
     cue_parser_(new CueParser(backend_, this))
 {
+  Utilities::SetThreadIOPriority(Utilities::IOPRIO_CLASS_IDLE);
+
   rescan_timer_->setInterval(1000);
   rescan_timer_->setSingleShot(true);