diff --git a/ChangeLog b/ChangeLog index 5aa482fcd..ba211d0a2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -31,6 +31,7 @@ Unreleased Version 2.2.2-pre * Fix: In layer view mode, render the layer truly in isolation instead of applying opacities, visibilities or alpha preserve to it. Thanks MachKerman and incoheart for reporting. * Feature: Add Layer > Group View to show the parent group of the current layer in isolation. Thanks Rylan for suggesting. * Fix: Save and export images according to the current view mode. Thanks incoheart for reporting. + * Server Fix: Don't get announcement refreshes stuck in an infinite loop. Thanks Meru for reporting. 2024-08-09 Version 2.2.2-beta.3 * Fix: Use more accurate timers for performance profiles if the platform supports it. diff --git a/src/libserver/announcements.cpp b/src/libserver/announcements.cpp index 0378fa17f..e03c52bcf 100644 --- a/src/libserver/announcements.cpp +++ b/src/libserver/announcements.cpp @@ -171,120 +171,124 @@ void Announcements::timerEvent(QTimerEvent *event) void Announcements::refreshListings() { - QVector> updates; - QUrl refreshServer; - bool needsSecondPass = false; + using Update = QPair; + using Updates = QVector; + using UpdatesByServer = QHash; - // Gather a list of announcements that need refreshing + UpdatesByServer serverUpdates; for(Listing &listing : m_announcements) { - bool shouldRefresh = listing.finishedListing && ( - listing.refreshTimer.hasExpired(listing.announcement.refreshInterval * 60 * 1000) || - listing.session->hasUrgentAnnouncementChange(listing.description) || - refreshServer == listing.listServer); - + bool shouldRefresh = + listing.finishedListing && + (listing.refreshTimer.hasExpired( + listing.announcement.refreshInterval * 60 * 1000) || + listing.session->hasUrgentAnnouncementChange(listing.description)); if(shouldRefresh) { - Q_ASSERT(listing.announcement.apiUrl == listing.listServer); - - // The bulk update function can only update one server at a time. - // Therefore, if there are refreshable listings for more than one server, - // we'll call refreshListings recursively until everything has been updated. - // Also, if there is at least one session that needs refreshing, we'll refresh - // all sessions listed at the same listserver. This way, the refresh intervals - // sync up and we minimize the number of requests we need to make. - if(!refreshServer.isValid()) { - refreshServer = listing.listServer; - - } else if(refreshServer != listing.listServer) { - needsSecondPass = true; - continue; - } - - Session description = listing.session->getSessionAnnouncement(); - - updates << QPair { - listing.announcement, - description - }; - + serverUpdates[listing.listServer].append( + {listing.announcement, + listing.session->getSessionAnnouncement()}); listing.refreshTimer.start(); } } - if(updates.isEmpty()) - return; - - // Bulk refresh - server::Log() - .about(server::Log::Level::Info, server::Log::Topic::PubList) - .message(QStringLiteral("Refreshing %1 announcements at %2").arg(updates.size()).arg(updates.first().first.apiUrl.toString())) - .to(m_config->logger()); - - auto *response = sessionlisting::refreshSessions(updates); - - connect(response, &AnnouncementApiResponse::finished, [this, refreshServer, updates, response](const QVariant &result, const QString &message, const QString &error) { - response->deleteLater(); - - if(!message.isEmpty()) { - server::Log() - .about(server::Log::Level::Info, server::Log::Topic::PubList) - .message(message) - .to(m_config->logger()); - } - - if(!error.isEmpty()) { - // If bulk refresh fails, there is a serious problem - // with the server. Remove all listings. - server::Log() - .about(server::Log::Level::Warn, server::Log::Topic::PubList) - .message(refreshServer.toString() + ": bulk refresh error: " + error) - .to(m_config->logger()); - - unlistSession(nullptr, refreshServer, false); - return; - } + for(UpdatesByServer::const_iterator it = serverUpdates.constBegin(), + end = serverUpdates.constEnd(); + it != end; ++it) { - const QVariantHash results = result.toHash(); - const QVariantHash responses = results["responses"].toHash(); - const QVariantHash errors = results["errors"].toHash(); + const QUrl &refreshServer = it.key(); + const Updates &updates = it.value(); - for(const auto &pair : updates) { - Q_ASSERT(pair.first.apiUrl == refreshServer); - - Listing *listing = findListingById(refreshServer, pair.first.listingId); - if(!listing) { - // Whoops! Looks like this session has ended - continue; - } - - listing->description = pair.second; - - // Remove missing listings - const QString listingId = QString::number(listing->announcement.listingId); - const QString resultValue = responses.value(listingId).toString(); - if(resultValue != "ok") { - QString errorMessage = errors[listingId].toString(); - server::Log() - .about(server::Log::Level::Warn, server::Log::Topic::PubList) - .session(listing->announcement.id) - .message(QStringLiteral("%1: %2 (%3)").arg( - listing->listServer.toString(), resultValue, - errorMessage.isEmpty() ? "no error message" : errorMessage)) - .to(m_config->logger()); - - QString announcementErrorMessage = - QStringLiteral("The session has been delisted from %1: %2").arg( - listing->listServer.host(), - errorMessage.isEmpty() ? "no reason given" : errorMessage); - emit announcementError(listing->session, announcementErrorMessage); - - unlistSession(listing->session, listing->listServer, false); - } - } - }); + server::Log() + .about(server::Log::Level::Info, server::Log::Topic::PubList) + .message(QStringLiteral("Refreshing %1 announcements at %2") + .arg(updates.size()) + .arg(updates.first().first.apiUrl.toString())) + .to(m_config->logger()); - // If there were refreshes for more than one server, do them next - if(needsSecondPass) - refreshListings(); + AnnouncementApiResponse *response = + sessionlisting::refreshSessions(updates); + + connect( + response, &AnnouncementApiResponse::finished, + [this, refreshServer, updates, response]( + const QVariant &result, const QString &message, + const QString &error) { + response->deleteLater(); + + if(!message.isEmpty()) { + server::Log() + .about( + server::Log::Level::Info, + server::Log::Topic::PubList) + .message(message) + .to(m_config->logger()); + } + + if(!error.isEmpty()) { + // If bulk refresh fails, there is a serious problem + // with the server. Remove all listings. + server::Log() + .about( + server::Log::Level::Warn, + server::Log::Topic::PubList) + .message(QStringLiteral("%1: bulk refresh error: %2") + .arg(refreshServer.toString(), error)) + .to(m_config->logger()); + unlistSession(nullptr, refreshServer, false); + return; + } + + const QVariantHash results = result.toHash(); + const QVariantHash responses = results["responses"].toHash(); + const QVariantHash errors = results["errors"].toHash(); + for(const Update &pair : updates) { + Q_ASSERT(pair.first.apiUrl == refreshServer); + + Listing *listing = + findListingById(refreshServer, pair.first.listingId); + if(!listing) { + // Whoops! Looks like this session has ended + continue; + } + + listing->description = pair.second; + + // Remove missing listings + const QString listingId = + QString::number(listing->announcement.listingId); + const QString resultValue = + responses.value(listingId).toString(); + if(resultValue != QStringLiteral("ok")) { + QString errorMessage = errors[listingId].toString(); + server::Log() + .about( + server::Log::Level::Warn, + server::Log::Topic::PubList) + .session(listing->announcement.id) + .message(QStringLiteral("%1: %2 (%3)") + .arg( + listing->listServer.toString(), + resultValue, + errorMessage.isEmpty() + ? "no error message" + : errorMessage)) + .to(m_config->logger()); + + QString announcementErrorMessage = + QStringLiteral( + "The session has been delisted from %1: %2") + .arg( + listing->listServer.host(), + errorMessage.isEmpty() ? "no reason given" + : errorMessage); + emit announcementError( + listing->session, announcementErrorMessage); + + unlistSession( + listing->session, listing->listServer, false); + } + } + }); + } } QVector Announcements::getAnnouncements(const Announcable *session) const