From 3e0f252b34a5fba00b5cd446146382791d4a2412 Mon Sep 17 00:00:00 2001 From: David Sansome Date: Sun, 8 Jan 2012 16:34:34 +0000 Subject: [PATCH] Exit worker processes when their sockets are closed, and make sure the main app closes sockets when exiting - fixes a crash dialog on Windows. --- ext/clementine-spotifyblob/spotifyclient.cpp | 6 ++++ ext/clementine-spotifyblob/spotifyclient.h | 1 + ext/clementine-tagreader/tagreaderworker.cpp | 7 ++++ ext/clementine-tagreader/tagreaderworker.h | 1 + ext/libclementine-common/core/workerpool.h | 38 ++++++++++++++++---- src/main.cpp | 10 ++++-- 6 files changed, 53 insertions(+), 10 deletions(-) diff --git a/ext/clementine-spotifyblob/spotifyclient.cpp b/ext/clementine-spotifyblob/spotifyclient.cpp index a87c25b2b..3bae1c4ed 100644 --- a/ext/clementine-spotifyblob/spotifyclient.cpp +++ b/ext/clementine-spotifyblob/spotifyclient.cpp @@ -962,3 +962,9 @@ void SpotifyClient::AlbumBrowseComplete(sp_albumbrowse* result, void* userdata) me->SendMessage(message); sp_albumbrowse_release(result); } + +void SpotifyClient::SocketClosed() { + AbstractMessageHandler::SocketClosed(); + + qApp->exit(); +} diff --git a/ext/clementine-spotifyblob/spotifyclient.h b/ext/clementine-spotifyblob/spotifyclient.h index 22f5af377..5ddd7c1c6 100644 --- a/ext/clementine-spotifyblob/spotifyclient.h +++ b/ext/clementine-spotifyblob/spotifyclient.h @@ -50,6 +50,7 @@ public: protected: void MessageArrived(const pb::spotify::Message& message); + void SocketClosed(); private slots: void ProcessEvents(); diff --git a/ext/clementine-tagreader/tagreaderworker.cpp b/ext/clementine-tagreader/tagreaderworker.cpp index abc768bd7..f5f08bd94 100644 --- a/ext/clementine-tagreader/tagreaderworker.cpp +++ b/ext/clementine-tagreader/tagreaderworker.cpp @@ -21,6 +21,7 @@ #include "core/logging.h" #include "core/timeconstants.h" +#include #include #include #include @@ -549,3 +550,9 @@ QByteArray TagReaderWorker::LoadEmbeddedArt(const QString& filename) const { return QByteArray(); } + +void TagReaderWorker::SocketClosed() { + AbstractMessageHandler::SocketClosed(); + + qApp->exit(); +} diff --git a/ext/clementine-tagreader/tagreaderworker.h b/ext/clementine-tagreader/tagreaderworker.h index 8bccd7fa8..acb2a1f4e 100644 --- a/ext/clementine-tagreader/tagreaderworker.h +++ b/ext/clementine-tagreader/tagreaderworker.h @@ -41,6 +41,7 @@ public: protected: void MessageArrived(const pb::tagreader::Message& message); + void SocketClosed(); private: void ReadFile(const QString& filename, pb::tagreader::SongMetadata* song) const; diff --git a/ext/libclementine-common/core/workerpool.h b/ext/libclementine-common/core/workerpool.h index 299fa24b8..ce1dad62c 100644 --- a/ext/libclementine-common/core/workerpool.h +++ b/ext/libclementine-common/core/workerpool.h @@ -63,6 +63,7 @@ template class WorkerPool : public _WorkerPoolBase { public: WorkerPool(QObject* parent = 0); + ~WorkerPool(); // Sets the name of the worker executable. This is looked for first in the // current directory, and then in $PATH. You must call this before calling @@ -81,9 +82,8 @@ public: // Starts all workers. void Start(); - // Returns a handler in a round-robin fashion. May return NULL if no handlers - // are running yet, in which case you must queue the request yourself and - // re-send it when the WorkerConnected() signal is emitted. + // Returns a handler in a round-robin fashion. Will block if no handlers are + // available yet. HandlerType* NextHandler(); protected: @@ -93,9 +93,11 @@ protected: private: struct Worker { - Worker() : local_server_(NULL), process_(NULL), handler_(NULL) {} + Worker() : local_server_(NULL), local_socket_(NULL), process_(NULL), + handler_(NULL) {} QLocalServer* local_server_; + QLocalSocket* local_socket_; QProcess* process_; HandlerType* handler_; }; @@ -144,6 +146,27 @@ WorkerPool::WorkerPool(QObject* parent) local_server_name_ = "workerpool"; } +template +WorkerPool::~WorkerPool() { + foreach (const Worker& worker, workers_) { + if (worker.local_socket_ && worker.process_) { + // The worker is connected. Close his socket and wait for him to exit. + qLog(Debug) << "Closing worker socket"; + worker.local_socket_->close(); + worker.process_->waitForFinished(500); + } + + if (worker.process_ && worker.process_->state() == QProcess::Running) { + // The worker is still running - kill it. + qLog(Debug) << "Killing worker process"; + worker.process_->terminate(); + if (!worker.process_->waitForFinished(500)) { + worker.process_->kill(); + } + } + } +} + template void WorkerPool::SetWorkerCount(int count) { Q_ASSERT(workers_.isEmpty()); @@ -201,6 +224,7 @@ void WorkerPool::DoStart() { template void WorkerPool::StartOneWorker(Worker* worker) { DeleteQObjectPointerLater(&worker->local_server_); + DeleteQObjectPointerLater(&worker->local_socket_); DeleteQObjectPointerLater(&worker->process_); DeleteQObjectPointerLater(&worker->handler_); @@ -242,15 +266,15 @@ void WorkerPool::NewConnection() { qLog(Debug) << "Worker connected to" << server->fullServerName(); // Accept the connection. - QLocalSocket* socket = server->nextPendingConnection(); + worker->local_socket_ = server->nextPendingConnection(); // We only ever accept one connection per worker, so destroy the server now. - socket->setParent(this); + worker->local_socket_->setParent(this); worker->local_server_->deleteLater(); worker->local_server_ = NULL; // Create the handler. - worker->handler_ = new HandlerType(socket, this); + worker->handler_ = new HandlerType(worker->local_socket_, this); emit WorkerConnected(); } diff --git a/src/main.cpp b/src/main.cpp index 92965dcee..2097bae66 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -370,12 +370,12 @@ int main(int argc, char *argv[]) { cover_providers.AddProvider(new AmazonCoverProvider); // Create the tag loader on another thread. - TagReaderClient tag_reader_client; + TagReaderClient* tag_reader_client = new TagReaderClient; QThread tag_reader_thread; tag_reader_thread.start(); - tag_reader_client.moveToThread(&tag_reader_thread); - tag_reader_client.Start(); + tag_reader_client->moveToThread(&tag_reader_thread); + tag_reader_client->Start(); // Create some key objects scoped_ptr > database( @@ -436,6 +436,10 @@ int main(int argc, char *argv[]) { int ret = a.exec(); + tag_reader_client->deleteLater(); + tag_reader_thread.quit(); + tag_reader_thread.wait(); + #ifdef Q_OS_LINUX // The nvidia driver would cause Clementine (or any application that used // opengl) to use 100% cpu on shutdown. See: