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

@@ -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());

src/collection/collectionbackend.cpp

@@ -91,7 +91,6 @@ void CollectionBackend::Exit() {
 
   assert(QThread::currentThread() == thread());
 
-  Close();
   moveToThread(original_thread_);
   emit ExitFinished();
 

src/collection/collectionmodel.cpp

@@ -130,7 +130,6 @@ CollectionModel::CollectionModel(CollectionBackend *backend, Application *app, Q
 }
 
 CollectionModel::~CollectionModel() {
-  backend_->Close();
   delete root_;
 }
 

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() {

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_);

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);

src/device/deviceinfo.h

@@ -68,7 +68,9 @@ class DeviceInfo : public SimpleTreeItem<DeviceInfo> {
         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<DeviceInfo>(type, parent),
@@ -76,7 +78,9 @@ class DeviceInfo : public SimpleTreeItem<DeviceInfo> {
         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<DeviceInfo> {
 
   int task_percentage_;
 
+  bool unmount_;
+  bool forget_;
+
 };
 
 #endif  // DEVICEINFO_H

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;
-
-}
-

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
+  // When connecting to a device for the first time, do we want an user's confirmation for scanning it? (by default yes)
-  // confirmation for scanning it? (by default yes)
   virtual bool AskForScan(const QString&) const { return true; }
 
   virtual QString MakeFriendlyName(const QString &id) = 0;
   virtual QList<QUrl> MakeDeviceUrls(const QString &id) = 0;
 
-  // Ensure the device is mounted.  This should run asynchronously and emit
+  // Ensure the device is mounted.  This should run asynchronously and emit DeviceMounted when it's done.
-  // DeviceMounted when it's done.
+  virtual int MountDeviceAsync(const QString &id);
-  virtual int MountDevice(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:

src/device/devicemanager.cpp

@@ -157,21 +157,86 @@ DeviceManager::~DeviceManager() {
     lister->ShutDown();
     delete lister;
   }
-
+  listers_.clear();
-  backend_->Close();
 
   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<QObject*>(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<DeviceLister*>(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<ConnectedDevice*>(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<ConnectedDevice> 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<ConnectedDevice> 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;
+  info->device_->Close();
-
-  DeviceInfo *info = IndexToItem(idx);
-  if (!info || !info->device_) return;
-
-  info->device_.reset();
-
-  emit DeviceDisconnected(idx);
-  emit dataChanged(idx, idx);
 
 }
 
@@ -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->device_) {
-
+    info->unmount_ = true;
-  if (info->BestBackend()->lister_)
+    Disconnect(info, idx);
-    info->BestBackend()->lister_->UnmountDevice(info->BestBackend()->unique_id_);
+  }
+  else if (info->BestBackend() && info->BestBackend()->lister_) {
+    info->BestBackend()->lister_->UnmountDeviceAsync(info->BestBackend()->unique_id_);
+  }
 
 }
 

src/device/devicemanager.h

@@ -104,7 +104,7 @@ class DeviceManager : public SimpleTreeModel<DeviceInfo> {
   // Actions on devices
   std::shared_ptr<ConnectedDevice> Connect(DeviceInfo *info);
   std::shared_ptr<ConnectedDevice> 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<DeviceInfo> {
   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<DeviceInfo> {
 
   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<DeviceInfo> {
   QMap<int, QPersistentModelIndex> active_tasks_;
 
   QThreadPool thread_pool_;
+
+  QList<QObject*> wait_for_exit_;
+
 };
 
 template <typename T>

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<QObject*>(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());
+  }
+
+}

src/device/filesystemdevice.h

@@ -27,6 +27,7 @@
 
 #include <QObject>
 #include <QThread>
+#include <QList>
 #include <QString>
 #include <QStringList>
 #include <QUrl>
@@ -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<QObject*> wait_for_exit_;
 };
 
 #endif // FILESYSTEMDEVICE_H

src/device/giolister.cpp

@@ -532,33 +532,6 @@ void GioLister::MountUnmountFinished(GObject *object, GAsyncResult *result, gpoi
   OperationFinished<GMount>(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) {
+void GioLister::MountDevice(const QString &id, const int request_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) {
 
   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);
+  }
+
+}

src/device/giolister.h

@@ -67,10 +67,9 @@ class GioLister : public DeviceLister {
   QString MakeFriendlyName(const QString &id);
   QList<QUrl> 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 <typename T>
   T LockAndGetDeviceInfo(const QString &id, T DeviceInfo::*field);
 
- private slots:
-  void DoMountDevice(const QString &id, int request_id);
-
  private:
   ScopedGObject<GVolumeMonitor> monitor_;
   QList<gulong> signals_;

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();

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_;

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<Itdb_Track*>(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
+  if (!abort_) {
-  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 db;

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

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<void*>(this));
@@ -191,8 +189,6 @@ QList<QUrl> iLister::MakeDeviceUrls(const QString &id) {
 
 }
 
-void iLister::UnmountDevice(const QString &id) { }
-
 iLister::DeviceInfo iLister::ReadDeviceInfo(const char *uuid) {
 
   DeviceInfo ret;

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<QUrl> MakeDeviceUrls(const QString &id);
-  virtual void UnmountDevice(const QString &id);
 
  public slots:
   virtual void UpdateDeviceFreeSpace(const QString &id);

src/device/macosdevicelister.h

@@ -35,7 +35,6 @@ class MacOsDeviceLister : public DeviceLister {
   virtual QString MakeFriendlyName(const QString &id);
   virtual QList<QUrl> 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:

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<Song::FileType> *supported_types) {
   // Ensure only one "organise files" can be active at any one time
   db_busy_.lock();
 
-  // Connect to the device
+  if (!connection_ || !connection_->is_valid()) return false;
-  if (!connection_.get() || !connection_->is_valid()) NewConnection();
-  if (!connection_.get() || !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();

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<Song::FileType>* 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<MtpConnection> connection_;
+  std::unique_ptr<MtpConnection> connection_;
 
 };
 

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<ConnectedDevice> 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<MtpDevice*>(device_.get()); // FIXME
+  connection_.reset(new MtpConnection(url_));
 
-  if (!device->connection() || !device->connection()->is_valid())
+  if (!connection_ || !connection_->is_valid()) {
-    device->NewConnection();
-
-  if (!device->connection() || !device->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
+  if (!abort_) {
-  backend_->DeleteSongs(backend_->FindSongsInDirectory(1));
+    // Need to remove all the existing songs in the database first
+    backend_->DeleteSongs(backend_->FindSongsInDirectory(1));
 
-  // Add the songs we've just loaded
+    // Add the songs we've just loaded
-  backend_->AddOrUpdateSongs(songs);
+    backend_->AddOrUpdateSongs(songs);
+  }
 
+  // This is done in the loader thread so close the unique DB connection.
   backend_->Close();
 
-  return true;
+  return !abort_;
 
 }
 

src/device/mtploader.h

@@ -32,6 +32,8 @@
 #include <QString>
 #include <QUrl>
 
+//#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<ConnectedDevice> 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<ConnectedDevice> device_;
+  std::unique_ptr<MtpConnection> connection_;
-  std::shared_ptr<MtpConnection> connection_;
   QThread *original_thread_;
+  bool abort_;
 
 };
 

src/device/udisks2lister.h

@@ -65,9 +65,8 @@ class Udisks2Lister : public DeviceLister {
   QString MakeFriendlyName(const QString &id) override;
   QList<QUrl> 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:

src/playlist/playlistbackend.cpp

@@ -84,7 +84,6 @@ void PlaylistBackend::Exit() {
 
   assert(QThread::currentThread() == thread());
 
-  Close();
   moveToThread(original_thread_);
   emit ExitFinished();