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 b29e838ff..728c5cc6d 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 @@ -148,35 +148,25 @@ void CardPictureLoaderWorker::resetRequestQuota() hostQuotaRemaining.insert(host, hostRequestQuota.value(host, MAX_REQUESTS_PER_SEC)); } - processQueuedRequests(); + updateTimerState(); } void CardPictureLoaderWorker::processQueuedRequests() { - Q_ASSERT(thread() == QThread::currentThread()); - - if (requestLoadQueue.isEmpty()) { - dispatchTimer.stop(); - requestTimer.stop(); + // QTimer must be started from the thread it lives in; if this public slot is ever reached from + // another thread, replay it on the worker's event loop instead of letting start() fail silently. + if (thread() != QThread::currentThread()) { + QMetaObject::invokeMethod(this, &CardPictureLoaderWorker::processQueuedRequests, Qt::QueuedConnection); return; } - // Start lazily from the worker's own thread: QTimer must be started in the thread it lives in. - if (!requestTimer.isActive()) { - requestTimer.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(); - } + updateTimerState(); } void CardPictureLoaderWorker::dispatchQueuedRequest() { if (requestLoadQueue.isEmpty()) { - // All queued requests have been dispatched; stop the pacing and quota-reset timers. - dispatchTimer.stop(); - requestTimer.stop(); + // All queued requests have been dispatched; stop the pacing timers. + updateTimerState(); return; } @@ -186,6 +176,40 @@ void CardPictureLoaderWorker::dispatchQueuedRequest() } } +void CardPictureLoaderWorker::updateTimerState() +{ + // Never restart an active timer: that would reset the pacing countdown and a burst of enqueues + // could keep starving the dispatcher, so only (re)start a timer that has actually stopped. + if (requestLoadQueue.isEmpty()) { + dispatchTimer.stop(); + // Forget per-second allowances once nothing is pending: a stale zero would otherwise delay + // the next single request by a full quota-reset interval. + hostQuotaRemaining.clear(); + } else if (!dispatchTimer.isActive()) { + dispatchTimer.start(); + } + + // The quota timer resets allowances every second and is also the only thing that heals a host + // after a 429 (see resetRequestQuota). It must keep ticking while work is queued or a host is + // still recovering below the ceiling, and only winds down once no host needs recovery anymore. + // Keeping it alive during such idle periods lets reduced quotas recover as intended. + bool hostRecovering = false; + for (auto it = hostRequestQuota.cbegin(); it != hostRequestQuota.cend(); ++it) { + if (it.value() < MAX_REQUESTS_PER_SEC) { + hostRecovering = true; + break; + } + } + + if (!requestLoadQueue.isEmpty() || hostRecovering) { + if (!requestTimer.isActive()) { + requestTimer.start(); + } + } else if (requestTimer.isActive()) { + requestTimer.stop(); + } +} + bool CardPictureLoaderWorker::processSingleRequest() { for (int i = 0; i < requestLoadQueue.size(); ++i) { 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 f0fe17976..2a13e847c 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 @@ -86,7 +86,7 @@ public slots: */ QNetworkReply *makeRequest(const QUrl &url, CardPictureLoaderWorkerWork *workThread); - /** @brief Starts the pacing timers if there is queued work, stops them when the queue is empty. */ + /** @brief Ensures the pacing and quota-reset timers reflect the current queue and recovery state. */ void processQueuedRequests(); /** @brief Chooses a request from the queue and starts it, respecting the quota and pacing. */ @@ -142,6 +142,9 @@ private: /** @brief Removes stale redirect entries older than TTL. */ void cleanStaleEntries(); + /** @brief Starts or stops the pacing and quota-reset timers to match the queue and recovery state. */ + void updateTimerState(); + private slots: /** @brief Resets the request quota for rate-limiting. */ void resetRequestQuota();