Fix LibraryModel async query crash.

A LibraryBackend may be deleted while an associated LibraryModel object is using
it. An example is an async query running while a connected device is removed.
To prevent this, use a share pointer for the LibraryBackend.

This fixes one case where LibraryBackend is used after deletion. However, the
raw pointer is still passed around in several other places. These should be
evaluated on a case-by-case basis to insure that circular depencencies aren't
introduced.
This commit is contained in:
Jim Broadus 2020-01-11 23:34:35 -08:00
parent 4563149482
commit 6a9276ec0a
19 changed files with 92 additions and 78 deletions

View File

@ -39,16 +39,15 @@ ConnectedDevice::ConnectedDevice(const QUrl& url, DeviceLister* lister,
unique_id_(unique_id),
database_id_(database_id),
manager_(manager),
backend_(nullptr),
model_(nullptr),
song_count_(0) {
qLog(Info) << "connected" << url << unique_id << first_time;
// Create the backend in the database thread.
backend_ = new LibraryBackend();
backend_.reset(new LibraryBackend(), [](QObject* obj) { delete obj; });
backend_->moveToThread(app_->database()->thread());
connect(backend_, SIGNAL(TotalSongCountUpdated(int)),
connect(backend_.get(), SIGNAL(TotalSongCountUpdated(int)),
SLOT(BackendTotalSongCountUpdated(int)));
backend_->Init(app_->database(), QString("device_%1_songs").arg(database_id),
@ -60,7 +59,7 @@ ConnectedDevice::ConnectedDevice(const QUrl& url, DeviceLister* lister,
model_ = new LibraryModel(backend_, app_, this);
}
ConnectedDevice::~ConnectedDevice() { backend_->deleteLater(); }
ConnectedDevice::~ConnectedDevice() {}
void ConnectedDevice::InitBackendDirectory(const QString& mount_point,
bool first_time, bool rewrite_path) {

View File

@ -84,7 +84,7 @@ signals:
int database_id_;
DeviceManager* manager_;
LibraryBackend* backend_;
std::shared_ptr<LibraryBackend> backend_;
LibraryModel* model_;
int song_count_;

View File

@ -43,24 +43,25 @@ FilesystemDevice::FilesystemDevice(const QUrl& url, DeviceLister* lister,
->data(manager->ItemToIndex(manager->FindDeviceById(unique_id)),
DeviceManager::Role_FriendlyName)
.toString());
watcher_->set_backend(backend_);
watcher_->set_backend(backend_.get());
watcher_->set_task_manager(app_->task_manager());
connect(backend_, SIGNAL(DirectoryDiscovered(Directory, SubdirectoryList)),
watcher_, SLOT(AddDirectory(Directory, SubdirectoryList)));
connect(backend_, SIGNAL(DirectoryDeleted(Directory)), watcher_,
connect(backend_.get(),
SIGNAL(DirectoryDiscovered(Directory, SubdirectoryList)), watcher_,
SLOT(AddDirectory(Directory, SubdirectoryList)));
connect(backend_.get(), SIGNAL(DirectoryDeleted(Directory)), watcher_,
SLOT(RemoveDirectory(Directory)));
connect(watcher_, SIGNAL(NewOrUpdatedSongs(SongList)), backend_,
connect(watcher_, SIGNAL(NewOrUpdatedSongs(SongList)), backend_.get(),
SLOT(AddOrUpdateSongs(SongList)));
connect(watcher_, SIGNAL(SongsMTimeUpdated(SongList)), backend_,
connect(watcher_, SIGNAL(SongsMTimeUpdated(SongList)), backend_.get(),
SLOT(UpdateMTimesOnly(SongList)));
connect(watcher_, SIGNAL(SongsDeleted(SongList)), backend_,
connect(watcher_, SIGNAL(SongsDeleted(SongList)), backend_.get(),
SLOT(DeleteSongs(SongList)));
connect(watcher_, SIGNAL(SubdirsDiscovered(SubdirectoryList)), backend_,
connect(watcher_, SIGNAL(SubdirsDiscovered(SubdirectoryList)), backend_.get(),
SLOT(AddOrUpdateSubdirs(SubdirectoryList)));
connect(watcher_, SIGNAL(SubdirsMTimeUpdated(SubdirectoryList)), backend_,
SLOT(AddOrUpdateSubdirs(SubdirectoryList)));
connect(watcher_, SIGNAL(CompilationsNeedUpdating()), backend_,
connect(watcher_, SIGNAL(SubdirsMTimeUpdated(SubdirectoryList)),
backend_.get(), SLOT(AddOrUpdateSubdirs(SubdirectoryList)));
connect(watcher_, SIGNAL(CompilationsNeedUpdating()), backend_.get(),
SLOT(UpdateCompilations()));
connect(watcher_, SIGNAL(ScanStarted(int)), SIGNAL(TaskStarted(int)));
}

View File

@ -28,7 +28,7 @@
#include <QtDebug>
GPodLoader::GPodLoader(const QString& mount_point, TaskManager* task_manager,
LibraryBackend* backend,
std::shared_ptr<LibraryBackend> backend,
std::shared_ptr<ConnectedDevice> device)
: QObject(nullptr),
device_(device),

View File

@ -35,7 +35,8 @@ class GPodLoader : public QObject {
public:
GPodLoader(const QString& mount_point, TaskManager* task_manager,
LibraryBackend* backend, std::shared_ptr<ConnectedDevice> device);
std::shared_ptr<LibraryBackend> backend,
std::shared_ptr<ConnectedDevice> device);
~GPodLoader();
void set_music_path_prefix(const QString& prefix) { path_prefix_ = prefix; }
@ -57,7 +58,7 @@ signals:
QString path_prefix_;
Song::FileType type_;
TaskManager* task_manager_;
LibraryBackend* backend_;
std::shared_ptr<LibraryBackend> backend_;
};
#endif // GPODLOADER_H

View File

@ -26,7 +26,7 @@
#include "library/librarybackend.h"
MtpLoader::MtpLoader(const QUrl& url, TaskManager* task_manager,
LibraryBackend* backend,
std::shared_ptr<LibraryBackend> backend,
std::shared_ptr<ConnectedDevice> device)
: QObject(nullptr),
device_(device),

View File

@ -31,7 +31,8 @@ class MtpLoader : public QObject {
Q_OBJECT
public:
MtpLoader(const QUrl& url, TaskManager* task_manager, LibraryBackend* backend,
MtpLoader(const QUrl& url, TaskManager* task_manager,
std::shared_ptr<LibraryBackend> backend,
std::shared_ptr<ConnectedDevice> device);
~MtpLoader();
@ -52,7 +53,7 @@ signals:
QUrl url_;
TaskManager* task_manager_;
LibraryBackend* backend_;
std::shared_ptr<LibraryBackend> backend_;
};
#endif // MTPLOADER_H

View File

@ -52,7 +52,8 @@ CloudFileService::CloudFileService(Application* app, InternetModel* parent,
indexing_task_id_(-1),
indexing_task_progress_(0),
indexing_task_max_(0) {
library_backend_ = new LibraryBackend;
library_backend_.reset(new LibraryBackend,
[](QObject* obj) { obj->deleteLater(); });
library_backend_->moveToThread(app_->database()->thread());
QString songs_table = service_id + "_songs";
@ -68,8 +69,8 @@ CloudFileService::CloudFileService(Application* app, InternetModel* parent,
library_sort_model_->setSortLocaleAware(true);
library_sort_model_->sort(0);
app->global_search()->AddProvider(
new CloudFileSearchProvider(library_backend_, service_id, icon_, this));
app->global_search()->AddProvider(new CloudFileSearchProvider(
library_backend_.get(), service_id, icon_, this));
}
QStandardItem* CloudFileService::CreateRootItem() {
@ -112,7 +113,7 @@ void CloudFileService::ShowContextMenu(const QPoint& global_pos) {
void CloudFileService::ShowCoverManager() {
if (!cover_manager_) {
cover_manager_.reset(new AlbumCoverManager(app_, library_backend_));
cover_manager_.reset(new AlbumCoverManager(app_, library_backend_.get()));
cover_manager_->Init();
connect(cover_manager_.get(), SIGNAL(AddToPlaylist(QMimeData*)),
SLOT(AddToPlaylist(QMimeData*)));

View File

@ -79,7 +79,7 @@ signals:
QStandardItem* root_;
NetworkAccessManager* network_;
LibraryBackend* library_backend_;
std::shared_ptr<LibraryBackend> library_backend_;
LibraryModel* library_model_;
QSortFilterProxyModel* library_sort_model_;

View File

@ -90,11 +90,12 @@ JamendoService::JamendoService(Application* app, InternetModel* parent)
load_database_task_id_(0),
total_song_count_(0),
accepted_download_(false) {
library_backend_ = new LibraryBackend;
library_backend_.reset(new LibraryBackend,
[](QObject* obj) { obj->deleteLater(); });
library_backend_->moveToThread(app_->database()->thread());
library_backend_->Init(app_->database(), kSongsTable, QString(), QString(),
kFtsTable);
connect(library_backend_, SIGNAL(TotalSongCountUpdated(int)),
connect(library_backend_.get(), SIGNAL(TotalSongCountUpdated(int)),
SLOT(UpdateTotalSongCount(int)));
using smart_playlists::Generator;
@ -134,7 +135,7 @@ JamendoService::JamendoService(Application* app, InternetModel* parent)
library_sort_model_->sort(0);
search_provider_ = new LibrarySearchProvider(
library_backend_, tr("Jamendo"), "jamendo",
library_backend_.get(), tr("Jamendo"), "jamendo",
IconLoader::Load("jamendo", IconLoader::Provider), false, app_, this);
app_->global_search()->AddProvider(search_provider_);
connect(app_->global_search(),
@ -234,9 +235,9 @@ void JamendoService::ParseDirectory(QIODevice* device) const {
int total_count = 0;
// Bit of a hack: don't update the model while we're parsing the xml
disconnect(library_backend_, SIGNAL(SongsDiscovered(SongList)),
disconnect(library_backend_.get(), SIGNAL(SongsDiscovered(SongList)),
library_model_, SLOT(SongsDiscovered(SongList)));
disconnect(library_backend_, SIGNAL(TotalSongCountUpdated(int)), this,
disconnect(library_backend_.get(), SIGNAL(TotalSongCountUpdated(int)), this,
SLOT(UpdateTotalSongCount(int)));
// Delete the database and recreate it. This is faster than dropping tables
@ -271,9 +272,9 @@ void JamendoService::ParseDirectory(QIODevice* device) const {
library_backend_->AddOrUpdateSongs(songs);
InsertTrackIds(track_ids);
connect(library_backend_, SIGNAL(SongsDiscovered(SongList)), library_model_,
SLOT(SongsDiscovered(SongList)));
connect(library_backend_, SIGNAL(TotalSongCountUpdated(int)),
connect(library_backend_.get(), SIGNAL(SongsDiscovered(SongList)),
library_model_, SLOT(SongsDiscovered(SongList)));
connect(library_backend_.get(), SIGNAL(TotalSongCountUpdated(int)),
SLOT(UpdateTotalSongCount(int)));
library_backend_->UpdateTotalSongCount();

View File

@ -53,7 +53,7 @@ class JamendoService : public InternetService {
QWidget* HeaderWidget() const;
LibraryBackend* library_backend() const { return library_backend_; }
LibraryBackend* library_backend() const { return library_backend_.get(); }
static const char* kServiceName;
static const char* kDirectoryUrl;
@ -110,7 +110,7 @@ class JamendoService : public InternetService {
QAction* album_info_;
QAction* download_album_;
LibraryBackend* library_backend_;
std::shared_ptr<LibraryBackend> library_backend_;
LibraryFilterWidget* library_filter_;
LibraryModel* library_model_;
QSortFilterProxyModel* library_sort_model_;

View File

@ -89,13 +89,14 @@ MagnatuneService::MagnatuneService(Application* app, InternetModel* parent)
total_song_count_(0),
network_(new NetworkAccessManager(this)) {
// Create the library backend in the database thread
library_backend_ = new LibraryBackend;
library_backend_.reset(new LibraryBackend,
[](QObject* obj) { obj->deleteLater(); });
library_backend_->moveToThread(app_->database()->thread());
library_backend_->Init(app_->database(), kSongsTable, QString(), QString(),
kFtsTable);
library_model_ = new LibraryModel(library_backend_, app_, this);
connect(library_backend_, SIGNAL(TotalSongCountUpdated(int)),
connect(library_backend_.get(), SIGNAL(TotalSongCountUpdated(int)),
SLOT(UpdateTotalSongCount(int)));
library_sort_model_->setSourceModel(library_model_);
@ -106,9 +107,8 @@ MagnatuneService::MagnatuneService(Application* app, InternetModel* parent)
app_->player()->RegisterUrlHandler(url_handler_);
app_->global_search()->AddProvider(new LibrarySearchProvider(
library_backend_, tr("Magnatune"), "magnatune",
IconLoader::Load("magnatune", IconLoader::Provider),
true, app_, this));
library_backend_.get(), tr("Magnatune"), "magnatune",
IconLoader::Load("magnatune", IconLoader::Provider), true, app_, this));
}
MagnatuneService::~MagnatuneService() { delete context_menu_; }

View File

@ -20,6 +20,8 @@
#ifndef INTERNET_MAGNATUNE_MAGNATUNESERVICE_H_
#define INTERNET_MAGNATUNE_MAGNATUNESERVICE_H_
#include <memory>
#include <QXmlStreamReader>
#include "internet/core/internetservice.h"
@ -84,7 +86,7 @@ class MagnatuneService : public InternetService {
PreferredFormat preferred_format() const { return format_; }
QString username() const { return username_; }
QString password() const { return password_; }
LibraryBackend* library_backend() const { return library_backend_; }
LibraryBackend* library_backend() const { return library_backend_.get(); }
QUrl ModifyUrl(const QUrl& url) const;
@ -113,7 +115,7 @@ class MagnatuneService : public InternetService {
QAction* download_;
LibraryBackend* library_backend_;
std::shared_ptr<LibraryBackend> library_backend_;
LibraryModel* library_model_;
LibraryFilterWidget* library_filter_;
QSortFilterProxyModel* library_sort_model_;

View File

@ -81,11 +81,12 @@ SubsonicService::SubsonicService(Application* app, InternetModel* parent)
connect(scanner_, SIGNAL(ScanFinished()), SLOT(ReloadDatabaseFinished()));
library_backend_ = new LibraryBackend;
library_backend_.reset(new LibraryBackend,
[](QObject* obj) { obj->deleteLater(); });
library_backend_->moveToThread(app_->database()->thread());
library_backend_->Init(app_->database(), kSongsTable, QString(), QString(),
kFtsTable);
connect(library_backend_, SIGNAL(TotalSongCountUpdated(int)),
connect(library_backend_.get(), SIGNAL(TotalSongCountUpdated(int)),
SLOT(UpdateTotalSongCount(int)));
using smart_playlists::Generator;
@ -147,7 +148,7 @@ SubsonicService::SubsonicService(Application* app, InternetModel* parent)
library_filter_->AddMenuAction(config_action);
app_->global_search()->AddProvider(new LibrarySearchProvider(
library_backend_, tr("Subsonic"), "subsonic",
library_backend_.get(), tr("Subsonic"), "subsonic",
IconLoader::Load("subsonic", IconLoader::Provider), true, app_, this));
}

View File

@ -22,6 +22,8 @@
#ifndef INTERNET_SUBSONIC_SUBSONICSERVICE_H_
#define INTERNET_SUBSONIC_SUBSONICSERVICE_H_
#include <memory>
#include <QQueue>
#include "internet/core/internetmodel.h"
@ -143,7 +145,7 @@ signals:
QMenu* context_menu_;
QStandardItem* root_;
LibraryBackend* library_backend_;
std::shared_ptr<LibraryBackend> library_backend_;
LibraryModel* library_model_;
LibraryFilterWidget* library_filter_;
QSortFilterProxyModel* library_sort_model_;

View File

@ -43,7 +43,7 @@ Library::Library(Application* app, QObject* parent)
watcher_thread_(nullptr),
save_statistics_in_files_(false),
save_ratings_in_files_(false) {
backend_ = new LibraryBackend;
backend_.reset(new LibraryBackend);
backend()->moveToThread(app->database()->thread());
backend_->Init(app->database(), kSongsTable, kDirsTable, kSubdirsTable,
@ -135,30 +135,31 @@ void Library::Init() {
watcher_->moveToThread(watcher_thread_);
watcher_thread_->start(QThread::IdlePriority);
watcher_->set_backend(backend_);
watcher_->set_backend(backend_.get());
watcher_->set_task_manager(app_->task_manager());
connect(backend_, SIGNAL(DirectoryDiscovered(Directory, SubdirectoryList)),
watcher_, SLOT(AddDirectory(Directory, SubdirectoryList)));
connect(backend_, SIGNAL(DirectoryDeleted(Directory)), watcher_,
connect(backend_.get(),
SIGNAL(DirectoryDiscovered(Directory, SubdirectoryList)), watcher_,
SLOT(AddDirectory(Directory, SubdirectoryList)));
connect(backend_.get(), SIGNAL(DirectoryDeleted(Directory)), watcher_,
SLOT(RemoveDirectory(Directory)));
connect(backend_, SIGNAL(SongsRatingChanged(SongList)),
connect(backend_.get(), SIGNAL(SongsRatingChanged(SongList)),
SLOT(SongsRatingChanged(SongList)));
connect(backend_, SIGNAL(SongsStatisticsChanged(SongList)),
connect(backend_.get(), SIGNAL(SongsStatisticsChanged(SongList)),
SLOT(SongsStatisticsChanged(SongList)));
connect(watcher_, SIGNAL(NewOrUpdatedSongs(SongList)), backend_,
connect(watcher_, SIGNAL(NewOrUpdatedSongs(SongList)), backend_.get(),
SLOT(AddOrUpdateSongs(SongList)));
connect(watcher_, SIGNAL(SongsMTimeUpdated(SongList)), backend_,
connect(watcher_, SIGNAL(SongsMTimeUpdated(SongList)), backend_.get(),
SLOT(UpdateMTimesOnly(SongList)));
connect(watcher_, SIGNAL(SongsDeleted(SongList)), backend_,
connect(watcher_, SIGNAL(SongsDeleted(SongList)), backend_.get(),
SLOT(MarkSongsUnavailable(SongList)));
connect(watcher_, SIGNAL(SongsReadded(SongList, bool)), backend_,
connect(watcher_, SIGNAL(SongsReadded(SongList, bool)), backend_.get(),
SLOT(MarkSongsUnavailable(SongList, bool)));
connect(watcher_, SIGNAL(SubdirsDiscovered(SubdirectoryList)), backend_,
connect(watcher_, SIGNAL(SubdirsDiscovered(SubdirectoryList)), backend_.get(),
SLOT(AddOrUpdateSubdirs(SubdirectoryList)));
connect(watcher_, SIGNAL(SubdirsMTimeUpdated(SubdirectoryList)), backend_,
SLOT(AddOrUpdateSubdirs(SubdirectoryList)));
connect(watcher_, SIGNAL(CompilationsNeedUpdating()), backend_,
connect(watcher_, SIGNAL(SubdirsMTimeUpdated(SubdirectoryList)),
backend_.get(), SLOT(AddOrUpdateSubdirs(SubdirectoryList)));
connect(watcher_, SIGNAL(CompilationsNeedUpdating()), backend_.get(),
SLOT(UpdateCompilations()));
connect(app_->playlist_manager(), SIGNAL(CurrentSongChanged(Song)),
SLOT(CurrentSongChanged(Song)));

View File

@ -18,6 +18,8 @@
#ifndef LIBRARY_H
#define LIBRARY_H
#include <memory>
#include <QHash>
#include <QObject>
#include <QUrl>
@ -46,7 +48,7 @@ class Library : public QObject {
void Init();
LibraryBackend* backend() const { return backend_; }
LibraryBackend* backend() const { return backend_.get(); }
LibraryModel* model() const { return model_; }
QString full_rescan_reason(int schema_version) const {
@ -76,7 +78,7 @@ class Library : public QObject {
private:
Application* app_;
LibraryBackend* backend_;
std::shared_ptr<LibraryBackend> backend_;
LibraryModel* model_;
LibraryWatcher* watcher_;

View File

@ -74,12 +74,12 @@ static bool IsCompilationArtistNode(const LibraryItem* node) {
return node == node->parent->compilation_artist_node_;
}
LibraryModel::LibraryModel(LibraryBackend* backend, Application* app,
QObject* parent)
LibraryModel::LibraryModel(std::shared_ptr<LibraryBackend> backend,
Application* app, QObject* parent)
: SimpleTreeModel<LibraryItem>(new LibraryItem(this), parent),
backend_(backend),
app_(app),
dir_model_(new LibraryDirectoryModel(backend, this)),
dir_model_(new LibraryDirectoryModel(backend.get(), this)),
show_smart_playlists_(false),
show_various_artists_(true),
total_song_count_(0),
@ -115,16 +115,16 @@ LibraryModel::LibraryModel(LibraryBackend* backend, Application* app,
Qt::KeepAspectRatio,
Qt::SmoothTransformation);
connect(backend_, SIGNAL(SongsDiscovered(SongList)),
connect(backend_.get(), SIGNAL(SongsDiscovered(SongList)),
SLOT(SongsDiscovered(SongList)));
connect(backend_, SIGNAL(SongsDeleted(SongList)),
connect(backend_.get(), SIGNAL(SongsDeleted(SongList)),
SLOT(SongsDeleted(SongList)));
connect(backend_, SIGNAL(SongsStatisticsChanged(SongList)),
connect(backend_.get(), SIGNAL(SongsStatisticsChanged(SongList)),
SLOT(SongsSlightlyChanged(SongList)));
connect(backend_, SIGNAL(SongsRatingChanged(SongList)),
connect(backend_.get(), SIGNAL(SongsRatingChanged(SongList)),
SLOT(SongsSlightlyChanged(SongList)));
connect(backend_, SIGNAL(DatabaseReset()), SLOT(Reset()));
connect(backend_, SIGNAL(TotalSongCountUpdated(int)),
connect(backend_.get(), SIGNAL(DatabaseReset()), SLOT(Reset()));
connect(backend_.get(), SIGNAL(TotalSongCountUpdated(int)),
SLOT(TotalSongCountUpdatedSlot(int)));
backend_->UpdateTotalSongCountAsync();
@ -1269,7 +1269,7 @@ QMimeData* LibraryModel::mimeData(const QModelIndexList& indexes) const {
QList<QUrl> urls;
QSet<int> song_ids;
data->backend = backend_;
data->backend = backend_.get();
for (const QModelIndex& index : indexes) {
GetChildSongs(IndexToItem(index), &urls, &data->songs, &song_ids);

View File

@ -18,6 +18,8 @@
#ifndef LIBRARYMODEL_H
#define LIBRARYMODEL_H
#include <memory>
#include <QAbstractItemModel>
#include <QIcon>
#include <QNetworkDiskCache>
@ -49,7 +51,7 @@ class LibraryModel : public SimpleTreeModel<LibraryItem> {
Q_ENUMS(GroupBy)
public:
LibraryModel(LibraryBackend* backend, Application* app,
LibraryModel(std::shared_ptr<LibraryBackend> backend, Application* app,
QObject* parent = nullptr);
~LibraryModel();
@ -117,7 +119,7 @@ class LibraryModel : public SimpleTreeModel<LibraryItem> {
bool create_va;
};
LibraryBackend* backend() const { return backend_; }
LibraryBackend* backend() const { return backend_.get(); }
LibraryDirectoryModel* directory_model() const { return dir_model_; }
typedef QList<smart_playlists::GeneratorPtr> GeneratorList;
@ -259,7 +261,7 @@ signals:
bool CompareItems(const LibraryItem* a, const LibraryItem* b) const;
private:
LibraryBackend* backend_;
std::shared_ptr<LibraryBackend> backend_;
Application* app_;
LibraryDirectoryModel* dir_model_;
bool show_smart_playlists_;