From 8962644ba859668d48d4e61d7af916966ada175d Mon Sep 17 00:00:00 2001 From: Jonas Kvinge Date: Sat, 7 Sep 2019 23:30:35 +0200 Subject: [PATCH] Improvements to device manager - Mount and unmount devices in lister thread - Safely close watcher and backends for devices - Enable abort loading device - Fix MTP connection --- src/collection/collection.cpp | 2 + src/collection/collectionbackend.cpp | 1 - src/collection/collectionmodel.cpp | 1 - src/core/application.cpp | 1 + src/device/connecteddevice.cpp | 15 ++- src/device/connecteddevice.h | 6 ++ src/device/deviceinfo.h | 11 +- src/device/devicelister.cpp | 47 +++++++-- src/device/devicelister.h | 18 ++-- src/device/devicemanager.cpp | 149 ++++++++++++++++++++++----- src/device/devicemanager.h | 15 ++- src/device/filesystemdevice.cpp | 49 +++++++-- src/device/filesystemdevice.h | 7 ++ src/device/giolister.cpp | 62 +++++------ src/device/giolister.h | 8 +- src/device/gpoddevice.cpp | 27 ++++- src/device/gpoddevice.h | 8 +- src/device/gpodloader.cpp | 15 ++- src/device/gpodloader.h | 4 + src/device/ilister.cpp | 8 +- src/device/ilister.h | 1 - src/device/macosdevicelister.h | 2 +- src/device/mtpdevice.cpp | 50 ++++++--- src/device/mtpdevice.h | 14 +-- src/device/mtploader.cpp | 31 +++--- src/device/mtploader.h | 11 +- src/device/udisks2lister.h | 3 +- src/playlist/playlistbackend.cpp | 1 - 28 files changed, 411 insertions(+), 156 deletions(-) diff --git a/src/collection/collection.cpp b/src/collection/collection.cpp index ffd5dc3bb..bfa578199 100644 --- a/src/collection/collection.cpp +++ b/src/collection/collection.cpp @@ -58,6 +58,7 @@ SCollection::SCollection(Application *app, QObject *parent) backend_ = new CollectionBackend(); backend()->moveToThread(app->database()->thread()); + qLog(Debug) << backend_ << "moved to thread" << app->database()->thread(); backend_->Init(app->database(), Song::Source_Collection, kSongsTable, kDirsTable, kSubdirsTable, kFtsTable); @@ -89,6 +90,7 @@ void SCollection::Init() { watcher_->moveToThread(watcher_thread_); watcher_thread_->start(QThread::IdlePriority); + qLog(Debug) << watcher_ << "moved to thread" << watcher_thread_; watcher_->set_backend(backend_); watcher_->set_task_manager(app_->task_manager()); diff --git a/src/collection/collectionbackend.cpp b/src/collection/collectionbackend.cpp index 1ceab777f..4142c5893 100644 --- a/src/collection/collectionbackend.cpp +++ b/src/collection/collectionbackend.cpp @@ -91,7 +91,6 @@ void CollectionBackend::Exit() { assert(QThread::currentThread() == thread()); - Close(); moveToThread(original_thread_); emit ExitFinished(); diff --git a/src/collection/collectionmodel.cpp b/src/collection/collectionmodel.cpp index fcb7efe1f..89748f558 100644 --- a/src/collection/collectionmodel.cpp +++ b/src/collection/collectionmodel.cpp @@ -130,7 +130,6 @@ CollectionModel::CollectionModel(CollectionBackend *backend, Application *app, Q } CollectionModel::~CollectionModel() { - backend_->Close(); delete root_; } diff --git a/src/core/application.cpp b/src/core/application.cpp index 9a49a4a99..13fde7e3c 100644 --- a/src/core/application.cpp +++ b/src/core/application.cpp @@ -246,6 +246,7 @@ QThread *Application::MoveToNewThread(QObject *object) { void Application::MoveToThread(QObject *object, QThread *thread) { object->setParent(nullptr); object->moveToThread(thread); + qLog(Debug) << object << "moved to thread" << thread; } void Application::Exit() { diff --git a/src/device/connecteddevice.cpp b/src/device/connecteddevice.cpp index e7b4d78ba..a78f82776 100644 --- a/src/device/connecteddevice.cpp +++ b/src/device/connecteddevice.cpp @@ -54,6 +54,7 @@ ConnectedDevice::ConnectedDevice(const QUrl &url, DeviceLister *lister, const QS // Create the backend in the database thread. backend_ = new CollectionBackend(); backend_->moveToThread(app_->database()->thread()); + qLog(Debug) << backend_ << "for device" << unique_id_ << "moved to thread" << app_->database()->thread(); connect(backend_, SIGNAL(TotalSongCountUpdated(int)), SLOT(BackendTotalSongCountUpdated(int))); @@ -70,7 +71,6 @@ ConnectedDevice::ConnectedDevice(const QUrl &url, DeviceLister *lister, const QS } ConnectedDevice::~ConnectedDevice() { - backend_->Close(); backend_->deleteLater(); } @@ -103,6 +103,19 @@ void ConnectedDevice::InitBackendDirectory(const QString &mount_point, bool firs void ConnectedDevice::ConnectAsync() { emit ConnectFinished(unique_id_, true); } +void ConnectedDevice::Close() { + + connect(backend_, SIGNAL(ExitFinished()), this, SLOT(CloseFinished())); + backend_->ExitAsync(); + +} + +void ConnectedDevice::CloseFinished() { + + emit CloseFinished(unique_id_); + +} + void ConnectedDevice::Eject() { DeviceInfo *info = manager_->FindDeviceById(unique_id_); diff --git a/src/device/connecteddevice.h b/src/device/connecteddevice.h index 25a7c6472..133e1a3fe 100644 --- a/src/device/connecteddevice.h +++ b/src/device/connecteddevice.h @@ -48,6 +48,7 @@ class ConnectedDevice : public QObject, public virtual MusicStorage, public std: ~ConnectedDevice(); virtual bool Init() = 0; + virtual bool IsLoading() { return false; } virtual void NewConnection() {} virtual void ConnectAsync(); // For some devices (e.g. CD devices) we don't have callbacks to be notified when something change: @@ -67,11 +68,16 @@ class ConnectedDevice : public QObject, public virtual MusicStorage, public std: virtual void FinishDelete(bool success); virtual void Eject(); + virtual void Close(); + + public slots: + void CloseFinished(); signals: void TaskStarted(int id); void SongCountUpdated(int count); void ConnectFinished(const QString& id, bool success); + void CloseFinished(const QString& id); protected: void InitBackendDirectory(const QString &mount_point, bool first_time, bool rewrite_path = true); diff --git a/src/device/deviceinfo.h b/src/device/deviceinfo.h index 681157fa7..590d6e225 100644 --- a/src/device/deviceinfo.h +++ b/src/device/deviceinfo.h @@ -68,7 +68,9 @@ class DeviceInfo : public SimpleTreeItem { size_(0), transcode_mode_(MusicStorage::Transcode_Unsupported), transcode_format_(Song::FileType_Unknown), - task_percentage_(-1) {} + task_percentage_(-1), + unmount_(false), + forget_(false) {} DeviceInfo(Type type, DeviceInfo *parent = nullptr) : SimpleTreeItem(type, parent), @@ -76,7 +78,9 @@ class DeviceInfo : public SimpleTreeItem { size_(0), transcode_mode_(MusicStorage::Transcode_Unsupported), transcode_format_(Song::FileType_Unknown), - task_percentage_(-1) {} + task_percentage_(-1), + unmount_(false), + forget_(false) {} // A device can be discovered in different ways (udisks2, gio, etc.) // Sometimes the same device is discovered more than once. In this case the device will have multiple "backends". @@ -116,6 +120,9 @@ class DeviceInfo : public SimpleTreeItem { int task_percentage_; + bool unmount_; + bool forget_; + }; #endif // DEVICEINFO_H diff --git a/src/device/devicelister.cpp b/src/device/devicelister.cpp index 5b5ca5770..7bd2ba9c1 100644 --- a/src/device/devicelister.cpp +++ b/src/device/devicelister.cpp @@ -39,7 +39,12 @@ DeviceLister::DeviceLister() : thread_(nullptr), - next_mount_request_id_(0) {} + original_thread_(nullptr), + next_mount_request_id_(0) { + + original_thread_ = thread(); + +} DeviceLister::~DeviceLister() { @@ -58,11 +63,42 @@ void DeviceLister::Start() { moveToThread(thread_); thread_->start(); + qLog(Debug) << this << "moved to thread" << thread_; } void DeviceLister::ThreadStarted() { Init(); } +int DeviceLister::MountDeviceAsync(const QString &id) { + + const int request_id = next_mount_request_id_++; + metaObject()->invokeMethod(this, "MountDevice", Qt::QueuedConnection, Q_ARG(QString, id), Q_ARG(int, request_id)); + return request_id; + +} + +void DeviceLister::UnmountDeviceAsync(const QString &id) { + metaObject()->invokeMethod(this, "UnmountDevice", Qt::QueuedConnection, Q_ARG(QString, id)); +} + +void DeviceLister::MountDevice(const QString &id, const int request_id) { + emit DeviceMounted(id, request_id, true); +} + +void DeviceLister::ExitAsync() { + metaObject()->invokeMethod(this, "Exit", Qt::QueuedConnection); +} + +void DeviceLister::Exit() { + + ShutDown(); + if (thread_) { + moveToThread(original_thread_); + } + emit ExitFinished(); + +} + namespace { #ifdef HAVE_LIBGPOD @@ -231,12 +267,3 @@ QStringList DeviceLister::GuessIconForModel(const QString &vendor, const QString return ret; } - -int DeviceLister::MountDevice(const QString &id) { - - const int ret = next_mount_request_id_++; - emit DeviceMounted(id, ret, true); - return ret; - -} - diff --git a/src/device/devicelister.h b/src/device/devicelister.h index 47b9fd8e7..b4fa4cb6a 100644 --- a/src/device/devicelister.h +++ b/src/device/devicelister.h @@ -43,6 +43,7 @@ class DeviceLister : public QObject { // Tries to start the thread and initialise the engine. This object will be moved to the new thread. void Start(); + void ExitAsync(); // If two listers know about the same device, then the metadata will get taken from the one with the highest priority. virtual int priority() const { return 100; } @@ -57,29 +58,33 @@ class DeviceLister : public QObject { virtual QVariantMap DeviceHardwareInfo(const QString &id) = 0; virtual bool DeviceNeedsMount(const QString &id) { return false; } - // When connecting to a device for the first time, do we want an user's - // confirmation for scanning it? (by default yes) + // When connecting to a device for the first time, do we want an user's confirmation for scanning it? (by default yes) virtual bool AskForScan(const QString&) const { return true; } virtual QString MakeFriendlyName(const QString &id) = 0; virtual QList MakeDeviceUrls(const QString &id) = 0; - // Ensure the device is mounted. This should run asynchronously and emit - // DeviceMounted when it's done. - virtual int MountDevice(const QString &id); + // Ensure the device is mounted. This should run asynchronously and emit DeviceMounted when it's done. + virtual int MountDeviceAsync(const QString &id); // Do whatever needs to be done to safely remove the device. - virtual void UnmountDevice(const QString &id) = 0; + virtual void UnmountDeviceAsync(const QString &id); + + private slots: + void Exit(); public slots: virtual void UpdateDeviceFreeSpace(const QString &id) = 0; virtual void ShutDown() {} + virtual void MountDevice(const QString &id, const int ret); + virtual void UnmountDevice(const QString &id) {} signals: void DeviceAdded(const QString &id); void DeviceRemoved(const QString &id); void DeviceChanged(const QString &id); void DeviceMounted(const QString &id, int request_id, bool success); + void ExitFinished(); protected: virtual bool Init() = 0; @@ -91,6 +96,7 @@ class DeviceLister : public QObject { protected: QThread *thread_; + QThread *original_thread_; int next_mount_request_id_; private slots: diff --git a/src/device/devicemanager.cpp b/src/device/devicemanager.cpp index c7d63de3e..168af840c 100644 --- a/src/device/devicemanager.cpp +++ b/src/device/devicemanager.cpp @@ -157,21 +157,86 @@ DeviceManager::~DeviceManager() { lister->ShutDown(); delete lister; } - - backend_->Close(); + listers_.clear(); delete root_; - delete backend_; + root_ = nullptr; + + backend_->deleteLater(); + backend_ = nullptr; } void DeviceManager::Exit() { + CloseDevices(); +} - connect(backend_, SIGNAL(ExitFinished()), this, SIGNAL(ExitFinished())); +void DeviceManager::CloseDevices() { + + for (DeviceInfo *info : devices_) { + if (!info->device_) continue; + if (wait_for_exit_.contains(info->device_.get())) continue; + wait_for_exit_ << info->device_.get(); + connect(info->device_.get(), SIGNAL(destroyed()), SLOT(DeviceDestroyed())); + info->device_->Close(); + } + if (wait_for_exit_.isEmpty()) CloseListers(); + +} + +void DeviceManager::CloseListers() { + + for (DeviceLister *lister : listers_) { + if (wait_for_exit_.contains(lister)) continue; + wait_for_exit_ << lister; + connect(lister, SIGNAL(ExitFinished()), this, SLOT(ListerClosed())); + lister->ExitAsync(); + } + if (wait_for_exit_.isEmpty()) CloseBackend(); + +} + +void DeviceManager::CloseBackend() { + + if (!backend_ || wait_for_exit_.contains(backend_)) return; + wait_for_exit_ << backend_; + connect(backend_, SIGNAL(ExitFinished()), this, SLOT(BackendClosed())); backend_->ExitAsync(); } +void DeviceManager::BackendClosed() { + + QObject *obj = static_cast(sender()); + disconnect(obj, 0, this, 0); + qLog(Debug) << obj << "successfully closed."; + wait_for_exit_.removeAll(obj); + if (wait_for_exit_.isEmpty()) emit ExitFinished(); + +} + +void DeviceManager::ListerClosed() { + + DeviceLister *lister = static_cast(sender()); + if (!lister) return; + + disconnect(lister, 0, this, 0); + qLog(Debug) << lister << "successfully closed."; + wait_for_exit_.removeAll(lister); + + if (wait_for_exit_.isEmpty()) CloseBackend(); + +} + +void DeviceManager::DeviceDestroyed() { + + ConnectedDevice *device = static_cast(sender()); + if (!wait_for_exit_.contains(device) || !backend_) return; + + wait_for_exit_.removeAll(device); + if (wait_for_exit_.isEmpty()) CloseListers(); + +} void DeviceManager::LoadAllDevices() { Q_ASSERT(QThread::currentThread() != qApp->thread()); @@ -183,6 +248,7 @@ void DeviceManager::LoadAllDevices() { emit DeviceCreatedFromDB(info); } + // This is done in a concurrent thread so close the unique DB connection. backend_->Close(); } @@ -238,7 +304,7 @@ QVariant DeviceManager::data(const QModelIndex &idx, int role) const { case Qt::DecorationRole: { QPixmap pixmap = info->icon_.pixmap(kDeviceIconSize); - if (info->backends_.isEmpty() || !info->BestBackend()->lister_) { + if (info->backends_.isEmpty() || !info->BestBackend() || !info->BestBackend()->lister_) { // Disconnected but remembered QPainter p(&pixmap); p.drawPixmap(kDeviceIconSize - kDeviceIconOverlaySize, kDeviceIconSize - kDeviceIconOverlaySize, not_connected_overlay_.pixmap(kDeviceIconOverlaySize)); @@ -404,7 +470,7 @@ void DeviceManager::PhysicalDeviceAdded(const QString &id) { info->backends_ << DeviceInfo::Backend(lister, id); // If the user hasn't saved the device in the DB yet then overwrite the device's name and icon etc. - if (info->database_id_ == -1 && info->BestBackend()->lister_ == lister) { + if (info->database_id_ == -1 && info->BestBackend() && info->BestBackend()->lister_ == lister) { info->friendly_name_ = lister->MakeFriendlyName(id); info->size_ = lister->DeviceCapacity(id); info->LoadIcon(lister->DeviceIcons(id), info->friendly_name_); @@ -414,12 +480,12 @@ void DeviceManager::PhysicalDeviceAdded(const QString &id) { } else { // It's a completely new device - beginInsertRows(ItemToIndex(root_), devices_.count(), devices_.count()); DeviceInfo *info = new DeviceInfo(DeviceInfo::Type_Device, root_); info->backends_ << DeviceInfo::Backend(lister, id); info->friendly_name_ = lister->MakeFriendlyName(id); info->size_ = lister->DeviceCapacity(id); info->LoadIcon(lister->DeviceIcons(id), info->friendly_name_); + beginInsertRows(ItemToIndex(root_), devices_.count(), devices_.count()); devices_ << info; endInsertRows(); } @@ -448,7 +514,9 @@ void DeviceManager::PhysicalDeviceRemoved(const QString &id) { } } - if (info->device_ && info->device_->lister() == lister) info->device_.reset(); + if (info->device_ && info->device_->lister() == lister) { + info->device_->Close(); + } if (!info->device_) emit DeviceDisconnected(idx); @@ -510,7 +578,7 @@ std::shared_ptr DeviceManager::Connect(DeviceInfo *info) { } if (info->BestBackend()->lister_->DeviceNeedsMount(info->BestBackend()->unique_id_)) { // Mount the device - info->BestBackend()->lister_->MountDevice(info->BestBackend()->unique_id_); + info->BestBackend()->lister_->MountDeviceAsync(info->BestBackend()->unique_id_); return ret; } @@ -597,6 +665,7 @@ std::shared_ptr DeviceManager::Connect(DeviceInfo *info) { connect(info->device_.get(), SIGNAL(TaskStarted(int)), SLOT(DeviceTaskStarted(int))); connect(info->device_.get(), SIGNAL(SongCountUpdated(int)), SLOT(DeviceSongCountUpdated(int))); connect(info->device_.get(), SIGNAL(ConnectFinished(const QString&, bool)), SLOT(DeviceConnectFinished(const QString&, bool))); + connect(info->device_.get(), SIGNAL(CloseFinished(const QString&)), SLOT(DeviceCloseFinished(const QString&))); ret->ConnectAsync(); return ret; @@ -614,7 +683,30 @@ void DeviceManager::DeviceConnectFinished(const QString &id, bool success) { emit DeviceConnected(idx); } else { - info->device_.reset(); + info->device_->Close(); + } + +} + +void DeviceManager::DeviceCloseFinished(const QString &id) { + + DeviceInfo *info = FindDeviceById(id); + if (!info) return; + + info->device_.reset(); + + QModelIndex idx = ItemToIndex(info); + if (!idx.isValid()) return; + + emit DeviceDisconnected(idx); + emit dataChanged(idx, idx); + + if (info->unmount_ && info->BestBackend() && info->BestBackend()->lister_) { + info->BestBackend()->lister_->UnmountDeviceAsync(info->BestBackend()->unique_id_); + } + + if (info->forget_) { + RemoveFromDB(info, idx); } } @@ -663,17 +755,9 @@ DeviceLister *DeviceManager::GetLister(QModelIndex idx) const { } -void DeviceManager::Disconnect(QModelIndex idx) { +void DeviceManager::Disconnect(DeviceInfo *info, QModelIndex idx) { - if (!idx.isValid()) return; - - DeviceInfo *info = IndexToItem(idx); - if (!info || !info->device_) return; - - info->device_.reset(); - - emit DeviceDisconnected(idx); - emit dataChanged(idx, idx); + info->device_->Close(); } @@ -685,7 +769,18 @@ void DeviceManager::Forget(QModelIndex idx) { if (!info) return; if (info->database_id_ == -1) return; - if (info->device_) Disconnect(idx); + + if (info->device_) { + info->forget_ = true; + Disconnect(info, idx); + } + else { + RemoveFromDB(info, idx); + } + +} + +void DeviceManager::RemoveFromDB(DeviceInfo *info, QModelIndex idx) { backend_->RemoveDevice(info->database_id_); info->database_id_ = -1; @@ -701,7 +796,6 @@ void DeviceManager::Forget(QModelIndex idx) { info->friendly_name_ = info->BestBackend()->lister_->MakeFriendlyName(id); info->LoadIcon(info->BestBackend()->lister_->DeviceIcons(id), info->friendly_name_); - dataChanged(idx, idx); } @@ -795,10 +889,13 @@ void DeviceManager::Unmount(QModelIndex idx) { if (info->database_id_ != -1 && !info->device_) return; - if (info->device_) Disconnect(idx); - - if (info->BestBackend()->lister_) - info->BestBackend()->lister_->UnmountDevice(info->BestBackend()->unique_id_); + if (info->device_) { + info->unmount_ = true; + Disconnect(info, idx); + } + else if (info->BestBackend() && info->BestBackend()->lister_) { + info->BestBackend()->lister_->UnmountDeviceAsync(info->BestBackend()->unique_id_); + } } diff --git a/src/device/devicemanager.h b/src/device/devicemanager.h index d916dfe5f..133637c88 100644 --- a/src/device/devicemanager.h +++ b/src/device/devicemanager.h @@ -104,7 +104,7 @@ class DeviceManager : public SimpleTreeModel { // Actions on devices std::shared_ptr Connect(DeviceInfo *info); std::shared_ptr Connect(QModelIndex idx); - void Disconnect(QModelIndex idx); + void Disconnect(DeviceInfo *info, QModelIndex idx); void Forget(QModelIndex idx); void UnmountAsync(QModelIndex idx); @@ -131,7 +131,11 @@ class DeviceManager : public SimpleTreeModel { void DeviceSongCountUpdated(int count); void LoadAllDevices(); void DeviceConnectFinished(const QString &id, bool success); + void DeviceCloseFinished(const QString &id); void AddDeviceFromDB(DeviceInfo *info); + void BackendClosed(); + void ListerClosed(); + void DeviceDestroyed(); protected: void LazyPopulate(DeviceInfo *item) { LazyPopulate(item, true); } @@ -144,6 +148,12 @@ class DeviceManager : public SimpleTreeModel { DeviceDatabaseBackend::Device InfoToDatabaseDevice(const DeviceInfo &info) const; + void RemoveFromDB(DeviceInfo *info, QModelIndex idx); + + void CloseDevices(); + void CloseListers(); + void CloseBackend(); + private: Application *app_; DeviceDatabaseBackend *backend_; @@ -161,6 +171,9 @@ class DeviceManager : public SimpleTreeModel { QMap active_tasks_; QThreadPool thread_pool_; + + QList wait_for_exit_; + }; template diff --git a/src/device/filesystemdevice.cpp b/src/device/filesystemdevice.cpp index 52b658029..163859269 100644 --- a/src/device/filesystemdevice.cpp +++ b/src/device/filesystemdevice.cpp @@ -46,6 +46,7 @@ FilesystemDevice::FilesystemDevice(const QUrl &url, DeviceLister *lister, const watcher_->moveToThread(watcher_thread_); watcher_thread_->start(QThread::IdlePriority); + qLog(Debug) << watcher_ << "for device" << unique_id << "moved to thread" << watcher_thread_; watcher_->set_device_name(manager->DeviceNameByID(unique_id)); watcher_->set_backend(backend_); @@ -63,16 +64,52 @@ FilesystemDevice::FilesystemDevice(const QUrl &url, DeviceLister *lister, const } -bool FilesystemDevice::Init() { - InitBackendDirectory(url_.toLocalFile(), first_time_); - model_->Init(); - return true; -} - FilesystemDevice::~FilesystemDevice() { + watcher_->Stop(); watcher_->deleteLater(); watcher_thread_->exit(); watcher_thread_->wait(); + } +bool FilesystemDevice::Init() { + + InitBackendDirectory(url_.toLocalFile(), first_time_); + model_->Init(); + return true; + +} + +void FilesystemDevice::CloseAsync() { + metaObject()->invokeMethod(this, "Close", Qt::QueuedConnection); +} + +void FilesystemDevice::Close() { + + assert(QThread::currentThread() == thread()); + + wait_for_exit_ << backend_ << watcher_; + + disconnect(backend_, 0, watcher_, 0); + disconnect(watcher_, 0, backend_, 0); + + connect(backend_, SIGNAL(ExitFinished()), this, SLOT(ExitFinished())); + connect(watcher_, SIGNAL(ExitFinished()), this, SLOT(ExitFinished())); + backend_->ExitAsync(); + watcher_->ExitAsync(); + +} + +void FilesystemDevice::ExitFinished() { + + QObject *obj = static_cast(sender()); + if (!obj) return; + disconnect(obj, 0, this, 0); + qLog(Debug) << obj << "successfully exited."; + wait_for_exit_.removeAll(obj); + if (wait_for_exit_.isEmpty()) { + emit CloseFinished(unique_id()); + } + +} diff --git a/src/device/filesystemdevice.h b/src/device/filesystemdevice.h index 1b2130096..17b0d369f 100644 --- a/src/device/filesystemdevice.h +++ b/src/device/filesystemdevice.h @@ -27,6 +27,7 @@ #include #include +#include #include #include #include @@ -51,12 +52,18 @@ public: ~FilesystemDevice(); bool Init(); + void CloseAsync(); static QStringList url_schemes() { return QStringList() << "file"; } + private slots: + void Close(); + void ExitFinished(); + private: CollectionWatcher *watcher_; QThread *watcher_thread_; + QList wait_for_exit_; }; #endif // FILESYSTEMDEVICE_H diff --git a/src/device/giolister.cpp b/src/device/giolister.cpp index e1df6ea1a..bf4428b4a 100644 --- a/src/device/giolister.cpp +++ b/src/device/giolister.cpp @@ -532,33 +532,6 @@ void GioLister::MountUnmountFinished(GObject *object, GAsyncResult *result, gpoi OperationFinished(std::bind(g_mount_unmount_with_operation_finish, _1, _2, _3), object, result); } -void GioLister::UnmountDevice(const QString &id) { - - QMutexLocker l(&mutex_); - if (!devices_.contains(id) || !devices_[id].mount || devices_[id].volume_root_uri.startsWith("mtp://")) return; - - const DeviceInfo &info = devices_[id]; - - if (!info.mount) return; - - if (info.volume) { - if (g_volume_can_eject(info.volume)) { - g_volume_eject_with_operation(info.volume, G_MOUNT_UNMOUNT_NONE, nullptr, nullptr, (GAsyncReadyCallback)VolumeEjectFinished, nullptr); - g_object_unref(info.volume); - return; - } - } - else return; - - if (g_mount_can_eject(info.mount)) { - g_mount_eject_with_operation(info.mount, G_MOUNT_UNMOUNT_NONE, nullptr, nullptr, (GAsyncReadyCallback)MountEjectFinished, nullptr); - } - else if (g_mount_can_unmount(info.mount)) { - g_mount_unmount_with_operation(info.mount, G_MOUNT_UNMOUNT_NONE, nullptr, nullptr, (GAsyncReadyCallback)MountUnmountFinished, nullptr); - } - -} - void GioLister::UpdateDeviceFreeSpace(const QString &id) { { @@ -592,14 +565,7 @@ bool GioLister::DeviceNeedsMount(const QString &id) { return devices_.contains(id) && !devices_[id].mount && !devices_[id].volume_root_uri.startsWith("mtp://"); } -int GioLister::MountDevice(const QString &id) { - - const int request_id = next_mount_request_id_++; - metaObject()->invokeMethod(this, "DoMountDevice", Qt::QueuedConnection, Q_ARG(QString, id), Q_ARG(int, request_id)); - return request_id; -} - -void GioLister::DoMountDevice(const QString &id, int request_id) { +void GioLister::MountDevice(const QString &id, const int request_id) { QMutexLocker l(&mutex_); if (!devices_.contains(id)) { @@ -619,3 +585,29 @@ void GioLister::DoMountDevice(const QString &id, int request_id) { } +void GioLister::UnmountDevice(const QString &id) { + + QMutexLocker l(&mutex_); + if (!devices_.contains(id) || !devices_[id].mount || devices_[id].volume_root_uri.startsWith("mtp://")) return; + + const DeviceInfo &info = devices_[id]; + + if (!info.mount) return; + + if (info.volume) { + if (g_volume_can_eject(info.volume)) { + g_volume_eject_with_operation(info.volume, G_MOUNT_UNMOUNT_NONE, nullptr, nullptr, (GAsyncReadyCallback)VolumeEjectFinished, nullptr); + g_object_unref(info.volume); + return; + } + } + else return; + + if (g_mount_can_eject(info.mount)) { + g_mount_eject_with_operation(info.mount, G_MOUNT_UNMOUNT_NONE, nullptr, nullptr, (GAsyncReadyCallback)MountEjectFinished, nullptr); + } + else if (g_mount_can_unmount(info.mount)) { + g_mount_unmount_with_operation(info.mount, G_MOUNT_UNMOUNT_NONE, nullptr, nullptr, (GAsyncReadyCallback)MountUnmountFinished, nullptr); + } + +} diff --git a/src/device/giolister.h b/src/device/giolister.h index 93c70fd4f..a6060abf5 100644 --- a/src/device/giolister.h +++ b/src/device/giolister.h @@ -67,10 +67,9 @@ class GioLister : public DeviceLister { QString MakeFriendlyName(const QString &id); QList MakeDeviceUrls(const QString &id); - int MountDevice(const QString &id); - void UnmountDevice(const QString &id); - public slots: + void MountDevice(const QString &id, const int request_id); + void UnmountDevice(const QString &id); void UpdateDeviceFreeSpace(const QString &id); protected: @@ -140,9 +139,6 @@ class GioLister : public DeviceLister { template T LockAndGetDeviceInfo(const QString &id, T DeviceInfo::*field); - private slots: - void DoMountDevice(const QString &id, int request_id); - private: ScopedGObject monitor_; QList signals_; diff --git a/src/device/gpoddevice.cpp b/src/device/gpoddevice.cpp index b1852cea7..580159107 100644 --- a/src/device/gpoddevice.cpp +++ b/src/device/gpoddevice.cpp @@ -47,9 +47,10 @@ class DeviceManager; GPodDevice::GPodDevice(const QUrl &url, DeviceLister *lister, const QString &unique_id, DeviceManager *manager, Application *app, int database_id, bool first_time) : ConnectedDevice(url, lister, unique_id, manager, app, database_id, first_time), - loader_thread_(new QThread()), loader_(nullptr), - db_(nullptr) {} + loader_thread_(nullptr), + db_(nullptr), + closing_(false) {} bool GPodDevice::Init() { @@ -57,6 +58,7 @@ bool GPodDevice::Init() { model_->Init(); loader_ = new GPodLoader(url_.path(), app_->task_manager(), backend_, shared_from_this()); + loader_thread_ = new QThread(); loader_->moveToThread(loader_thread_); connect(loader_, SIGNAL(Error(QString)), SLOT(LoaderError(QString))); @@ -82,6 +84,19 @@ void GPodDevice::ConnectAsync() { } +void GPodDevice::Close() { + + closing_ = true; + + if (IsLoading()) { + loader_->Abort(); + } + else { + ConnectedDevice::Close(); + } + +} + void GPodDevice::LoadFinished(Itdb_iTunesDB *db, bool success) { QMutexLocker l(&db_mutex_); @@ -98,7 +113,12 @@ void GPodDevice::LoadFinished(Itdb_iTunesDB *db, bool success) { loader_->deleteLater(); loader_ = nullptr; - emit ConnectFinished(unique_id_, success); + if (closing_) { + ConnectedDevice::Close(); + } + else { + emit ConnectFinished(unique_id_, success); + } } @@ -232,6 +252,7 @@ void GPodDevice::WriteDatabase(bool success) { } } + // This is done in the organise thread so close the unique DB connection. backend_->Close(); songs_to_add_.clear(); diff --git a/src/device/gpoddevice.h b/src/device/gpoddevice.h index 084b7ed06..9a125214e 100644 --- a/src/device/gpoddevice.h +++ b/src/device/gpoddevice.h @@ -38,11 +38,11 @@ #include "core/song.h" #include "core/musicstorage.h" #include "connecteddevice.h" +#include "gpodloader.h" class Application; class DeviceLister; class DeviceManager; -class GPodLoader; class GPodDevice : public ConnectedDevice, public virtual MusicStorage { Q_OBJECT @@ -59,6 +59,9 @@ class GPodDevice : public ConnectedDevice, public virtual MusicStorage { bool Init(); void ConnectAsync(); + void Close(); + bool IsLoading() { return loader_; } + QObject *Loader() { return loader_; } static QStringList url_schemes() { return QStringList() << "ipod"; } @@ -86,12 +89,13 @@ class GPodDevice : public ConnectedDevice, public virtual MusicStorage { void WriteDatabase(bool success); protected: - QThread *loader_thread_; GPodLoader *loader_; + QThread *loader_thread_; QWaitCondition db_wait_cond_; QMutex db_mutex_; Itdb_iTunesDB *db_; + bool closing_; QMutex db_busy_; SongList songs_to_add_; diff --git a/src/device/gpodloader.cpp b/src/device/gpodloader.cpp index 8aa2a4bae..6bb2d5791 100644 --- a/src/device/gpodloader.cpp +++ b/src/device/gpodloader.cpp @@ -41,7 +41,8 @@ GPodLoader::GPodLoader(const QString &mount_point, TaskManager *task_manager, Co mount_point_(mount_point), type_(Song::FileType_Unknown), task_manager_(task_manager), - backend_(backend) { + backend_(backend), + abort_(false) { original_thread_ = thread(); } @@ -57,7 +58,7 @@ void GPodLoader::LoadDatabase() { moveToThread(original_thread_); task_manager_->SetTaskFinished(task_id); - emit LoadFinished(db, db); + emit LoadFinished(db, !abort_); } @@ -86,6 +87,9 @@ Itdb_iTunesDB *GPodLoader::TryLoad() { SongList songs; for (GList *tracks = db->tracks; tracks != nullptr; tracks = tracks->next) { + + if (abort_) break; + Itdb_Track *track = static_cast(tracks->data); Song song(Song::Source_Device); @@ -99,9 +103,12 @@ Itdb_iTunesDB *GPodLoader::TryLoad() { // Need to remove all the existing songs in the database first backend_->DeleteSongs(backend_->FindSongsInDirectory(1)); - // Add the songs we've just loaded - backend_->AddOrUpdateSongs(songs); + if (!abort_) { + // Add the songs we've just loaded + backend_->AddOrUpdateSongs(songs); + } + // This is done in the loader thread so close the unique DB connection. backend_->Close(); return db; diff --git a/src/device/gpodloader.h b/src/device/gpodloader.h index 9718e839a..5680a731f 100644 --- a/src/device/gpodloader.h +++ b/src/device/gpodloader.h @@ -46,6 +46,8 @@ class GPodLoader : public QObject { void set_music_path_prefix(const QString &prefix) { path_prefix_ = prefix; } void set_song_type(Song::FileType type) { type_ = type; } + void Abort() { abort_ = true; } + public slots: void LoadDatabase(); @@ -66,6 +68,8 @@ class GPodLoader : public QObject { Song::FileType type_; TaskManager *task_manager_; CollectionBackend *backend_; + bool abort_; + }; #endif // GPODLOADER_H diff --git a/src/device/ilister.cpp b/src/device/ilister.cpp index 295bb1d51..89bb9302e 100644 --- a/src/device/ilister.cpp +++ b/src/device/ilister.cpp @@ -31,11 +31,9 @@ #include "ilister.h" #include "imobiledeviceconnection.h" -iLister::iLister() { -} +iLister::iLister() {} -iLister::~iLister() { -} +iLister::~iLister() {} bool iLister::Init() { idevice_event_subscribe(&EventCallback, reinterpret_cast(this)); @@ -191,8 +189,6 @@ QList iLister::MakeDeviceUrls(const QString &id) { } -void iLister::UnmountDevice(const QString &id) { } - iLister::DeviceInfo iLister::ReadDeviceInfo(const char *uuid) { DeviceInfo ret; diff --git a/src/device/ilister.h b/src/device/ilister.h index a85fcaa7f..78f833421 100644 --- a/src/device/ilister.h +++ b/src/device/ilister.h @@ -52,7 +52,6 @@ class iLister : public DeviceLister { virtual QVariantMap DeviceHardwareInfo(const QString &id); virtual QString MakeFriendlyName(const QString &id); virtual QList MakeDeviceUrls(const QString &id); - virtual void UnmountDevice(const QString &id); public slots: virtual void UpdateDeviceFreeSpace(const QString &id); diff --git a/src/device/macosdevicelister.h b/src/device/macosdevicelister.h index 7ad709e4d..38a359ddf 100644 --- a/src/device/macosdevicelister.h +++ b/src/device/macosdevicelister.h @@ -35,7 +35,6 @@ class MacOsDeviceLister : public DeviceLister { virtual QString MakeFriendlyName(const QString &id); virtual QList MakeDeviceUrls(const QString &id); - virtual void UnmountDevice(const QString &id); virtual void UpdateDeviceFreeSpace(const QString &id); struct MTPDevice { @@ -54,6 +53,7 @@ class MacOsDeviceLister : public DeviceLister { }; public slots: + virtual void UnmountDevice(const QString &id); virtual void ShutDown(); private: diff --git a/src/device/mtpdevice.cpp b/src/device/mtpdevice.cpp index 3226eb1bc..76053f009 100644 --- a/src/device/mtpdevice.cpp +++ b/src/device/mtpdevice.cpp @@ -51,7 +51,10 @@ class DeviceManager; bool MtpDevice::sInitialisedLibMTP = false; MtpDevice::MtpDevice(const QUrl &url, DeviceLister *lister, const QString &unique_id, DeviceManager *manager, Application *app, int database_id, bool first_time) - : ConnectedDevice(url, lister, unique_id, manager, app, database_id, first_time), loader_thread_(new QThread()), loader_(nullptr) { + : ConnectedDevice(url, lister, unique_id, manager, app, database_id, first_time), + loader_(nullptr), + loader_thread_(nullptr), + closing_(false) { if (!sInitialisedLibMTP) { LIBMTP_Init(); @@ -61,6 +64,7 @@ MtpDevice::MtpDevice(const QUrl &url, DeviceLister *lister, const QString &uniqu } MtpDevice::~MtpDevice() { + if (loader_) { loader_thread_->exit(); loader_->deleteLater(); @@ -68,6 +72,7 @@ MtpDevice::~MtpDevice() { db_busy_.unlock(); loader_thread_->deleteLater(); } + } bool MtpDevice::Init() { @@ -75,25 +80,19 @@ bool MtpDevice::Init() { InitBackendDirectory("/", first_time_, false); model_->Init(); - loader_ = new MtpLoader(url_, app_->task_manager(), backend_, shared_from_this()); - + loader_ = new MtpLoader(url_, app_->task_manager(), backend_); + loader_thread_ = new QThread(); loader_->moveToThread(loader_thread_); connect(loader_, SIGNAL(Error(QString)), SLOT(LoaderError(QString))); connect(loader_, SIGNAL(TaskStarted(int)), SIGNAL(TaskStarted(int))); - connect(loader_, SIGNAL(LoadFinished(bool)), SLOT(LoadFinished(bool))); + connect(loader_, SIGNAL(LoadFinished(bool, MtpConnection*)), SLOT(LoadFinished(bool, MtpConnection*))); connect(loader_thread_, SIGNAL(started()), loader_, SLOT(LoadDatabase())); return true; } -void MtpDevice::NewConnection() { - - connection_.reset(new MtpConnection(url_)); - -} - void MtpDevice::ConnectAsync() { db_busy_.lock(); @@ -101,13 +100,33 @@ void MtpDevice::ConnectAsync() { } -void MtpDevice::LoadFinished(bool success) { +void MtpDevice::Close() { + + closing_ = true; + + if (IsLoading()) { + loader_->Abort(); + } + else { + ConnectedDevice::Close(); + } + +} + +void MtpDevice::LoadFinished(bool success, MtpConnection *connection) { + + connection_.reset(connection); loader_thread_->exit(); loader_->deleteLater(); loader_ = nullptr; db_busy_.unlock(); - emit ConnectFinished(unique_id_, success); + if (closing_) { + ConnectedDevice::Close(); + } + else { + emit ConnectFinished(unique_id_, success); + } } @@ -120,9 +139,7 @@ bool MtpDevice::StartCopy(QList *supported_types) { // Ensure only one "organise files" can be active at any one time db_busy_.lock(); - // Connect to the device - if (!connection_.get() || !connection_->is_valid()) NewConnection(); - if (!connection_.get() || !connection_->is_valid()) return false; + if (!connection_ || !connection_->is_valid()) return false; // Did the caller want a list of supported types? if (supported_types) { @@ -147,7 +164,7 @@ static int ProgressCallback(uint64_t const sent, uint64_t const total, void cons bool MtpDevice::CopyToStorage(const CopyJob &job) { - if (!connection_.get() || !connection_->is_valid()) return false; + if (!connection_ || !connection_->is_valid()) return false; // Convert metadata LIBMTP_track_t track; @@ -184,6 +201,7 @@ void MtpDevice::FinishCopy(bool success) { songs_to_add_.clear(); songs_to_remove_.clear(); + // This is done in the organise thread so close the unique DB connection. backend_->Close(); db_busy_.unlock(); diff --git a/src/device/mtpdevice.h b/src/device/mtpdevice.h index 66be34546..c68a8b7ff 100644 --- a/src/device/mtpdevice.h +++ b/src/device/mtpdevice.h @@ -37,13 +37,13 @@ #include "core/song.h" #include "connecteddevice.h" +#include "mtploader.h" class Application; class DeviceLister; class DeviceManager; class DeviceConnection; class MtpConnection; -class MtpLoader; struct LIBMTP_mtpdevice_struct; class MtpDevice : public ConnectedDevice { @@ -56,8 +56,9 @@ class MtpDevice : public ConnectedDevice { static QStringList url_schemes() { return QStringList() << "mtp" << "gphoto2"; } bool Init(); - void NewConnection(); void ConnectAsync(); + void Close(); + bool IsLoading() { return loader_; } bool GetSupportedFiletypes(QList* ret); int GetFreeSpace(); @@ -71,10 +72,8 @@ class MtpDevice : public ConnectedDevice { bool DeleteFromStorage(const DeleteJob& job); void FinishDelete(bool success); - MtpConnection *connection() { return connection_.get(); } - private slots: - void LoadFinished(bool success); + void LoadFinished(bool success, MtpConnection *connection); void LoaderError(const QString& message); private: @@ -85,14 +84,15 @@ class MtpDevice : public ConnectedDevice { private: static bool sInitialisedLibMTP; - QThread *loader_thread_; MtpLoader *loader_; + QThread *loader_thread_; + bool closing_; QMutex db_busy_; SongList songs_to_add_; SongList songs_to_remove_; - std::shared_ptr connection_; + std::unique_ptr connection_; }; diff --git a/src/device/mtploader.cpp b/src/device/mtploader.cpp index 40c7112a1..445d610aa 100644 --- a/src/device/mtploader.cpp +++ b/src/device/mtploader.cpp @@ -35,12 +35,12 @@ #include "mtpconnection.h" #include "mtploader.h" -MtpLoader::MtpLoader(const QUrl &url, TaskManager *task_manager, CollectionBackend *backend, std::shared_ptr device) +MtpLoader::MtpLoader(const QUrl &url, TaskManager *task_manager, CollectionBackend *backend) : QObject(nullptr), url_(url), task_manager_(task_manager), backend_(backend), - device_(device) { + abort_(false) { original_thread_ = thread(); } @@ -58,26 +58,26 @@ void MtpLoader::LoadDatabase() { moveToThread(original_thread_); task_manager_->SetTaskFinished(task_id); - emit LoadFinished(success); + emit LoadFinished(success, connection_.release()); } bool MtpLoader::TryLoad() { - MtpDevice *device = dynamic_cast(device_.get()); // FIXME + connection_.reset(new MtpConnection(url_)); - if (!device->connection() || !device->connection()->is_valid()) - device->NewConnection(); - - if (!device->connection() || !device->connection()->is_valid()) { + if (!connection_ || !connection_->is_valid()) { emit Error(tr("Error connecting MTP device %1").arg(url_.toString())); return false; } // Load the list of songs on the device SongList songs; - LIBMTP_track_t* tracks = LIBMTP_Get_Tracklisting_With_Callback(device->connection()->device(), nullptr, nullptr); + LIBMTP_track_t* tracks = LIBMTP_Get_Tracklisting_With_Callback(connection_->device(), nullptr, nullptr); while (tracks) { + + if (abort_) break; + LIBMTP_track_t *track = tracks; Song song(Song::Source_Device); @@ -90,15 +90,18 @@ bool MtpLoader::TryLoad() { LIBMTP_destroy_track_t(track); } - // Need to remove all the existing songs in the database first - backend_->DeleteSongs(backend_->FindSongsInDirectory(1)); + if (!abort_) { + // Need to remove all the existing songs in the database first + backend_->DeleteSongs(backend_->FindSongsInDirectory(1)); - // Add the songs we've just loaded - backend_->AddOrUpdateSongs(songs); + // Add the songs we've just loaded + backend_->AddOrUpdateSongs(songs); + } + // This is done in the loader thread so close the unique DB connection. backend_->Close(); - return true; + return !abort_; } diff --git a/src/device/mtploader.h b/src/device/mtploader.h index 6262500f6..c298dfdf3 100644 --- a/src/device/mtploader.h +++ b/src/device/mtploader.h @@ -32,6 +32,8 @@ #include #include +//#include "mtpconnection.h" + class TaskManager; class CollectionBackend; class ConnectedDevice; @@ -42,10 +44,11 @@ class MtpLoader : public QObject { Q_OBJECT public: - MtpLoader(const QUrl &url, TaskManager *task_manager, CollectionBackend *backend, std::shared_ptr device); + MtpLoader(const QUrl &url, TaskManager *task_manager, CollectionBackend *backend); ~MtpLoader(); bool Init(); + void Abort() { abort_ = true; } public slots: void LoadDatabase(); @@ -53,7 +56,7 @@ class MtpLoader : public QObject { signals: void Error(const QString &message); void TaskStarted(int task_id); - void LoadFinished(bool success); + void LoadFinished(bool success, MtpConnection*); private: bool TryLoad(); @@ -62,9 +65,9 @@ class MtpLoader : public QObject { QUrl url_; TaskManager *task_manager_; CollectionBackend *backend_; - std::shared_ptr device_; - std::shared_ptr connection_; + std::unique_ptr connection_; QThread *original_thread_; + bool abort_; }; diff --git a/src/device/udisks2lister.h b/src/device/udisks2lister.h index 4c4d57996..c17f011ec 100644 --- a/src/device/udisks2lister.h +++ b/src/device/udisks2lister.h @@ -65,9 +65,8 @@ class Udisks2Lister : public DeviceLister { QString MakeFriendlyName(const QString &id) override; QList MakeDeviceUrls(const QString &id) override; - void UnmountDevice(const QString &id) override; - public slots: + void UnmountDevice(const QString &id) override; void UpdateDeviceFreeSpace(const QString &id) override; protected: diff --git a/src/playlist/playlistbackend.cpp b/src/playlist/playlistbackend.cpp index 666ae624d..4a0d81b7d 100644 --- a/src/playlist/playlistbackend.cpp +++ b/src/playlist/playlistbackend.cpp @@ -84,7 +84,6 @@ void PlaylistBackend::Exit() { assert(QThread::currentThread() == thread()); - Close(); moveToThread(original_thread_); emit ExitFinished();