diff --git a/cockatrice/src/client/settings/cache_settings.h b/cockatrice/src/client/settings/cache_settings.h index 23cdb4dbf..7a90d82f1 100644 --- a/cockatrice/src/client/settings/cache_settings.h +++ b/cockatrice/src/client/settings/cache_settings.h @@ -58,33 +58,33 @@ signals: void themeChanged(); private: - QSettings *settings; - ShortcutsSettings *shortcutsSettings; - CardDatabaseSettings *cardDatabaseSettings; - ServersSettings *serversSettings; - MessageSettings *messageSettings; - GameFiltersSettings *gameFiltersSettings; - LayoutsSettings *layoutsSettings; - DownloadSettings *downloadSettings; - RecentsSettings *recentsSettings; - CardOverrideSettings *cardOverrideSettings; - DebugSettings *debugSettings; - CardCounterSettings *cardCounterSettings; - TabsSettings *tabsSettings; - SoundSettings *soundSettings; - GameSettings *gameSettings; - ChatSettings *chatSettings; - CacheStorageSettings *cacheStorageSettings; - UpdatesSettings *updatesSettings; - PersonalSettings *personalSettings; - CardsDisplaySettings *cardsDisplaySettings; - InterfaceSettings *interfaceSettings; - DeckEditorSettings *deckEditorSettings; - PathsSettings *pathsSettings; - VisualDeckStorageSettings *visualDeckStorageSettings; - AppearanceSettings *appearanceSettings; - NetworkSettings *networkSettings; - CommanderBracketSettings *commanderBracketSettings; + QSettings *settings = nullptr; + ShortcutsSettings *shortcutsSettings = nullptr; + CardDatabaseSettings *cardDatabaseSettings = nullptr; + ServersSettings *serversSettings = nullptr; + MessageSettings *messageSettings = nullptr; + GameFiltersSettings *gameFiltersSettings = nullptr; + LayoutsSettings *layoutsSettings = nullptr; + DownloadSettings *downloadSettings = nullptr; + RecentsSettings *recentsSettings = nullptr; + CardOverrideSettings *cardOverrideSettings = nullptr; + DebugSettings *debugSettings = nullptr; + CardCounterSettings *cardCounterSettings = nullptr; + TabsSettings *tabsSettings = nullptr; + SoundSettings *soundSettings = nullptr; + GameSettings *gameSettings = nullptr; + ChatSettings *chatSettings = nullptr; + CacheStorageSettings *cacheStorageSettings = nullptr; + UpdatesSettings *updatesSettings = nullptr; + PersonalSettings *personalSettings = nullptr; + CardsDisplaySettings *cardsDisplaySettings = nullptr; + InterfaceSettings *interfaceSettings = nullptr; + DeckEditorSettings *deckEditorSettings = nullptr; + PathsSettings *pathsSettings = nullptr; + VisualDeckStorageSettings *visualDeckStorageSettings = nullptr; + AppearanceSettings *appearanceSettings = nullptr; + NetworkSettings *networkSettings = nullptr; + CommanderBracketSettings *commanderBracketSettings = nullptr; QString themeName; diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_loader.cpp index e7e31f5a2..0d464a3f7 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader.cpp +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader.cpp @@ -41,6 +41,7 @@ CardPictureLoader::CardPictureLoader() : QObject(nullptr) qRegisterMetaType(); connect(worker, &CardPictureLoaderWorker::imageLoaded, this, &CardPictureLoader::imageLoaded); + connect(worker, &CardPictureLoaderWorker::networkCacheCleared, this, &CardPictureLoader::networkCacheCleared); statusBar = new CardPictureLoaderStatusBar(nullptr); QMainWindow *mainWindow = qobject_cast(QApplication::activeWindow()); @@ -60,9 +61,13 @@ CardPictureLoader::~CardPictureLoader() // 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(); + const bool stopped = worker->shutdownThread(); worker = nullptr; - delete pictureLoaderThread; + // 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; + } } } @@ -303,15 +308,17 @@ void CardPictureLoader::clearPixmapCache() void CardPictureLoader::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(); + // 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 &cards) diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader.h b/cockatrice/src/interface/card_picture_loader/card_picture_loader.h index 5c3ac84a3..c53fdfcc4 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader.h +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader.h @@ -110,6 +110,9 @@ public: public slots: /** * @brief Clears the network disk cache of the worker. + * + * The clear runs on the worker thread, so this returns before it has completed; connect to + * networkCacheCleared() to act once it is done. */ static void clearNetworkCache(); @@ -122,6 +125,10 @@ public slots: void imageLoaded(const ExactCard &card, const QImage &image); void saveCardImageToLocalStorage(const ExactCard &card, const QPixmap &pixmap); +signals: + /** @brief Emitted after the worker has finished clearing the network and redirect caches. */ + void networkCacheCleared(); + private slots: /** * @brief Triggered when the user changes the picture download settings. diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp index daac1bff5..13407aab9 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp @@ -21,10 +21,10 @@ static constexpr int MIN_HOST_QUOTA = DownloadSettings::MIN_HOST_REQUEST_LIMIT; static constexpr qint64 QUOTA_RECOVER_MS = 60000; ///< Idle time before a reduced quota starts recovering static constexpr int DISPATCH_INTERVAL_MS = 100; ///< Pacing between individual network requests static constexpr qint64 QUOTA_RESET_INTERVAL_MS = 1000; ///< Interval at which the request quota resets +static constexpr int THREAD_SHUTDOWN_WAIT_MS = 5000; ///< Bounded wait for the worker thread to stop at exit CardPictureLoaderWorker::CardPictureLoaderWorker() : QObject(nullptr), picDownload(SettingsCache::instance().downloads().getPicDownload()), - requestQuota(MAX_REQUESTS_PER_SEC), hostRequestLimits(SettingsCache::instance().downloads().getHostRequestLimits()) { networkManager = new QNetworkAccessManager(this); @@ -88,16 +88,26 @@ CardPictureLoaderWorker::~CardPictureLoaderWorker() saveRedirectCache(); } -void CardPictureLoaderWorker::shutdownThread() +bool 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(); + if (!thread) { + return true; } + thread->quit(); + // Only an unbounded wait() would guarantee the thread stops, but this runs from a function-local + // static destructor after main() has returned, with no UI left to interrupt a worker stuck in a + // slow slot or on a stalled filesystem. Bound the wait and leave such a thread to the OS rather + // than hanging the process forever. + if (!thread->wait(THREAD_SHUTDOWN_WAIT_MS)) { + qCWarning(CardPictureLoaderWorkerLog) << "Picture loader worker thread did not stop within" + << THREAD_SHUTDOWN_WAIT_MS << "ms; leaving it to be torn down by the OS"; + return false; + } + return true; } QThread *CardPictureLoaderWorker::workerThread() const @@ -105,11 +115,6 @@ QThread *CardPictureLoaderWorker::workerThread() const return pictureLoaderThread; } -bool CardPictureLoaderWorker::isRunning() const -{ - return pictureLoaderThread != nullptr && pictureLoaderThread->isRunning(); -} - void CardPictureLoaderWorker::queueRequest(const QUrl &url, CardPictureLoaderWorkerWork *worker) { QUrl cachedRedirect = getCachedRedirect(url); @@ -137,6 +142,14 @@ QNetworkReply *CardPictureLoaderWorker::makeRequest(const QUrl &url, CardPicture QUrl cachedRedirect = getCachedRedirect(url); if (!cachedRedirect.isEmpty()) { emit imageRequestSucceeded(url); + // The redirect target is a different host, which may itself be in 429 backoff; hand the + // entry back to its worker so it waits the backoff out instead of dispatching straight + // onto the backed-off host. + if (CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(cachedRedirect.host(), + QDateTime::currentDateTime())) { + worker->scheduleDeferredRetry(); + return nullptr; + } return makeRequest(cachedRedirect, worker); } @@ -157,31 +170,43 @@ QNetworkReply *CardPictureLoaderWorker::makeRequest(const QUrl &url, CardPicture QNetworkReply *reply = networkManager->get(req); + // Track in-flight replies per host so the unlocked fast path can bound how many requests it + // issues at once, instead of creating replies that time out before Qt opens a connection. + const QString host = url.host(); + hostInFlight.insert(host, hostInFlight.value(host) + 1); + // Connect reply handling - connect(reply, &QNetworkReply::finished, worker, [reply, worker] { worker->handleNetworkReply(reply); }); + connect(reply, &QNetworkReply::finished, worker, [this, reply, worker, host] { + hostInFlight.insert(host, qMax(0, hostInFlight.value(host) - 1)); + worker->handleNetworkReply(reply); + }); return reply; } void CardPictureLoaderWorker::resetRequestQuota() { - requestQuota = MAX_REQUESTS_PER_SEC; - // Allowances are seeded per host on demand in processSingleRequest(), so a - // rate-limited host never gets a fresh full quota mid-second. + // Allowances are seeded lazily per host in processSingleRequest() when a request is first + // looked at in a new second, so a host that enters the queue mid-second now gets its reduced + // per-host allowance instead of falling through to the full per-second default. hostQuotaRemaining.clear(); QDateTime now = QDateTime::currentDateTime(); for (auto it = hostRequestQuota.begin(); it != hostRequestQuota.end();) { - // An unlocked host has no per-host allowance; drop any stale entry instead of - // recovering it towards the UNLIMITED_HOST_QUOTA sentinel, which would poison it. - if (hostAllowanceCeiling(it.key()) == DownloadSettings::UNLIMITED_HOST_QUOTA) { - it = hostRequestQuota.erase(it); - continue; - } if (!hostLast429.contains(it.key()) || now.msecsTo(hostLast429.value(it.key())) < -QUOTA_RECOVER_MS) { - // Recover towards the host's effective allowance ceiling, which may be - // lowered by the user's per-host request limits. - it.value() = qMin(hostAllowanceCeiling(it.key()), it.value() + 1); + if (hostAllowanceCeiling(it.key()) == DownloadSettings::UNLIMITED_HOST_QUOTA) { + // A developer-unlocked host that fell back after a 429 recovers towards the default + // allowance; once it gets there it becomes unlocked (fast-path) again. + if (it.value() + 1 >= DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT) { + it = hostRequestQuota.erase(it); + continue; + } + it.value() += 1; + } else { + // Recover towards the host's effective allowance ceiling, which may be + // lowered by the user's per-host request limits. + it.value() = qMin(hostAllowanceCeiling(it.key()), it.value() + 1); + } } ++it; } @@ -191,49 +216,76 @@ void CardPictureLoaderWorker::resetRequestQuota() void CardPictureLoaderWorker::processQueuedRequests() { + Q_ASSERT(thread() == QThread::currentThread()); + if (requestLoadQueue.isEmpty()) { dispatchTimer.stop(); + requestTimer.stop(); return; } // Start lazily from the worker's own thread: QTimer must be started in the thread it lives in. if (!requestTimer.isActive()) { requestTimer.start(); } - dispatchTimer.start(); + // Restarting an active timer would reset the pacing countdown, so a burst of enqueues could + // keep starving the dispatcher; only start it when it has actually stopped. + if (!dispatchTimer.isActive()) { + dispatchTimer.start(); + } } void CardPictureLoaderWorker::dispatchQueuedRequest() { if (requestLoadQueue.isEmpty()) { + // All queued requests have been dispatched; stop the pacing and quota-reset timers. dispatchTimer.stop(); + requestTimer.stop(); return; } - // Unlocked hosts (developer cap UNLIMITED_HOST_QUOTA) skip the dispatch pacing and the - // global per-second quota: dispatch every queued request for them back-to-back, bounded - // only by their 429 backoff window and Qt's per-host connection pool. QDateTime now = QDateTime::currentDateTime(); + bool dispatched = false; + // Set while an unlocked host still has queued work blocked only by the in-flight cap; the + // timer must keep running so it gets another try as soon as a slot frees. A host blocked by + // its 429 backoff instead waits for the next quota-reset tick to restart the dispatcher. + bool unlockedCapped = false; + + // Unlocked hosts (developer cap UNLIMITED_HOST_QUOTA) skip the pacing and the per-host + // allowance: dispatch their queued requests back-to-back, bounded by their 429 backoff and the + // per-host in-flight cap so a large burst can't queue replies that time out before Qt opens a + // connection for them. for (int i = 0; i < requestLoadQueue.size();) { const auto &request = requestLoadQueue.at(i); const QString host = request.first.host(); - if (hostAllowanceCeiling(host) == DownloadSettings::UNLIMITED_HOST_QUOTA && - !CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(host, now)) { - makeRequest(request.first, request.second); - requestLoadQueue.removeAt(i); - } else { - ++i; + if (isUnlockedHost(host)) { + if (CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(host, now)) { + ++i; + continue; + } + if (hostInFlight.value(host) < MAX_IN_FLIGHT_PER_HOST) { + makeRequest(request.first, request.second); + requestLoadQueue.removeAt(i); + dispatched = true; + continue; + } + unlockedCapped = true; } + ++i; } - if (requestLoadQueue.isEmpty() || requestQuota <= 0) { + if (requestLoadQueue.isEmpty()) { dispatchTimer.stop(); + requestTimer.stop(); return; } if (processSingleRequest()) { - --requestQuota; - } else { - // No queued host currently has allowance left in this second; wait for the quota reset. + dispatched = true; + } + + // Keep the timer running while there is progress to make or unlocked work waiting on a free + // in-flight slot; otherwise no host has allowance left this second, so wait for the quota reset. + if (!dispatched && !unlockedCapped) { dispatchTimer.stop(); } } @@ -244,14 +296,28 @@ bool CardPictureLoaderWorker::processSingleRequest() for (int i = 0; i < requestLoadQueue.size(); ++i) { const auto &request = requestLoadQueue.at(i); const QString host = request.first.host(); - // Don't dispatch requests to a host that is currently in its 429 backoff. + // Don't dispatch requests to a host that is currently in its 429 backoff; hand the entry + // back to its worker so it can wait the backoff out or fall through to another source, + // instead of leaving it parked in the queue with no reply pending. if (CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(host, now)) { + requestLoadQueue.removeAt(i); + request.second->startNextPicDownload(); + return true; + } + // Unlocked hosts are handled by dispatchQueuedRequest's fast path, bounded by the in-flight + // cap; they must not fall through to the per-host allowance arithmetic below. + if (isUnlockedHost(host)) { continue; } - const int ceiling = hostAllowanceCeiling(host); - // Seed the allowance only now, so a host that was rate limited last second - // doesn't get a fresh full quota the moment it is queried mid-second. Clamp - // against the ceiling so a lowered user cap applies from this second onward. + int ceiling = hostAllowanceCeiling(host); + if (ceiling == DownloadSettings::UNLIMITED_HOST_QUOTA) { + // A 429 dropped this unlocked host out of the fast path and installed a concrete + // allowance; pace it against that allowance until the recovery loop unlocks it again. + ceiling = hostRequestQuota.value(host, DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT); + } + // Seed the allowance lazily so a host that enters the queue mid-second gets its reduced + // per-host allowance, clamped against the ceiling so a lowered user cap applies from this + // second onward. if (!hostQuotaRemaining.contains(host)) { hostQuotaRemaining.insert(host, qMin(ceiling, hostRequestQuota.value(host, ceiling))); } @@ -276,15 +342,20 @@ int CardPictureLoaderWorker::hostAllowanceCeiling(const QString &host) const return SettingsCache::instance().downloads().clampHostRequestLimit(host, requested); } +bool CardPictureLoaderWorker::isUnlockedHost(const QString &host) const +{ + return hostAllowanceCeiling(host) == DownloadSettings::UNLIMITED_HOST_QUOTA && !hostRequestQuota.contains(host); +} + void CardPictureLoaderWorker::onHostRateLimited(const QString &host) { const int ceiling = hostAllowanceCeiling(host); - if (ceiling == DownloadSettings::UNLIMITED_HOST_QUOTA) { - // Unlocked hosts have no per-host allowance to halve; the shared backoff - // window tracked by the rate limiter still paces them. - return; - } - hostRequestQuota.insert(host, qMax(MIN_HOST_QUOTA, hostRequestQuota.value(host, ceiling) / 2)); + // An unlocked host has no per-host allowance to halve. Install one instead so it drops out of + // the unlocked fast path and is paced like a throttled host; the recovery loop in + // resetRequestQuota() then walks it back up and unlocks it again. + const int base = + ceiling == DownloadSettings::UNLIMITED_HOST_QUOTA ? DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT : ceiling; + hostRequestQuota.insert(host, qMax(MIN_HOST_QUOTA, hostRequestQuota.value(host, base) / 2)); hostLast429.insert(host, QDateTime::currentDateTime()); } @@ -394,4 +465,5 @@ void CardPictureLoaderWorker::clearNetworkCache() { networkManager->cache()->clear(); redirectCache.clear(); + emit networkCacheCleared(); } diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h index 96405252f..8ae4ffc3b 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h @@ -75,19 +75,19 @@ public: void onHostRateLimited(const QString &host); /** - * @brief Stops the worker thread and releases it. + * @brief Stops the worker thread and reports whether it stopped. * * 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 + * exit request to the worker's event loop and QThread::wait() blocks (bounded) 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. + * + * @return true if the thread stopped within the timeout, false if it is still running (in + * which case the owner must not delete the QThread). */ - void shutdownThread(); - - /** @return Whether the worker's thread is currently running. */ - bool isRunning() const; + [[nodiscard]] bool shutdownThread(); /** * @brief Returns the worker's QThread. @@ -113,7 +113,7 @@ public slots: */ QNetworkReply *makeRequest(const QUrl &url, CardPictureLoaderWorkerWork *workThread); - /** @brief Processes all queued requests respecting the request quota. */ + /** @brief Starts the pacing timers if there is queued work, stops them when the queue is empty. */ void processQueuedRequests(); /** @brief Chooses a request from the queue and starts it, respecting the quota and pacing. */ @@ -148,13 +148,16 @@ private: bool picDownload; ///< Whether downloading images from network is enabled QQueue> requestLoadQueue; ///< Queue of pending network requests - int requestQuota; ///< Remaining requests allowed per second QTimer requestTimer; ///< Timer to reset the request quota QTimer dispatchTimer; ///< Timer pacing individual network requests QHash hostRequestQuota; ///< Sustained per-host request allowance QHash hostRequestLimits; ///< User-set per-host request allowances QHash hostQuotaRemaining; ///< Per-host allowance left in the current second QHash hostLast429; ///< When each host was last rate limited + QHash hostInFlight; ///< Network replies currently in flight, per host + + /** @brief Maximum concurrent in-flight network replies per host. */ + static constexpr int MAX_IN_FLIGHT_PER_HOST = 6; CardPictureLoaderLocal *localLoader; ///< Loader for local images QSet currentlyLoading; ///< Deduplication: contains pixmapCacheKey currently being loaded @@ -167,6 +170,14 @@ private: */ [[nodiscard]] int hostAllowanceCeiling(const QString &host) const; + /** + * @brief Whether a host may skip dispatch pacing and per-host allowance entirely. + * + * A host is unlocked while it has no user limit and no reduced allowance installed by a 429. + * A 429 drops it out of the fast path until resetRequestQuota() walks the allowance back up. + */ + [[nodiscard]] bool isUnlockedHost(const QString &host) const; + /** @brief Returns cached redirect URL for the given original URL, if available. */ [[nodiscard]] QUrl getCachedRedirect(const QUrl &originalUrl) const; @@ -198,6 +209,9 @@ signals: /** @brief Emitted when a network request successfully completes. */ void imageRequestSucceeded(const QUrl &url); + + /** @brief Emitted after clearNetworkCache() has finished clearing both caches. */ + void networkCacheCleared(); }; #endif // PICTURE_LOADER_WORKER_H diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp index 072a919d7..6cedc9e8b 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp @@ -22,13 +22,13 @@ static const QStringList MD5_BLACKLIST = { "fbc7d763c08771c260b39e2115414eeb" // Current card back hash }; -ServerRateLimiter &CardPictureLoaderWorkerWork::rateLimiter() +const ServerRateLimiter &CardPictureLoaderWorkerWork::rateLimiter() { return s_rateLimiter; } -CardPictureLoaderWorkerWork::CardPictureLoaderWorkerWork(const CardPictureLoaderWorker *worker, const ExactCard &toLoad) - : QObject(nullptr), cardToDownload(CardPictureToLoad(toLoad)), +CardPictureLoaderWorkerWork::CardPictureLoaderWorkerWork(CardPictureLoaderWorker *worker, const ExactCard &toLoad) + : QObject(worker), cardToDownload(CardPictureToLoad(toLoad)), picDownload(SettingsCache::instance().downloads().getPicDownload()) { // Hook up signals to the orchestrator diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h index 8490cb3ac..da5049ca7 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h @@ -36,15 +36,36 @@ class CardPictureLoaderWorkerWork : public QObject public: /** * @brief Constructs a worker for downloading a specific card image. - * @param worker The orchestrating CardPictureLoaderWorker + * @param worker The orchestrating CardPictureLoaderWorker; the work object becomes its child so + * it is destroyed with the worker even if it never reaches concludeImageLoad(). * @param toLoad The ExactCard to download */ - explicit CardPictureLoaderWorkerWork(const CardPictureLoaderWorker *worker, const ExactCard &toLoad); + explicit CardPictureLoaderWorkerWork(CardPictureLoaderWorker *worker, const ExactCard &toLoad); CardPictureToLoad cardToDownload; ///< The card and associated URLs to try downloading /** @brief Shared per-server 429 backoff state. */ - static ServerRateLimiter &rateLimiter(); + static const ServerRateLimiter &rateLimiter(); + + /** + * @brief Starts downloading the next URL for this card. + * + * Skips URLs whose server is currently in 429 backoff, either waiting the + * backoff out or falling through to the other configured sources. Also used by + * the dispatch machinery to hand an entry back after it was removed from the + * request queue when its host turned out to be backed off. + */ + void startNextPicDownload(); + + /** + * @brief Schedules a deferred retry after the relevant server backoff expires. + * + * Waits on the current URL's server when it is the reason we are blocked, + * otherwise on the earliest active backoff. If no servers are in backoff, + * concludes with failure. Otherwise resets the CardPictureToLoad indices and + * retries after the backoff period. + */ + void scheduleDeferredRetry(); public slots: /** @@ -58,9 +79,6 @@ private: static ServerRateLimiter s_rateLimiter; ///< Shared per-server 429 backoff state - /** @brief Starts downloading the next URL for this card. */ - void startNextPicDownload(); - /** @brief Called when all URLs have been exhausted or download failed. */ void picDownloadFailed(); @@ -85,16 +103,6 @@ private: */ void concludeImageLoad(const QImage &image); - /** - * @brief Schedules a deferred retry after the relevant server backoff expires. - * - * Waits on the current URL's server when it is the reason we are blocked, - * otherwise on the earliest active backoff. If no servers are in backoff, - * concludes with failure. Otherwise resets the CardPictureToLoad indices and - * retries after the backoff period. - */ - void scheduleDeferredRetry(); - private slots: /** @brief Updates the picDownload setting when it changes. */ void picDownloadChanged(); diff --git a/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp b/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp index 6223cd480..8628aa5a8 100644 --- a/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp +++ b/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -17,8 +18,6 @@ #include #include -static constexpr int UNLOCKED_HOST_LIMIT_MAX = 50; ///< Upper bound for rate limits on hosts unlocked by the developer - DeckEditorSettingsPage::DeckEditorSettingsPage() { picDownloadCheckBox.setChecked(SettingsCache::instance().downloads().getPicDownload()); @@ -54,7 +53,9 @@ DeckEditorSettingsPage::DeckEditorSettingsPage() urlList->setDragDropMode(QAbstractItemView::InternalMove); connect(urlList->model(), &QAbstractItemModel::rowsMoved, this, &DeckEditorSettingsPage::urlListChanged); - urlList->addItems(SettingsCache::instance().downloads().getAllURLs()); + for (const QString &url : SettingsCache::instance().downloads().getAllURLs()) { + addUrlItem(url); + } aAdd = new QAction(this); aAdd->setIcon(themePixmap(QStringLiteral("icons/increment"))); @@ -125,7 +126,9 @@ void DeckEditorSettingsPage::resetDownloadedURLsButtonClicked() { SettingsCache::instance().downloads().resetToDefaultURLs(); urlList->clear(); - urlList->addItems(SettingsCache::instance().downloads().getAllURLs()); + for (const QString &url : SettingsCache::instance().downloads().getAllURLs()) { + addUrlItem(url); + } QMessageBox::information(this, tr("Success"), tr("Download URLs have been reset.")); } @@ -134,7 +137,7 @@ void DeckEditorSettingsPage::actAddURL() bool ok; QString msg = QInputDialog::getText(this, tr("Add URL"), tr("URL:"), QLineEdit::Normal, QString(), &ok); if (ok) { - urlList->addItem(msg); + addUrlItem(msg); storeSettings(); } } @@ -149,12 +152,14 @@ void DeckEditorSettingsPage::actRemoveURL() void DeckEditorSettingsPage::actEditURL() { - if (urlList->currentItem()) { - QString oldText = urlList->currentItem()->text(); + QListWidgetItem *item = urlList->currentItem(); + if (item) { + const QString oldText = urlForItem(item); bool ok; QString msg = QInputDialog::getText(this, tr("Edit URL"), tr("URL:"), QLineEdit::Normal, oldText, &ok); if (ok) { - urlList->currentItem()->setText(msg); + item->setData(Qt::UserRole, msg); + item->setText(urlLabel(msg)); storeSettings(); } } @@ -166,10 +171,77 @@ void DeckEditorSettingsPage::storeSettings() QStringList downloadUrls; for (int i = 0; i < urlList->count(); i++) { - qInfo() << "Priority" << i << ":" << urlList->item(i)->text(); - downloadUrls << urlList->item(i)->text(); + const QString url = urlForItem(urlList->item(i)); + qInfo() << "Priority" << i << ":" << url; + downloadUrls << url; } SettingsCache::instance().downloads().setDownloadUrls(downloadUrls); + + // Drop per-host limits whose host is no longer referenced by any configured URL, so removing + // a URL doesn't leave a stale throttle behind that reactivates if the host is re-added. + QSet usedHosts; + for (const QString &url : downloadUrls) { + const QString host = QUrl(url).host(); + if (!host.isEmpty()) { + usedHosts.insert(host); + } + } + QHash limits = SettingsCache::instance().downloads().getHostRequestLimits(); + bool limitsChanged = false; + for (auto it = limits.begin(); it != limits.end();) { + if (!usedHosts.contains(it.key())) { + it = limits.erase(it); + limitsChanged = true; + } else { + ++it; + } + } + if (limitsChanged) { + SettingsCache::instance().downloads().setHostRequestLimits(limits); + } + + refreshUrlItems(); +} + +QListWidgetItem *DeckEditorSettingsPage::addUrlItem(const QString &url) +{ + auto *item = new QListWidgetItem(urlLabel(url)); + item->setData(Qt::UserRole, url); + urlList->addItem(item); + return item; +} + +QString DeckEditorSettingsPage::urlForItem(const QListWidgetItem *item) const +{ + return item->data(Qt::UserRole).toString(); +} + +QString DeckEditorSettingsPage::urlLabel(const QString &url) const +{ + const QString host = QUrl(url).host(); + if (host.isEmpty()) { + return url; + } + + const QHash limits = SettingsCache::instance().downloads().getHostRequestLimits(); + const int devCap = + DownloadSettings::getDeveloperHostCaps().value(host, DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT); + if (devCap == DownloadSettings::UNLIMITED_HOST_QUOTA && !limits.contains(host)) { + return tr("%1 (unlimited)").arg(url); + } + + const int requested = limits.value( + host, devCap == DownloadSettings::UNLIMITED_HOST_QUOTA ? DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT : devCap); + const int effective = SettingsCache::instance().downloads().clampHostRequestLimit(host, requested); + return tr("%1 (%2/s)").arg(url).arg(effective); +} + +void DeckEditorSettingsPage::refreshUrlItems() +{ + for (int i = 0; i < urlList->count(); ++i) { + QListWidgetItem *item = urlList->item(i); + item->setText(urlLabel(urlForItem(item))); + } } void DeckEditorSettingsPage::actAdjustRateLimit() @@ -179,7 +251,7 @@ void DeckEditorSettingsPage::actAdjustRateLimit() return; } - const QString host = QUrl(urlList->currentItem()->text()).host(); + const QString host = QUrl(urlForItem(urlList->currentItem())).host(); if (host.isEmpty()) { QMessageBox::information(this, tr("Adjust Rate Limit"), tr("The selected URL does not have a valid host.")); return; @@ -194,19 +266,21 @@ void DeckEditorSettingsPage::actAdjustRateLimit() int minimum; int maximum; int defaultValue; + QString prompt; if (unlocked) { minimum = 0; // 0 means "unlimited" - maximum = UNLOCKED_HOST_LIMIT_MAX; + maximum = DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT; defaultValue = currentLimits.value(host, 0); + prompt = tr("Requests per second (0 = unlimited, fastest; up to %1):").arg(maximum); } else { minimum = DownloadSettings::MIN_HOST_REQUEST_LIMIT; maximum = devCap; defaultValue = currentLimits.value(host, devCap); + prompt = tr("Requests per second (developer maximum is %1):").arg(maximum); } - const int value = QInputDialog::getInt(this, tr("Adjust Rate Limit for %1").arg(host), - tr("Requests per second (developer maximum is %1):").arg(maximum), - defaultValue, minimum, maximum, 1, &ok); + const int value = QInputDialog::getInt(this, tr("Adjust Rate Limit for %1").arg(host), prompt, defaultValue, + minimum, maximum, 1, &ok); if (!ok) { return; } @@ -218,6 +292,7 @@ void DeckEditorSettingsPage::actAdjustRateLimit() limits.insert(host, value); } SettingsCache::instance().downloads().setHostRequestLimits(limits); + refreshUrlItems(); } void DeckEditorSettingsPage::urlListChanged(const QModelIndex &, int, int, const QModelIndex &, int) @@ -301,4 +376,7 @@ void DeckEditorSettingsPage::retranslateUi() aEdit->setText(tr("Edit URL")); aRemove->setText(tr("Remove URL")); aRateLimit->setText(tr("Adjust Rate Limit")); + + // The per-URL rate limit suffixes are translated, so refresh them when the language changes. + refreshUrlItems(); } diff --git a/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h b/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h index 57de5699e..23e33fa20 100644 --- a/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h +++ b/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h @@ -47,6 +47,18 @@ private: QLabel infoOnSpoilersLabel; QPushButton *mpSpoilerPathButton; QPushButton *updateNowButton; + + /** @brief Adds a list item for the given URL, storing the raw URL alongside its displayed label. */ + QListWidgetItem *addUrlItem(const QString &url); + + /** @brief Returns the raw URL stored on a list item. */ + [[nodiscard]] QString urlForItem(const QListWidgetItem *item) const; + + /** @brief Returns the display label for a URL, including its current effective rate limit. */ + [[nodiscard]] QString urlLabel(const QString &url) const; + + /** @brief Refreshes the displayed label of every URL item after limits or settings change. */ + void refreshUrlItems(); }; #endif // COCKATRICE_DECK_EDITOR_SETTINGS_PAGE_H diff --git a/cockatrice/src/interface/widgets/settings_page/storage_settings_page.cpp b/cockatrice/src/interface/widgets/settings_page/storage_settings_page.cpp index 17838e501..4aad91115 100644 --- a/cockatrice/src/interface/widgets/settings_page/storage_settings_page.cpp +++ b/cockatrice/src/interface/widgets/settings_page/storage_settings_page.cpp @@ -181,9 +181,14 @@ StorageSettingsPage::StorageSettingsPage() void StorageSettingsPage::clearDownloadedPicsButtonClicked() { - CardPictureLoader::clearNetworkCache(); + // The network cache is cleared asynchronously on the worker thread, so wait for the completion + // signal before confirming; the in-memory pixmap cache is cleared synchronously right away. + connect( + &CardPictureLoader::getInstance(), &CardPictureLoader::networkCacheCleared, this, + [this] { QMessageBox::information(this, tr("Success"), tr("Cached card pictures have been reset.")); }, + Qt::SingleShotConnection); CardPictureLoader::clearPixmapCache(); - QMessageBox::information(this, tr("Success"), tr("Cached card pictures have been reset.")); + CardPictureLoader::clearNetworkCache(); } void StorageSettingsPage::clearImageBackupsButtonClicked() diff --git a/libcockatrice_settings/libcockatrice/settings/download_settings.cpp b/libcockatrice_settings/libcockatrice/settings/download_settings.cpp index 5293e390b..a321f0bd0 100644 --- a/libcockatrice_settings/libcockatrice/settings/download_settings.cpp +++ b/libcockatrice_settings/libcockatrice/settings/download_settings.cpp @@ -69,21 +69,46 @@ void DownloadSettings::setDownloadSpoilerStatus(bool _spoilerStatus) QHash DownloadSettings::getHostRequestLimits() const { - const QVariantMap stored = getValue("hostRequestLimits").toMap(); + auto settings = getSettings(); + if (!defaultGroup.isEmpty()) { + settings.beginGroup(defaultGroup); + } + settings.beginGroup("hostRequestLimits"); + QHash hostRequestLimits; - for (auto it = stored.cbegin(); it != stored.cend(); ++it) { - hostRequestLimits.insert(it.key(), it.value().toInt()); + const QStringList hosts = settings.childKeys(); + for (const QString &host : hosts) { + hostRequestLimits.insert(host, settings.value(host).toInt()); + } + + settings.endGroup(); + if (!defaultGroup.isEmpty()) { + settings.endGroup(); } return hostRequestLimits; } void DownloadSettings::setHostRequestLimits(const QHash &hostRequestLimits) { - QVariantMap stored; - for (auto it = hostRequestLimits.cbegin(); it != hostRequestLimits.cend(); ++it) { - stored.insert(it.key(), it.value()); + auto settings = getSettings(); + if (!defaultGroup.isEmpty()) { + settings.beginGroup(defaultGroup); } - setValue(stored, "hostRequestLimits"); + + // Drop the legacy single-key form (an opaque @Variant blob) written by earlier builds so each + // host is stored as a plain, hand-editable key in its own subgroup. + settings.remove("hostRequestLimits"); + settings.beginGroup("hostRequestLimits"); + settings.remove(QString()); + for (auto it = hostRequestLimits.cbegin(); it != hostRequestLimits.cend(); ++it) { + settings.setValue(it.key(), it.value()); + } + settings.endGroup(); + + if (!defaultGroup.isEmpty()) { + settings.endGroup(); + } + settings.sync(); emit hostRequestLimitsChanged(); } diff --git a/tests/picture_loader_benchmark/picture_loader_benchmark.cpp b/tests/picture_loader_benchmark/picture_loader_benchmark.cpp index 44d930d37..f991f2ec0 100644 --- a/tests/picture_loader_benchmark/picture_loader_benchmark.cpp +++ b/tests/picture_loader_benchmark/picture_loader_benchmark.cpp @@ -37,8 +37,11 @@ #include #include #include +#include #include #include +#include +#include #include #include #include @@ -165,17 +168,38 @@ struct LogCounters QMutex mutex; }; -static LogCounters *s_activeCounters = nullptr; +static std::atomic s_activeCounters{nullptr}; static QtMessageHandler s_previousMessageHandler = nullptr; static void stressLogHandler(QtMsgType type, const QMessageLogContext &context, const QString &msg) { - if (s_activeCounters && msg.contains(QStringLiteral("Too many requests from"))) { - QMutexLocker locker(&s_activeCounters->mutex); - ++s_activeCounters->http429; + if (LogCounters *counters = s_activeCounters.load(std::memory_order_acquire); + counters && msg.contains(QStringLiteral("Too many requests from"))) { + QMutexLocker locker(&counters->mutex); + ++counters->http429; } if (s_previousMessageHandler) { s_previousMessageHandler(type, context, msg); + } else { + // qInstallMessageHandler() reports the built-in handler as nullptr, so a plain `if` would + // swallow every message - including the 429 warnings this run is meant to surface. Fall + // back to Qt's message pattern written to stderr instead. + std::fprintf(stderr, "%s\n", qPrintable(qFormatLogMessage(type, context, msg))); + } +} + +// Stops a worker's thread and frees it. shutdownThread()'s bounded wait guarantees that the worker +// object was freed by its finished() -> deleteLater chain, so the thread itself can then be deleted +// safely. A worker whose thread refused to stop is left alone (and leaked) rather than freed while +// still running. +static void destroyWorker(CardPictureLoaderWorker *worker) +{ + if (!worker) { + return; + } + QThread *thread = worker->workerThread(); + if (worker->shutdownThread()) { + delete thread; } } @@ -234,8 +258,14 @@ static StressResult runStress(CardPictureLoaderWorker *workerA, result.a.elapsedMs = elapsedMs; result.b.elapsedMs = elapsedMs; + // Stop both workers before touching the counters or restoring the message handler: their + // threads log from stressLogHandler, and must not outlive the stack-local counters (which is + // guaranteed on the watchdog path, where requests and deferred retries are still pending). + destroyWorker(workerA); + destroyWorker(workerB); + result.http429Count = counters.http429; - s_activeCounters = nullptr; + s_activeCounters.store(nullptr, std::memory_order_release); qInstallMessageHandler(s_previousMessageHandler); return result; } @@ -280,6 +310,7 @@ int main(int argc, char **argv) QStringList explicitUrls; int count = 300; bool stress = false; + bool stressUrlSet = false; QString stressUrl(QStringLiteral("https://api.scryfall.com/cards/!set:uuid!?format=image")); QString cacheDirArg; std::optional timeoutMin; @@ -301,8 +332,15 @@ int main(int argc, char **argv) stress = true; } else if (arg == QLatin1String("--stress-url")) { stressUrl = value(); + stressUrlSet = true; } else if (arg == QLatin1String("--timeout-min")) { - timeoutMin = value().toInt(); + bool ok = false; + const int parsed = value().toInt(&ok); + if (!ok || parsed <= 0) { + std::fprintf(stderr, "error: --timeout-min must be a positive integer\n"); + return 2; + } + timeoutMin = parsed; } else if (arg == QLatin1String("--cache-dir")) { cacheDirArg = value(); } else if (arg == QLatin1String("--help") || arg == QLatin1String("-h")) { @@ -325,37 +363,53 @@ int main(int argc, char **argv) return 2; } + // In --stress mode an explicit --url selects the template to hammer, matching --url's meaning + // in the normal mode; only fall back to the built-in Scryfall template when neither --stress-url + // nor --url was supplied. + if (stress && !stressUrlSet && !explicitUrls.isEmpty()) { + stressUrl = explicitUrls.first(); + } + QCoreApplication app(argc, argv); - app.setApplicationName(QStringLiteral("Cockatrice")); - app.setOrganizationName(QStringLiteral("Cockatrice")); + // Unique names so a benchmark run can never read or write the real client's settings, cache or + // picture URLs on platforms where the XDG redirection below does not apply (macOS, Windows). + app.setApplicationName(QStringLiteral("Cockatrice-benchmark")); + app.setOrganizationName(QStringLiteral("Cockatrice-benchmark")); app.setApplicationVersion(QStringLiteral("9.0.0-benchmark")); // The ExactCard argument of imageLoaded crosses threads via a queued connection. qRegisterMetaType(); QTemporaryDir sandbox; + if (cacheDirArg.isEmpty() && !sandbox.isValid()) { + std::fprintf(stderr, "error: could not create a temporary sandbox directory\n"); + return 2; + } const QString rootDir = cacheDirArg.isEmpty() ? sandbox.path() : cacheDirArg; QDir().mkpath(rootDir); QDir().mkpath(rootDir + "/config"); QDir().mkpath(rootDir + "/data"); QDir().mkpath(rootDir + "/cache"); -#ifdef Q_OS_UNIX - // Redirect every QStandardPaths lookup (and therefore SettingsCache paths) - // into the sandbox so the benchmark never touches user config or caches. +#ifdef Q_OS_LINUX + // Redirect every QStandardPaths lookup (and therefore SettingsCache paths) into the sandbox so + // the benchmark never touches user config or caches. XDG_* only affects Qt's path resolution on + // Linux; elsewhere the unique application/organization names above keep the run isolated. qputenv("XDG_CONFIG_HOME", (rootDir + "/config").toUtf8()); qputenv("XDG_DATA_HOME", (rootDir + "/data").toUtf8()); qputenv("XDG_CACHE_HOME", (rootDir + "/cache").toUtf8()); #endif - const bool warmStart = QDir(rootDir + "/cache/Cockatrice/downloaded") + // Derive the probe from SettingsCache rather than reconstructing it: Qt appends both the + // organization and the application name, so a hand-built path is easy to get wrong. + const bool warmStart = QDir(SettingsCache::instance().getNetworkCachePath()) .entryList(QDir::Files | QDir::AllDirs | QDir::NoDotAndDotDot, QDir::Name) .size() > 0; // Copied into the sandbox so the loader's binary cache ("cards.xml.cache") // is written next to it instead of next to the user's file, and so a // --cache-dir rerun can pick it up again. - const QString dataPath = rootDir + "/data/Cockatrice"; + const QString dataPath = SettingsCache::instance().getDataPath(); QDir().mkpath(dataPath); const QString cardsXml = dataPath + "/cards.xml"; if (!QFile::exists(cardsXml)) { @@ -427,7 +481,6 @@ int main(int argc, char **argv) std::printf("=== PICTURE LOADER BENCHMARK (%d cards available, %d per template)%s ===\n", availableCards, count, warmStart ? ", WARM cache from previous run" : ""); - auto *worker = new CardPictureLoaderWorker(); for (const QString &urlTemplate : urlsToTest) { const QList cards = selectCardsForTemplate(db, urlTemplate, count); if (cards.isEmpty()) { @@ -442,10 +495,21 @@ int main(int argc, char **argv) const int timeoutMs = (timeoutMin.has_value() ? timeoutMin.value() : (cards.size() * perCardMs * 8 + 60000) / 60000) * 60 * 1000; const qint64 coldLowerMs = static_cast(cards.size()) * perCardMs / 2; - const qint64 cachedUpperMs = 8000; + // Decoding and cache-reading scale with the card count, so a flat budget would spuriously + // fail larger --count runs served entirely from a healthy cache. + const qint64 cachedUpperMs = qMax(2000, static_cast(cards.size()) * 10); - const PassResult cold = runPass(worker, cards, timeoutMs); - const PassResult cached = runPass(worker, cards, timeoutMs); + // A fresh worker per pass: if the cold pass hits the watchdog, its outstanding cards stay + // in the worker's currentlyLoading set, which would make the cached pass silently skip them + // and burn its own watchdog; late cold replies would also be misattributed to the cached + // pass. + auto *coldWorker = new CardPictureLoaderWorker(); + const PassResult cold = runPass(coldWorker, cards, timeoutMs); + destroyWorker(coldWorker); + + auto *cachedWorker = new CardPictureLoaderWorker(); + const PassResult cached = runPass(cachedWorker, cards, timeoutMs); + destroyWorker(cachedWorker); const bool coldComplete = cold.finished >= cold.enqueued; const bool coldZeroFailures = cold.failed == 0; @@ -472,8 +536,12 @@ int main(int argc, char **argv) } } - std::printf("cache root: %s%s\n", qPrintable(rootDir), - cacheDirArg.isEmpty() ? " (reuse with --cache-dir to warm on the next run)" : ""); + if (cacheDirArg.isEmpty()) { + std::printf("cache root: %s (temporary; pass --cache-dir to persist it for a warm rerun)\n", + qPrintable(rootDir)); + } else { + std::printf("cache root: %s\n", qPrintable(rootDir)); + } std::printf("RESULT: %s\n", allPass ? "PASS" : "FAIL"); return allPass ? 0 : 1; -} \ No newline at end of file +} diff --git a/tests/picture_loader_benchmark/settings_cache_mock.cpp b/tests/picture_loader_benchmark/settings_cache_mock.cpp index 9b20db603..d93c7e002 100644 --- a/tests/picture_loader_benchmark/settings_cache_mock.cpp +++ b/tests/picture_loader_benchmark/settings_cache_mock.cpp @@ -197,4 +197,4 @@ SettingsCache &SettingsCache::instance() { static SettingsCache settingsCache; return settingsCache; -} \ No newline at end of file +}