mirror of
https://github.com/Cockatrice/Cockatrice.git
synced 2026-09-28 08:52:19 -07:00
[PictureLoader] Fix worker thread shutdown and cross-thread cache clearing (#7292)
* [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. * [PictureLoader] Guard cache teardown and stop blocking the UI thread --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
This commit is contained in:
parent
14acf3bf64
commit
9f66a098ac
7 changed files with 108 additions and 11 deletions
|
|
@ -11,6 +11,7 @@
|
|||
#include <QDir>
|
||||
#include <QFileInfo>
|
||||
#include <QMainWindow>
|
||||
#include <QMetaObject>
|
||||
#include <QMovie>
|
||||
#include <QNetworkRequest>
|
||||
#include <QPainter>
|
||||
|
|
@ -43,6 +44,7 @@ CardPictureLoader::CardPictureLoader() : QObject(nullptr)
|
|||
|
||||
qRegisterMetaType<ExactCard>("ExactCard");
|
||||
connect(worker, &CardPictureLoaderWorker::imageLoaded, this, &CardPictureLoader::imageLoaded);
|
||||
connect(worker, &CardPictureLoaderWorker::networkCacheCleared, this, &CardPictureLoader::networkCacheCleared);
|
||||
|
||||
statusBar = new CardPictureLoaderStatusBar(nullptr);
|
||||
QMainWindow *mainWindow = qobject_cast<QMainWindow *>(QApplication::activeWindow());
|
||||
|
|
@ -58,7 +60,18 @@ 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();
|
||||
const bool stopped = worker->shutdownThread();
|
||||
worker = nullptr;
|
||||
// Deleting a QThread that is still running is undefined behaviour, so only free it once the
|
||||
// bounded wait in shutdownThread() confirmed that it stopped.
|
||||
if (stopped) {
|
||||
delete pictureLoaderThread;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void CardPictureLoader::getCardBackPixmap(QPixmap &pixmap, QSize size)
|
||||
|
|
@ -457,7 +470,17 @@ void CardPictureLoader::clearPixmapCache()
|
|||
|
||||
void CardPictureLoader::clearNetworkCache()
|
||||
{
|
||||
getInstance().worker->clearNetworkCache();
|
||||
// During teardown the worker is released before this singleton, so a queued clear may still
|
||||
// arrive with no worker left to run it.
|
||||
CardPictureLoaderWorker *worker = getInstance().worker;
|
||||
if (!worker) {
|
||||
return;
|
||||
}
|
||||
// The disk cache and redirect cache are owned by the worker thread, so the clear has to run
|
||||
// there. Invoke it asynchronously to keep the GUI responsive while the worker may be walking
|
||||
// the user's picture directories or recursively deleting the cache directory; callers that
|
||||
// need to know when it is done can listen for networkCacheCleared().
|
||||
QMetaObject::invokeMethod(worker, &CardPictureLoaderWorker::clearNetworkCache, Qt::QueuedConnection);
|
||||
}
|
||||
|
||||
void CardPictureLoader::cacheCardPixmaps(const QList<ExactCard> &cards)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue