Don't send messages to a handler in the time between its socket closing and its process ending. Fixes a race condition that would result in a tag loading operation never completing. Fixes issue 2616

ext/clementine-spotifyblob/spotifyclient.cpp

@@ -963,8 +963,8 @@ void SpotifyClient::AlbumBrowseComplete(sp_albumbrowse* result, void* userdata)
   sp_albumbrowse_release(result);
 }
 
-void SpotifyClient::SocketClosed() {
-  AbstractMessageHandler<pb::spotify::Message>::SocketClosed();
+void SpotifyClient::DeviceClosed() {
+  AbstractMessageHandler<pb::spotify::Message>::DeviceClosed();
 
   qApp->exit();
 }

ext/clementine-spotifyblob/spotifyclient.h

@@ -50,7 +50,7 @@ public:
 
 protected:
   void MessageArrived(const pb::spotify::Message& message);
-  void SocketClosed();
+  void DeviceClosed();
 
 private slots:
   void ProcessEvents();

ext/clementine-tagreader/tagreaderworker.cpp

@@ -559,8 +559,8 @@ QByteArray TagReaderWorker::LoadEmbeddedArt(const QString& filename) const {
   return QByteArray();
 }
 
-void TagReaderWorker::SocketClosed() {
-  AbstractMessageHandler<pb::tagreader::Message>::SocketClosed();
+void TagReaderWorker::DeviceClosed() {
+  AbstractMessageHandler<pb::tagreader::Message>::DeviceClosed();
 
   qApp->exit();
 }

ext/clementine-tagreader/tagreaderworker.h

@@ -41,7 +41,7 @@ public:
 
 protected:
   void MessageArrived(const pb::tagreader::Message& message);
-  void SocketClosed();
+  void DeviceClosed();
 
 private:
   void ReadFile(const QString& filename, pb::tagreader::SongMetadata* song) const;

ext/libclementine-common/core/messagehandler.cpp

@@ -31,7 +31,8 @@ _MessageHandlerBase::_MessageHandlerBase(QIODevice* device, QObject* parent)
     flush_abstract_socket_(NULL),
     flush_local_socket_(NULL),
     reading_protobuf_(false),
-    expected_length_(0) {
+    expected_length_(0),
+    is_device_closed_(false) {
   if (device) {
     SetDevice(device);
   }
@@ -47,10 +48,10 @@ void _MessageHandlerBase::SetDevice(QIODevice* device) {
   // Yeah I know.
   if (QAbstractSocket* socket = qobject_cast<QAbstractSocket*>(device)) {
     flush_abstract_socket_ = &QAbstractSocket::flush;
-    connect(socket, SIGNAL(disconnected()), SLOT(SocketClosed()));
+    connect(socket, SIGNAL(disconnected()), SLOT(DeviceClosed()));
   } else if (QLocalSocket* socket = qobject_cast<QLocalSocket*>(device)) {
     flush_local_socket_ = &QLocalSocket::flush;
-    connect(socket, SIGNAL(disconnected()), SLOT(SocketClosed()));
+    connect(socket, SIGNAL(disconnected()), SLOT(DeviceClosed()));
   } else {
     qFatal("Unsupported device type passed to _MessageHandlerBase");
   }
@@ -100,4 +101,7 @@ void _MessageHandlerBase::WriteMessage(const QByteArray& data) {
   }
 }
 
+void _MessageHandlerBase::DeviceClosed() {
+  is_device_closed_ = true;
+  AbortAll();
 }

ext/libclementine-common/core/messagehandler.h

@@ -56,13 +56,17 @@ public:
 
   void SetDevice(QIODevice* device);
 
+  // After this is true, messages cannot be sent to the handler any more.
+  bool is_device_closed() const { return is_device_closed_; }
+
 protected slots:
   void WriteMessage(const QByteArray& data);
   void DeviceReadyRead();
-  virtual void SocketClosed() {}
+  virtual void DeviceClosed();
 
 protected:
   virtual bool RawMessageArrived(const QByteArray& data) = 0;
+  virtual void AbortAll() = 0;
 
 protected:
   typedef bool (QAbstractSocket::*FlushAbstractSocket)();
@@ -75,6 +79,8 @@ protected:
   bool reading_protobuf_;
   quint32 expected_length_;
   QBuffer buffer_;
+
+  bool is_device_closed_;
 };
 
 
@@ -85,6 +91,7 @@ template <typename MT>
 class AbstractMessageHandler : public _MessageHandlerBase {
 public:
   AbstractMessageHandler(QIODevice* device, QObject* parent);
+  ~AbstractMessageHandler() { AbortAll(); }
 
   typedef MT MessageType;
   typedef MessageReply<MT> ReplyType;
@@ -112,7 +119,7 @@ protected:
 
   // _MessageHandlerBase
   bool RawMessageArrived(const QByteArray& data);
-  void SocketClosed();
+  void AbortAll();
 
 private:
   QMap<int, ReplyType*> pending_replies_;
@@ -174,9 +181,7 @@ bool AbstractMessageHandler<MT>::RawMessageArrived(const QByteArray& data) {
 }
 
 template<typename MT>
-void AbstractMessageHandler<MT>::SocketClosed() {
-  QMutexLocker l(&mutex_);
-
+void AbstractMessageHandler<MT>::AbortAll() {
   foreach (ReplyType* reply, pending_replies_) {
     reply->Abort();
   }

ext/libclementine-common/core/workerpool.h

@@ -390,7 +390,8 @@ HandlerType* WorkerPool<HandlerType>::NextHandler() const {
   for (int i=0 ; i<workers_.count() ; ++i) {
     const int worker_index = (next_worker_ + i) % workers_.count();
 
-    if (workers_[worker_index].handler_) {
+    if (workers_[worker_index].handler_ &&
+        !workers_[worker_index].handler_->is_device_closed()) {
       next_worker_ = (worker_index + 1) % workers_.count();
       return workers_[worker_index].handler_;
     }