[PictureLoader] Fix worker thread shutdown and cross-thread cache clearing

clearNetworkCache() ran directly on the UI thread while the worker thread
owned the disk cache and redirect cache, racing cache reads/writes. Make it
a worker-thread slot invoked via a blocking queued call when the thread is
running, so the 'Cached card pictures have been reset.' message is truthful.

The worker thread was also never quit()/wait()ed: both destructors only
deleteLater'd their objects, so Qt warned 'QThread: Destroyed while thread
is still running' and leaked a running loop at exit. Wire the worker's
finished() signal to its own deleteLater() (canonical worker-object
pattern), add shutdownThread() to stop the loop, and let CardPictureLoader
destroy the QThread only after wait() has returned.

cockatrice/src/interface/card_picture_loader/card_picture_loader.cpp

@@ -11,6 +11,7 @@
 #include <QDir>
 #include <QFileInfo>
 #include <QMainWindow>
+#include <QMetaObject>
 #include <QMovie>
 #include <QNetworkRequest>
 #include <QPainter>
@@ -58,7 +59,14 @@ CardPictureLoader::CardPictureLoader() : QObject(nullptr)
 
 CardPictureLoader::~CardPictureLoader()
 {
-    worker->deleteLater();
+    if (worker) {
+        // Capture the thread first: shutdownThread() blocks until the worker has been freed by the
+        // finished() -> deleteLater chain, after which the worker pointer must not be dereferenced.
+        QThread *pictureLoaderThread = worker->workerThread();
+        worker->shutdownThread();
+        worker = nullptr;
+        delete pictureLoaderThread;
+    }
 }
 
 void CardPictureLoader::getCardBackPixmap(QPixmap &pixmap, QSize size)
@@ -457,7 +465,15 @@ void CardPictureLoader::clearPixmapCache()
 
 void CardPictureLoader::clearNetworkCache()
 {
-    getInstance().worker->clearNetworkCache();
+    auto &worker = *getInstance().worker;
+    // The disk cache and redirect cache are owned by the worker thread; clearing them from the
+    // UI thread would race with the worker's cache reads/writes. Block until the worker thread
+    // has executed the clear so the "Cached card pictures have been reset." message is truthful.
+    if (worker.isRunning()) {
+        QMetaObject::invokeMethod(&worker, "clearNetworkCache", Qt::BlockingQueuedConnection);
+    } else {
+        worker.clearNetworkCache();
+    }
 }
 
 void CardPictureLoader::cacheCardPixmaps(const QList<ExactCard> &cards)

cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp

@@ -59,6 +59,9 @@ CardPictureLoaderWorker::CardPictureLoaderWorker()
     localLoader = new CardPictureLoaderLocal(this);
 
     pictureLoaderThread = new QThread;
+    // The worker object frees itself once its thread finishes, so no event loop is left
+    // running and the QThread is never destroyed while still executing.
+    connect(pictureLoaderThread, &QThread::finished, this, &QObject::deleteLater);
     pictureLoaderThread->start(QThread::LowPriority);
     moveToThread(pictureLoaderThread);
 
@@ -82,7 +85,28 @@ CardPictureLoaderWorker::CardPictureLoaderWorker()
 CardPictureLoaderWorker::~CardPictureLoaderWorker()
 {
     saveRedirectCache();
-    pictureLoaderThread->deleteLater();
+}
+
+void CardPictureLoaderWorker::shutdownThread()
+{
+    // The finished() -> deleteLater chain (wired in the constructor) frees this worker as soon as
+    // its event loop exits, so nothing - not even a member read - may run once wait() returns.
+    // QThread::quit() and QThread::wait() are thread-safe and may be called from the owning thread.
+    QThread *thread = pictureLoaderThread;
+    if (thread) {
+        thread->quit();
+        thread->wait();
+    }
+}
+
+QThread *CardPictureLoaderWorker::workerThread() const
+{
+    return pictureLoaderThread;
+}
+
+bool CardPictureLoaderWorker::isRunning() const
+{
+    return pictureLoaderThread != nullptr && pictureLoaderThread->isRunning();
 }
 
 void CardPictureLoaderWorker::queueRequest(const QUrl &url, CardPictureLoaderWorkerWork *worker)

cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h

@@ -74,10 +74,37 @@ public:
      */
     void onHostRateLimited(const QString &host);
 
-    /** @brief Clears the network cache and redirect cache. */
+    /**
-    void clearNetworkCache();
+     * @brief Stops the worker thread and releases it.
+     *
+     * Called from the owning thread (CardPictureLoader) on its way out. QThread::quit() posts an
+     * exit request to the worker's event loop and QThread::wait() blocks until the loop has
+     * returned and the thread finished. Only QThread members are touched here, so this method is
+     * safe to call from the owning thread. The worker object itself is freed by the finished() ->
+     * deleteLater chain (see the constructor); the QThread object is deleted afterwards by the
+     * owner (CardPictureLoader::~CardPictureLoader), not by this method.
+     */
+    void shutdownThread();
+
+    /** @return Whether the worker's thread is currently running. */
+    bool isRunning() const;
+
+    /**
+     * @brief Returns the worker's QThread.
+     * @return The worker thread
+     *
+     * Only meaningful while the worker object is alive; capture it before calling shutdownThread().
+     */
+    QThread *workerThread() const;
 
 public slots:
+    /**
+     * @brief Clears the network cache and redirect cache.
+     *
+     * Runs on the worker thread; invoke it via a queued call when coming from another thread,
+     * since both caches are owned by the worker thread.
+     */
+    void clearNetworkCache();
     /**
      * @brief Makes a network request for the given URL using the specified worker.
      * @param url URL to load