From 03d876a599aead5118ccc8614680775c61f0eb90 Mon Sep 17 00:00:00 2001 From: David Sansome Date: Wed, 2 Jun 2010 15:58:07 +0000 Subject: [PATCH] Put the Database object in its own thread, and create the Library and Playlist backends in that database thread. The database calls don't happen in the database thread yet, but this is the first step towards making sure sqlite access is thread safe. --- src/core/backgroundthread.cpp | 40 ++++++++++++++++++++-- src/core/backgroundthread.h | 57 +++++++++++++++++++++++++++++--- src/core/database.h | 2 ++ src/library/library.cpp | 11 ++++-- src/library/library.h | 2 +- src/library/librarybackend.cpp | 18 +++++----- src/library/librarybackend.h | 10 +++--- src/playlist/playlistbackend.cpp | 5 ++- src/playlist/playlistbackend.h | 7 ++-- src/radio/magnatuneservice.cpp | 11 ++++-- src/radio/radiomodel.cpp | 5 +-- src/radio/radiomodel.h | 8 +++-- src/ui/mainwindow.cpp | 28 ++++++++++++---- src/ui/mainwindow.h | 2 +- 14 files changed, 163 insertions(+), 43 deletions(-) diff --git a/src/core/backgroundthread.cpp b/src/core/backgroundthread.cpp index bdb2aa531..a1773cca4 100644 --- a/src/core/backgroundthread.cpp +++ b/src/core/backgroundthread.cpp @@ -16,6 +16,41 @@ #include "backgroundthread.h" +int BackgroundThreadBase::CreateInThreadEvent::sEventType = -1; + +BackgroundThreadBase::BackgroundThreadBase(QObject *parent) + : QThread(parent), + io_priority_(IOPRIO_CLASS_NONE), + cpu_priority_(InheritPriority), + object_creator_(NULL) +{ + if (CreateInThreadEvent::sEventType == -1) + CreateInThreadEvent::sEventType = QEvent::registerEventType(); +} + +BackgroundThreadBase::CreateInThreadEvent::CreateInThreadEvent(CreateInThreadRequest *req) + : QEvent(QEvent::Type(sEventType)), + req_(req) +{ +} + +bool BackgroundThreadBase::ObjectCreator::event(QEvent* e) { + if (e->type() != CreateInThreadEvent::sEventType) + return false; + + // Create the object, parented to this object so it gets destroyed when the + // thread ends. + CreateInThreadRequest* req = static_cast(e)->req_; + req->object_ = req->meta_object_.newInstance(Q_ARG(QObject*, this)); + + // Wake up the calling thread + QMutexLocker l(&req->mutex_); + req->wait_condition_.wakeAll(); + + return true; +} + + int BackgroundThreadBase::SetIOPriority() { #ifdef Q_OS_LINUX return syscall(SYS_ioprio_set, IOPRIO_WHO_PROCESS, gettid(), @@ -39,7 +74,7 @@ int BackgroundThreadBase::gettid() { void BackgroundThreadBase::Start(bool block) { if (!block) { // Just start the thread and return immediately - QThread::start(cpu_priority_); + start(cpu_priority_); return; } @@ -48,8 +83,9 @@ void BackgroundThreadBase::Start(bool block) { QMutexLocker l(&started_wait_condition_mutex_); // Start the thread. - QThread::start(cpu_priority_); + start(cpu_priority_); // Wait for the thread to initalise. started_wait_condition_.wait(l.mutex()); } + diff --git a/src/core/backgroundthread.h b/src/core/backgroundthread.h index 3a032cfb7..aaffb681b 100644 --- a/src/core/backgroundthread.h +++ b/src/core/backgroundthread.h @@ -21,6 +21,7 @@ #include #include #include +#include #include @@ -53,10 +54,7 @@ class BackgroundThreadBase : public QThread { Q_OBJECT public: - BackgroundThreadBase(QObject* parent = 0) - : QThread(parent), - io_priority_(IOPRIO_CLASS_NONE), - cpu_priority_(InheritPriority) {} + BackgroundThreadBase(QObject* parent = 0); // Borrowed from schedutils enum IoPriority { @@ -71,10 +69,41 @@ class BackgroundThreadBase : public QThread { virtual void Start(bool block = false); + // Creates a new QObject in this thread synchronously. + // The class T needs to have a Q_INVOKABLE constructor that takes a single + // QObject* argument. + template + T* CreateInThread(); + signals: void Initialised(); protected: + struct CreateInThreadRequest { + CreateInThreadRequest(const QMetaObject& meta_object) + : meta_object_(meta_object), object_(NULL) {} + + const QMetaObject& meta_object_; + QObject* object_; + + QWaitCondition wait_condition_; + QMutex mutex_; + }; + + struct CreateInThreadEvent : public QEvent { + CreateInThreadEvent(CreateInThreadRequest* req); + + static int sEventType; + + CreateInThreadRequest* req_; + }; + + class ObjectCreator : public QObject { + public: + bool event(QEvent *); + }; + + int SetIOPriority(); static int gettid(); @@ -90,6 +119,8 @@ class BackgroundThreadBase : public QThread { QWaitCondition started_wait_condition_; QMutex started_wait_condition_mutex_; + + ObjectCreator* object_creator_; }; // This is the templated class that stores and returns the worker object. @@ -136,6 +167,22 @@ class BackgroundThreadFactoryImplementation : public BackgroundThreadFactory +T* BackgroundThreadBase::CreateInThread() { + // Create the request and lock it. + CreateInThreadRequest req(T::staticMetaObject); + QMutexLocker l(&req.mutex_); + + // Post an event to the thread. It will create the object and signal the + // wait condition. Ownership of the event is transferred to Qt. + CreateInThreadEvent* event = new CreateInThreadEvent(&req); + QCoreApplication::postEvent(object_creator_, event); + + req.wait_condition_.wait(&req.mutex_); + + return static_cast(req.object_); +} + template BackgroundThread::BackgroundThread(QObject *parent) : BackgroundThreadBase(parent) @@ -169,6 +216,7 @@ void BackgroundThreadImplementation::run() { this->SetIOPriority(); this->worker_.reset(new DerivedType); + this->object_creator_ = new BackgroundThreadBase::ObjectCreator; { // Tell the calling thread that we've initialised the worker. @@ -179,6 +227,7 @@ void BackgroundThreadImplementation::run() { emit this->Initialised(); QThread::exec(); + delete this->object_creator_; this->worker_.reset(); } diff --git a/src/core/database.h b/src/core/database.h index 5f7289bab..d7865305b 100644 --- a/src/core/database.h +++ b/src/core/database.h @@ -36,6 +36,8 @@ class Database : public QObject { static const int kSchemaVersion; static const char* kDatabaseFilename; + void Stop() {} + QSqlDatabase Connect(); bool CheckErrors(const QSqlError& error); diff --git a/src/library/library.cpp b/src/library/library.cpp index ded3d5a88..278730dc0 100644 --- a/src/library/library.cpp +++ b/src/library/library.cpp @@ -14,6 +14,7 @@ along with Clementine. If not, see . */ +#include "core/database.h" #include "library.h" #include "librarymodel.h" #include "librarybackend.h" @@ -22,13 +23,17 @@ const char* Library::kSongsTable = "songs"; const char* Library::kDirsTable = "directories"; const char* Library::kSubdirsTable = "subdirectories"; -Library::Library(Database *db, QObject *parent) +Library::Library(BackgroundThread* db_thread, QObject *parent) : QObject(parent), - backend_(new LibraryBackend(db, kSongsTable, kDirsTable, kSubdirsTable, this)), - model_(new LibraryModel(backend_, parent)), + backend_(NULL), + model_(NULL), watcher_factory_(new BackgroundThreadFactoryImplementation), watcher_(NULL) { + backend_ = db_thread->CreateInThread(); + backend_->Init(db_thread->Worker(), kSongsTable, kDirsTable, kSubdirsTable); + + model_ = new LibraryModel(backend_, this); } void Library::set_watcher_factory(BackgroundThreadFactory* factory) { diff --git a/src/library/library.h b/src/library/library.h index 5b3da252b..857eab45e 100644 --- a/src/library/library.h +++ b/src/library/library.h @@ -32,7 +32,7 @@ class Library : public QObject { Q_OBJECT public: - Library(Database* db, QObject* parent); + Library(BackgroundThread* db_thread, QObject* parent); static const char* kSongsTable; static const char* kDirsTable; diff --git a/src/library/librarybackend.cpp b/src/library/librarybackend.cpp index aca80b625..0f8c30eb9 100644 --- a/src/library/librarybackend.cpp +++ b/src/library/librarybackend.cpp @@ -25,17 +25,19 @@ #include #include -LibraryBackend::LibraryBackend(Database *db, const QString& songs_table, - const QString& dirs_table, - const QString& subdirs_table, QObject *parent) - : QObject(parent), - db_(db), - songs_table_(songs_table), - dirs_table_(dirs_table), - subdirs_table_(subdirs_table) +LibraryBackend::LibraryBackend(QObject *parent) + : QObject(parent) { } +void LibraryBackend::Init(boost::shared_ptr db, const QString &songs_table, + const QString &dirs_table, const QString &subdirs_table) { + db_ = db; + songs_table_ = songs_table; + dirs_table_ = dirs_table; + subdirs_table_ = subdirs_table; +} + void LibraryBackend::LoadDirectoriesAsync() { metaObject()->invokeMethod(this, "LoadDirectories", Qt::QueuedConnection); } diff --git a/src/library/librarybackend.h b/src/library/librarybackend.h index 05f65c61a..3fe0565c7 100644 --- a/src/library/librarybackend.h +++ b/src/library/librarybackend.h @@ -24,15 +24,17 @@ #include "libraryquery.h" #include "core/song.h" +#include + class Database; class LibraryBackend : public QObject { Q_OBJECT public: - LibraryBackend(Database* db, const QString& songs_table, - const QString& dirs_table, const QString& subdirs_table, - QObject* parent = 0); + Q_INVOKABLE LibraryBackend(QObject* parent = 0); + void Init(boost::shared_ptr db, const QString& songs_table, + const QString& dirs_table, const QString& subdirs_table); struct Album { Album() {} @@ -121,7 +123,7 @@ class LibraryBackend : public QObject { SubdirectoryList SubdirsInDirectory(int id, QSqlDatabase& db); private: - Database* db_; + boost::shared_ptr db_; QString songs_table_; QString dirs_table_; QString subdirs_table_; diff --git a/src/playlist/playlistbackend.cpp b/src/playlist/playlistbackend.cpp index dc656d50f..014de8a33 100644 --- a/src/playlist/playlistbackend.cpp +++ b/src/playlist/playlistbackend.cpp @@ -24,9 +24,8 @@ using boost::shared_ptr; -PlaylistBackend::PlaylistBackend(Database* db, QObject* parent) - : QObject(parent), - db_(db) +PlaylistBackend::PlaylistBackend(QObject* parent) + : QObject(parent) { } diff --git a/src/playlist/playlistbackend.h b/src/playlist/playlistbackend.h index 50b0f230f..a9ab01f1c 100644 --- a/src/playlist/playlistbackend.h +++ b/src/playlist/playlistbackend.h @@ -22,13 +22,16 @@ #include "playlistitem.h" +#include + class Database; class PlaylistBackend : public QObject { Q_OBJECT public: - PlaylistBackend(Database* db, QObject* parent = 0); + Q_INVOKABLE PlaylistBackend(QObject* parent = 0); + void SetDatabase(boost::shared_ptr db) { db_ = db; } struct Playlist { int id; @@ -52,7 +55,7 @@ class PlaylistBackend : public QObject { void SavePlaylist(int playlist, const PlaylistItemList& items, int last_played); private: - Database* db_; + boost::shared_ptr db_; }; #endif // PLAYLISTBACKEND_H diff --git a/src/radio/magnatuneservice.cpp b/src/radio/magnatuneservice.cpp index 2b94b8e14..c51c39a9c 100644 --- a/src/radio/magnatuneservice.cpp +++ b/src/radio/magnatuneservice.cpp @@ -50,13 +50,18 @@ MagnatuneService::MagnatuneService(RadioModel* parent) : RadioService(kServiceName, parent), root_(NULL), context_menu_(new QMenu), - library_backend_(new LibraryBackend(parent->db(), kSongsTable, - QString::null, QString::null, this)), - library_model_(new LibraryModel(library_backend_, this)), + library_backend_(NULL), + library_model_(NULL), library_sort_model_(new QSortFilterProxyModel(this)), total_song_count_(0), network_(parent->network()->network()) { + // Create the library backend in the database thread + library_backend_ = parent->db_thread()->CreateInThread(); + library_backend_->Init(parent->db_thread()->Worker(), kSongsTable, + QString::null, QString::null); + library_model_ = new LibraryModel(library_backend_, this); + connect(library_backend_, SIGNAL(TotalSongCountUpdated(int)), SLOT(UpdateTotalSongCount(int))); diff --git a/src/radio/radiomodel.cpp b/src/radio/radiomodel.cpp index 5653c56f5..e07b89d17 100644 --- a/src/radio/radiomodel.cpp +++ b/src/radio/radiomodel.cpp @@ -28,9 +28,10 @@ QMap RadioModel::sServices; -RadioModel::RadioModel(Database* db, NetworkAccessManager* network, QObject* parent) +RadioModel::RadioModel(BackgroundThread* db_thread, + NetworkAccessManager* network, QObject* parent) : SimpleTreeModel(new RadioItem(this), parent), - db_(db), + db_thread_(db_thread), merged_model_(new MergedProxyModel(this)), network_(network) { diff --git a/src/radio/radiomodel.h b/src/radio/radiomodel.h index d9be1455d..565e64ff4 100644 --- a/src/radio/radiomodel.h +++ b/src/radio/radiomodel.h @@ -18,6 +18,7 @@ #define RADIOMODEL_H #include "radioitem.h" +#include "core/backgroundthread.h" #include "core/simpletreemodel.h" #include "core/song.h" #include "playlist/playlistitem.h" @@ -33,7 +34,8 @@ class RadioModel : public SimpleTreeModel { Q_OBJECT public: - RadioModel(Database* db, NetworkAccessManager* network, QObject* parent = 0); + RadioModel(BackgroundThread* db_thread, + NetworkAccessManager* network, QObject* parent = 0); enum { Role_Type = Qt::UserRole + 1, @@ -65,7 +67,7 @@ class RadioModel : public SimpleTreeModel { const QPoint& global_pos); void ReloadSettings(); - Database* db() const { return db_; } + BackgroundThread* db_thread() const { return db_thread_; } MergedProxyModel* merged_model() const { return merged_model_; } NetworkAccessManager* network() const { return network_; } @@ -88,7 +90,7 @@ class RadioModel : public SimpleTreeModel { private: static QMap sServices; - Database* db_; + BackgroundThread* db_thread_; MergedProxyModel* merged_model_; NetworkAccessManager* network_; }; diff --git a/src/ui/mainwindow.cpp b/src/ui/mainwindow.cpp index cbf47cf9a..948e15e26 100644 --- a/src/ui/mainwindow.cpp +++ b/src/ui/mainwindow.cpp @@ -103,17 +103,17 @@ MainWindow::MainWindow(NetworkAccessManager* network, Engine::Type engine, QWidg edit_tag_dialog_(new EditTagDialog), multi_loading_indicator_(new MultiLoadingIndicator(this)), about_dialog_(new About), - database_(new Database(this)), - radio_model_(new RadioModel(database_, network, this)), - playlist_backend_(new PlaylistBackend(database_, this)), + database_(new BackgroundThreadImplementation(this)), + radio_model_(NULL), + playlist_backend_(NULL), playlists_(new PlaylistManager(this)), playlist_parser_(new PlaylistParser(this)), - player_(new Player(playlists_, radio_model_->GetLastFMService(), engine, this)), - library_(new Library(database_, this)), + player_(NULL), + library_(NULL), global_shortcuts_(new GlobalShortcuts(this)), settings_dialog_(new SettingsDialog), add_stream_dialog_(new AddStreamDialog), - cover_manager_(new AlbumCoverManager(network, library_->model()->backend())), + cover_manager_(NULL), equalizer_(new Equalizer), transcode_dialog_(new TranscodeDialog), global_shortcuts_dialog_(new GlobalShortcutsDialog(global_shortcuts_)), @@ -123,6 +123,20 @@ MainWindow::MainWindow(NetworkAccessManager* network, Engine::Type engine, QWidg track_position_timer_(new QTimer(this)), was_maximized_(false) { + // Wait for the database thread to start - lots of stuff depends on it. + database_->Start(true); + + // Create some objects in the database thread + playlist_backend_ = database_->CreateInThread(); + playlist_backend_->SetDatabase(database_->Worker()); + + // Create stuff that needs the database + radio_model_ = new RadioModel(database_, network, this); + player_ = new Player(playlists_, radio_model_->GetLastFMService(), engine, this); + library_ = new Library(database_, this); + cover_manager_.reset(new AlbumCoverManager(network, library_->backend())); + + // Initialise the UI ui_->setupUi(this); tray_icon_->setIcon(windowIcon()); tray_icon_->setToolTip(QCoreApplication::applicationName()); @@ -278,7 +292,7 @@ MainWindow::MainWindow(NetworkAccessManager* network, Engine::Type engine, QWidg connect(track_slider_, SIGNAL(ValueChanged(int)), player_, SLOT(Seek(int))); // Database connections - connect(database_, SIGNAL(Error(QString)), error_dialog_.get(), SLOT(ShowMessage(QString))); + connect(database_->Worker().get(), SIGNAL(Error(QString)), error_dialog_.get(), SLOT(ShowMessage(QString))); // Library connections connect(ui_->library_view, SIGNAL(doubleClicked(QModelIndex)), SLOT(LibraryItemDoubleClicked(QModelIndex))); diff --git a/src/ui/mainwindow.h b/src/ui/mainwindow.h index 45a079d02..27dabe609 100644 --- a/src/ui/mainwindow.h +++ b/src/ui/mainwindow.h @@ -159,7 +159,7 @@ class MainWindow : public QMainWindow { MultiLoadingIndicator* multi_loading_indicator_; boost::scoped_ptr about_dialog_; - Database* database_; + BackgroundThread* database_; RadioModel* radio_model_; PlaylistBackend* playlist_backend_; PlaylistManager* playlists_;