[PictureLoader] Guard dispatch timer restarts and drop dead request quota

This commit is contained in:
Lukas Brübach 2026-09-18 03:55:45 +02:00
parent 1c6ee62393
commit 91519166f9
2 changed files with 14 additions and 11 deletions

View file

@ -23,8 +23,7 @@ static constexpr int DISPATCH_INTERVAL_MS = 100; ///< Pacing between indi
static constexpr qint64 QUOTA_RESET_INTERVAL_MS = 1000; ///< Interval at which the request quota resets static constexpr qint64 QUOTA_RESET_INTERVAL_MS = 1000; ///< Interval at which the request quota resets
CardPictureLoaderWorker::CardPictureLoaderWorker() CardPictureLoaderWorker::CardPictureLoaderWorker()
: QObject(nullptr), picDownload(SettingsCache::instance().downloads().getPicDownload()), : QObject(nullptr), picDownload(SettingsCache::instance().downloads().getPicDownload())
requestQuota(MAX_REQUESTS_PER_SEC)
{ {
networkManager = new QNetworkAccessManager(this); networkManager = new QNetworkAccessManager(this);
// We need a timeout to ensure requests don't hang indefinitely in case of // We need a timeout to ensure requests don't hang indefinitely in case of
@ -137,8 +136,6 @@ QNetworkReply *CardPictureLoaderWorker::makeRequest(const QUrl &url, CardPicture
void CardPictureLoaderWorker::resetRequestQuota() void CardPictureLoaderWorker::resetRequestQuota()
{ {
requestQuota = MAX_REQUESTS_PER_SEC;
QDateTime now = QDateTime::currentDateTime(); QDateTime now = QDateTime::currentDateTime();
for (auto it = hostRequestQuota.begin(); it != hostRequestQuota.end(); ++it) { for (auto it = hostRequestQuota.begin(); it != hostRequestQuota.end(); ++it) {
if (!hostLast429.contains(it.key()) || now.msecsTo(hostLast429.value(it.key())) < -QUOTA_RECOVER_MS) { if (!hostLast429.contains(it.key()) || now.msecsTo(hostLast429.value(it.key())) < -QUOTA_RECOVER_MS) {
@ -156,27 +153,34 @@ void CardPictureLoaderWorker::resetRequestQuota()
void CardPictureLoaderWorker::processQueuedRequests() void CardPictureLoaderWorker::processQueuedRequests()
{ {
Q_ASSERT(thread() == QThread::currentThread());
if (requestLoadQueue.isEmpty()) { if (requestLoadQueue.isEmpty()) {
dispatchTimer.stop(); dispatchTimer.stop();
requestTimer.stop();
return; return;
} }
// Start lazily from the worker's own thread: QTimer must be started in the thread it lives in. // Start lazily from the worker's own thread: QTimer must be started in the thread it lives in.
if (!requestTimer.isActive()) { if (!requestTimer.isActive()) {
requestTimer.start(); 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() void CardPictureLoaderWorker::dispatchQueuedRequest()
{ {
if (requestLoadQueue.isEmpty() || requestQuota <= 0) { if (requestLoadQueue.isEmpty()) {
// All queued requests have been dispatched; stop the pacing and quota-reset timers.
dispatchTimer.stop(); dispatchTimer.stop();
requestTimer.stop();
return; return;
} }
if (processSingleRequest()) { if (!processSingleRequest()) {
--requestQuota;
} else {
// No queued host currently has allowance left in this second; wait for the quota reset. // No queued host currently has allowance left in this second; wait for the quota reset.
dispatchTimer.stop(); dispatchTimer.stop();
} }

View file

@ -86,7 +86,7 @@ public slots:
*/ */
QNetworkReply *makeRequest(const QUrl &url, CardPictureLoaderWorkerWork *workThread); 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(); void processQueuedRequests();
/** @brief Chooses a request from the queue and starts it, respecting the quota and pacing. */ /** @brief Chooses a request from the queue and starts it, respecting the quota and pacing. */
@ -121,7 +121,6 @@ private:
bool picDownload; ///< Whether downloading images from network is enabled bool picDownload; ///< Whether downloading images from network is enabled
QQueue<QPair<QUrl, CardPictureLoaderWorkerWork *>> requestLoadQueue; ///< Queue of pending network requests QQueue<QPair<QUrl, CardPictureLoaderWorkerWork *>> requestLoadQueue; ///< Queue of pending network requests
int requestQuota; ///< Remaining requests allowed per second
QTimer requestTimer; ///< Timer to reset the request quota QTimer requestTimer; ///< Timer to reset the request quota
QTimer dispatchTimer; ///< Timer pacing individual network requests QTimer dispatchTimer; ///< Timer pacing individual network requests
QHash<QString, int> hostRequestQuota; ///< Sustained per-host request allowance QHash<QString, int> hostRequestQuota; ///< Sustained per-host request allowance