From 5d7358edc858531c080e542910a42ac061305225 Mon Sep 17 00:00:00 2001 From: Artem Lytkin Date: Thu, 20 Aug 2026 01:33:56 +0300 Subject: [PATCH] Fix #1790: discard test poll results from a finished sweep The ResultPoller ticks that drain partial results check sessionGen_ once, at the top of the tick, and then block in QueryURLTest / QueryIPTest / QueryCurrentSpeedTests / QueryCountryTestResults. ~ResultPoller only sets the stop flag and deliberately does not join, so a tick already inside its RPC outlives the sweep that created it. By the time that RPC returns the next sweep may have started: seedLatencyTest() has zeroed testProgress, and the tick then calls addTestProgress() once per result it drained. The new sweep's counter is credited with the previous run's leftovers, so the panel shows more than totalProfiles and the percentage runs past 100%. That is why it reproduces on the second Url Test Selected rather than the first. The same window misfiles results: BuildTestConfig numbers chain outbounds positionally (hopTag prefix-1, prefix-2, ...) and restarts the counter for every batch, so a stale tag2entID resolves a new sweep's tag to whatever profile held that slot before, and applyUrlResult writes the latency onto the wrong row. Re-check sessionGen_ after each query returns, before crediting progress or touching profiles. No profile data is lost by dropping a late drain: the owning sweep's Test / IPTest / SpeedTest response carries the full result set and its final pass applies all of it. The counter is a separate matter - the final pass never calls addTestProgress, so results a poll never reached already go uncounted today and the bar can stop short of 100%. This does not change that; it stops the count from being credited to the wrong run. pollSpeedTest and pollCountryTest take the generation as an argument since they run the query themselves. --- include/ui/mainWindow/TestRunner.h | 5 +++-- src/ui/mainWindow/TestRunner.cpp | 18 ++++++++++++++---- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/include/ui/mainWindow/TestRunner.h b/include/ui/mainWindow/TestRunner.h index 7d7407c1f..5993d0fc1 100644 --- a/include/ui/mainWindow/TestRunner.h +++ b/include/ui/mainWindow/TestRunner.h @@ -75,9 +75,10 @@ class TestRunner { QString contextName(int entID) const; - void pollSpeedTest(const QMap& tag2entID, bool testCurrent); + // `gen` is the sweep the poll belongs to, re-checked once the query returns. + void pollSpeedTest(const QMap& tag2entID, bool testCurrent, quint64 gen); - void pollCountryTest(const QMap& tag2entID, bool testCurrent); + void pollCountryTest(const QMap& tag2entID, bool testCurrent, quint64 gen); void creditTraffic(const std::shared_ptr& profile, const QString& tag, qint64 curUp, qint64 curDown); diff --git a/src/ui/mainWindow/TestRunner.cpp b/src/ui/mainWindow/TestRunner.cpp index d4ed17d1d..1aeb4a157 100644 --- a/src/ui/mainWindow/TestRunner.cpp +++ b/src/ui/mainWindow/TestRunner.cpp @@ -155,6 +155,10 @@ void TestRunner::runUrlProbe(const Target& target) { if (sessionGen_.load() != gen) return; bool ok = false; const auto resp = defaultClient->QueryURLTest(&ok); + // Re-checked: the tick's opening check is stale by now. A poll can sit in + // this RPC while its sweep ends and the next one zeroes the counter and + // reuses the positional outbound tags, so a late drain lands on the wrong run. + if (sessionGen_.load() != gen) return; if (!ok || resp.results.empty()) return; QList updated; @@ -217,6 +221,8 @@ void TestRunner::runIpProbe(const Target& target) { if (sessionGen_.load() != gen) return; bool ok = false; const auto resp = defaultClient->QueryIPTest(&ok); + // See runUrlProbe: the opening check cannot cover the query itself. + if (sessionGen_.load() != gen) return; if (!ok || resp.results.empty()) return; QList updated; @@ -462,10 +468,12 @@ void TestRunner::creditTraffic(const std::shared_ptr& profile, Configs::dataManager->profilesRepo->SaveTraffic(profile); } -void TestRunner::pollSpeedTest(const QMap& tag2entID, bool testCurrent) +void TestRunner::pollSpeedTest(const QMap& tag2entID, bool testCurrent, quint64 gen) { bool ok = false; const auto res = defaultClient->QueryCurrentSpeedTests(&ok); + // See runUrlProbe: the opening check cannot cover the query itself. + if (sessionGen_.load() != gen) return; if (!ok || !res.is_running.value()) { return; @@ -496,10 +504,12 @@ void TestRunner::pollSpeedTest(const QMap& tag2entID, bool testCur }); } -void TestRunner::pollCountryTest(const QMap& tag2entID, bool testCurrent) +void TestRunner::pollCountryTest(const QMap& tag2entID, bool testCurrent, quint64 gen) { bool ok = false; const auto res = defaultClient->QueryCountryTestResults(&ok); + // See runUrlProbe: the opening check cannot cover the query itself. + if (sessionGen_.load() != gen) return; if (!ok || res.results.empty()) { return; @@ -562,8 +572,8 @@ void TestRunner::runSpeedProbe(const Target& target) { ResultPoller poller([this, gen = sessionGen_.load(), tag2entID = target.tag2entID, testCurrent = target.testCurrent, speedtestConf] { if (sessionGen_.load() != gen) return; - if (speedtestConf == Configs::TestConfig::COUNTRY) pollCountryTest(tag2entID, testCurrent); - else pollSpeedTest(tag2entID, testCurrent); + if (speedtestConf == Configs::TestConfig::COUNTRY) pollCountryTest(tag2entID, testCurrent, gen); + else pollSpeedTest(tag2entID, testCurrent, gen); }, kSpeedPollIntervalMs); result = defaultClient->SpeedTest(&rpcOK, req, &coreError);