From ed6ddead3dc7016b7b94c50942ab67606d3255bc Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 18 Nov 2024 08:17:56 +0100 Subject: [PATCH 1/8] chore(deps): bump codecov/codecov-action from 5.0.0 to 5.0.2 (#5717) --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1ce616951ee..4c32d569ee9 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -96,7 +96,7 @@ jobs: working-directory: build-test - name: Upload coverage reports to Codecov - uses: codecov/codecov-action@v5.0.0 + uses: codecov/codecov-action@v5.0.2 with: token: ${{ secrets.CODECOV_TOKEN }} plugins: gcov From 0ed8311c1a8c4b9a1854a3182bd36c9fbeb8eacc Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 19 Nov 2024 04:22:29 +0000 Subject: [PATCH 2/8] chore(deps): bump lib/settings from `4a0a1e5` to `46eb250` (#5718) --- lib/settings | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/settings b/lib/settings index 4a0a1e59937..46eb2509eb3 160000 --- a/lib/settings +++ b/lib/settings @@ -1 +1 @@ -Subproject commit 4a0a1e599377cdcdc91b0fbbefc312936b48730c +Subproject commit 46eb2509eb37a1c8b593d37bc51359ec2f31a541 From 19f449866e1e7f77e213b6224a39d9fd974de902 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 19 Nov 2024 15:12:25 +0000 Subject: [PATCH 3/8] chore(deps): bump ZedThree/clang-tidy-review from 0.20.0 to 0.20.1 (#5719) --- .github/workflows/clang-tidy.yml | 4 ++-- .github/workflows/post-clang-tidy-review.yml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/clang-tidy.yml b/.github/workflows/clang-tidy.yml index 54111c29a81..9e0e286e429 100644 --- a/.github/workflows/clang-tidy.yml +++ b/.github/workflows/clang-tidy.yml @@ -41,7 +41,7 @@ jobs: - name: clang-tidy review timeout-minutes: 20 - uses: ZedThree/clang-tidy-review@v0.20.0 + uses: ZedThree/clang-tidy-review@v0.20.1 with: build_dir: build-clang-tidy config_file: ".clang-tidy" @@ -63,4 +63,4 @@ jobs: libxkbcommon-x11-0, libxcb-xkb-dev, libxcb-cursor0 - name: clang-tidy-review upload - uses: ZedThree/clang-tidy-review/upload@v0.20.0 + uses: ZedThree/clang-tidy-review/upload@v0.20.1 diff --git a/.github/workflows/post-clang-tidy-review.yml b/.github/workflows/post-clang-tidy-review.yml index df8ce219c2c..e2eadad5e6b 100644 --- a/.github/workflows/post-clang-tidy-review.yml +++ b/.github/workflows/post-clang-tidy-review.yml @@ -14,7 +14,7 @@ jobs: if: ${{ github.event.workflow_run.conclusion == 'success' }} steps: - - uses: ZedThree/clang-tidy-review/post@v0.20.0 + - uses: ZedThree/clang-tidy-review/post@v0.20.1 with: lgtm_comment_body: "" num_comments_as_exitcode: false From 1c827f6288e32e397418c7815f8123ab6c0acb52 Mon Sep 17 00:00:00 2001 From: nerix Date: Wed, 20 Nov 2024 22:29:47 +0100 Subject: [PATCH 4/8] chore: use condition variable to shutdown websocket pools (#5721) --- CHANGELOG.md | 1 + src/CMakeLists.txt | 2 + .../liveupdates/BasicPubSubManager.hpp | 48 +++++++--- src/providers/twitch/PubSubManager.cpp | 50 +++++++---- src/providers/twitch/PubSubManager.hpp | 3 + src/util/OnceFlag.cpp | 33 +++++++ src/util/OnceFlag.hpp | 41 +++++++++ tests/CMakeLists.txt | 1 + tests/src/OnceFlag.cpp | 88 +++++++++++++++++++ 9 files changed, 237 insertions(+), 30 deletions(-) create mode 100644 src/util/OnceFlag.cpp create mode 100644 src/util/OnceFlag.hpp create mode 100644 tests/src/OnceFlag.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b3d80fd5f3..7428d23d0b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -131,6 +131,7 @@ - Dev: Unified parsing of historic and live IRC messages. (#5678) - Dev: 7TV's `entitlement.reset` is now explicitly ignored. (#5685) - Dev: Qt 6.8 and later now default to the GDI fontengine. (#5710) +- Dev: Moved to condition variables when shutting down worker threads. (#5721) ## 2.5.1 diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 04b863c7995..61c19e011bc 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -514,6 +514,8 @@ set(SOURCE_FILES util/LayoutHelper.hpp util/LoadPixmap.cpp util/LoadPixmap.hpp + util/OnceFlag.cpp + util/OnceFlag.hpp util/RapidjsonHelpers.cpp util/RapidjsonHelpers.hpp util/RatelimitBucket.cpp diff --git a/src/providers/liveupdates/BasicPubSubManager.hpp b/src/providers/liveupdates/BasicPubSubManager.hpp index e8d95b52872..299c84bc08e 100644 --- a/src/providers/liveupdates/BasicPubSubManager.hpp +++ b/src/providers/liveupdates/BasicPubSubManager.hpp @@ -8,10 +8,12 @@ #include "providers/twitch/PubSubHelpers.hpp" #include "util/DebugCount.hpp" #include "util/ExponentialBackoff.hpp" +#include "util/OnceFlag.hpp" #include "util/RenameThread.hpp" #include #include +#include #include #include #include @@ -120,6 +122,11 @@ class BasicPubSubManager this->work_ = std::make_shared( this->websocketClient_.get_io_service()); this->mainThread_.reset(new std::thread([this] { + // make sure we set in any case, even exceptions + auto guard = qScopeGuard([&] { + this->stoppedFlag_.set(); + }); + runThread(); })); @@ -142,22 +149,34 @@ class BasicPubSubManager this->work_.reset(); - if (this->mainThread_->joinable()) + if (!this->mainThread_->joinable()) { - // NOTE: We spawn a new thread to join the websocket thread. - // There is a case where a new client was initiated but not added to the clients list. - // We just don't join the thread & let the operating system nuke the thread if joining fails - // within 1s. - auto joiner = std::async(std::launch::async, &std::thread::join, - this->mainThread_.get()); - if (joiner.wait_for(std::chrono::seconds(1)) == - std::future_status::timeout) - { - qCWarning(chatterinoLiveupdates) - << "Thread didn't join within 1 second, rip it out"; - this->websocketClient_.stop(); - } + return; } + + // NOTE: + // There is a case where a new client was initiated but not added to the clients list. + // We just don't join the thread & let the operating system nuke the thread if joining fails + // within 1s. + if (this->stoppedFlag_.waitFor(std::chrono::seconds{1})) + { + this->mainThread_->join(); + return; + } + + qCWarning(chatterinoLiveupdates) + << "Thread didn't finish within 1 second, force-stop the client"; + this->websocketClient_.stop(); + if (this->stoppedFlag_.waitFor(std::chrono::milliseconds{100})) + { + this->mainThread_->join(); + return; + } + + qCWarning(chatterinoLiveupdates) + << "Thread didn't finish after stopping, discard it"; + // detach the thread so the destructor doesn't attempt any joining + this->mainThread_->detach(); } protected: @@ -394,6 +413,7 @@ class BasicPubSubManager liveupdates::WebsocketClient websocketClient_; std::unique_ptr mainThread_; + OnceFlag stoppedFlag_; const QString host_; diff --git a/src/providers/twitch/PubSubManager.cpp b/src/providers/twitch/PubSubManager.cpp index 9a2f0384838..ba41a7b501d 100644 --- a/src/providers/twitch/PubSubManager.cpp +++ b/src/providers/twitch/PubSubManager.cpp @@ -15,10 +15,10 @@ #include "util/RenameThread.hpp" #include +#include #include #include -#include #include #include #include @@ -560,6 +560,11 @@ void PubSub::start() this->work = std::make_shared( this->websocketClient.get_io_service()); this->thread = std::make_unique([this] { + // make sure we set in any case, even exceptions + auto guard = qScopeGuard([&] { + this->stoppedFlag_.set(); + }); + runThread(); }); renameThread(*this->thread, "PubSub"); @@ -578,23 +583,36 @@ void PubSub::stop() this->work.reset(); - if (this->thread->joinable()) + if (!this->thread->joinable()) { - // NOTE: We spawn a new thread to join the websocket thread. - // There is a case where a new client was initiated but not added to the clients list. - // We just don't join the thread & let the operating system nuke the thread if joining fails - // within 1s. - // We could fix the underlying bug, but this is easier & we realistically won't use this exact code - // for super much longer. - auto joiner = std::async(std::launch::async, &std::thread::join, - this->thread.get()); - if (joiner.wait_for(1s) == std::future_status::timeout) - { - qCWarning(chatterinoPubSub) - << "Thread didn't join within 1 second, rip it out"; - this->websocketClient.stop(); - } + return; } + + // NOTE: + // There is a case where a new client was initiated but not added to the clients list. + // We just don't join the thread & let the operating system nuke the thread if joining fails + // within 1s. + // We could fix the underlying bug, but this is easier & we realistically won't use this exact code + // for super much longer. + if (this->stoppedFlag_.waitFor(std::chrono::seconds{1})) + { + this->thread->join(); + return; + } + + qCWarning(chatterinoLiveupdates) + << "Thread didn't finish within 1 second, force-stop the client"; + this->websocketClient.stop(); + if (this->stoppedFlag_.waitFor(std::chrono::milliseconds{100})) + { + this->thread->join(); + return; + } + + qCWarning(chatterinoLiveupdates) + << "Thread didn't finish after stopping, discard it"; + // detach the thread so the destructor doesn't attempt any joining + this->thread->detach(); } bool PubSub::listenToWhispers() diff --git a/src/providers/twitch/PubSubManager.hpp b/src/providers/twitch/PubSubManager.hpp index f14eabf7741..bfa228719e2 100644 --- a/src/providers/twitch/PubSubManager.hpp +++ b/src/providers/twitch/PubSubManager.hpp @@ -3,6 +3,7 @@ #include "providers/twitch/PubSubClientOptions.hpp" #include "providers/twitch/PubSubWebsocket.hpp" #include "util/ExponentialBackoff.hpp" +#include "util/OnceFlag.hpp" #include #include @@ -267,6 +268,8 @@ class PubSub const QString host_; const PubSubClientOptions clientOptions_; + OnceFlag stoppedFlag_; + bool stopping_{false}; #ifdef FRIEND_TEST diff --git a/src/util/OnceFlag.cpp b/src/util/OnceFlag.cpp new file mode 100644 index 00000000000..cb42ea5abbf --- /dev/null +++ b/src/util/OnceFlag.cpp @@ -0,0 +1,33 @@ +#include "util/OnceFlag.hpp" + +namespace chatterino { + +OnceFlag::OnceFlag() = default; +OnceFlag::~OnceFlag() = default; + +void OnceFlag::set() +{ + { + std::unique_lock guard(this->mutex); + this->flag = true; + } + this->condvar.notify_all(); +} + +bool OnceFlag::waitFor(std::chrono::milliseconds ms) +{ + std::unique_lock lock(this->mutex); + return this->condvar.wait_for(lock, ms, [this] { + return this->flag; + }); +} + +void OnceFlag::wait() +{ + std::unique_lock lock(this->mutex); + this->condvar.wait(lock, [this] { + return this->flag; + }); +} + +} // namespace chatterino diff --git a/src/util/OnceFlag.hpp b/src/util/OnceFlag.hpp new file mode 100644 index 00000000000..4060dff2976 --- /dev/null +++ b/src/util/OnceFlag.hpp @@ -0,0 +1,41 @@ +#pragma once + +#include +#include +#include + +namespace chatterino { + +/// @brief A flag that can only be set once which notifies waiters. +/// +/// This can be used to synchronize with other threads. Note that waiting +/// threads will be suspended. +class OnceFlag +{ +public: + OnceFlag(); + ~OnceFlag(); + + /// Set this flag and notify waiters + void set(); + + /// @brief Wait for at most `ms` until this flag is set. + /// + /// The calling thread will be suspended during the wait. + /// + /// @param ms The maximum time to wait for this flag + /// @returns `true` if this flag was set during the wait or before + bool waitFor(std::chrono::milliseconds ms); + + /// @brief Wait until this flag is set by another thread + /// + /// The calling thread will be suspended during the wait. + void wait(); + +private: + std::mutex mutex; + std::condition_variable condvar; + bool flag = false; +}; + +} // namespace chatterino diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 547f0e7c050..06349c0e0a4 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -51,6 +51,7 @@ set(test_SOURCES ${CMAKE_CURRENT_LIST_DIR}/src/Plugins.cpp ${CMAKE_CURRENT_LIST_DIR}/src/TwitchIrc.cpp ${CMAKE_CURRENT_LIST_DIR}/src/IgnoreController.cpp + ${CMAKE_CURRENT_LIST_DIR}/src/OnceFlag.cpp ${CMAKE_CURRENT_LIST_DIR}/src/lib/Snapshot.cpp ${CMAKE_CURRENT_LIST_DIR}/src/lib/Snapshot.hpp # Add your new file above this line! diff --git a/tests/src/OnceFlag.cpp b/tests/src/OnceFlag.cpp new file mode 100644 index 00000000000..0b0dded299b --- /dev/null +++ b/tests/src/OnceFlag.cpp @@ -0,0 +1,88 @@ +#include "util/OnceFlag.hpp" + +#include "Test.hpp" + +#include + +using namespace chatterino; + +// this test shouldn't time out (no assert necessary) +TEST(OnceFlag, basic) +{ + OnceFlag startedFlag; + OnceFlag startedAckFlag; + OnceFlag stoppedFlag; + + std::thread t([&] { + startedFlag.set(); + startedAckFlag.wait(); + std::this_thread::sleep_for(std::chrono::milliseconds{50}); + stoppedFlag.set(); + }); + + startedFlag.wait(); + startedAckFlag.set(); + stoppedFlag.wait(); + + t.join(); +} + +TEST(OnceFlag, waitFor) +{ + OnceFlag startedFlag; + OnceFlag startedAckFlag; + OnceFlag stoppedFlag; + + std::thread t([&] { + startedFlag.set(); + startedAckFlag.wait(); + + std::this_thread::sleep_for(std::chrono::milliseconds{100}); + stoppedFlag.set(); + }); + + startedFlag.wait(); + startedAckFlag.set(); + + auto start = std::chrono::system_clock::now(); + ASSERT_TRUE(stoppedFlag.waitFor(std::chrono::milliseconds{200})); + auto stop = std::chrono::system_clock::now(); + + ASSERT_LT(stop - start, std::chrono::milliseconds{150}); + + start = std::chrono::system_clock::now(); + ASSERT_TRUE(stoppedFlag.waitFor(std::chrono::milliseconds{1000})); + stop = std::chrono::system_clock::now(); + + ASSERT_LT(stop - start, std::chrono::milliseconds{10}); + + start = std::chrono::system_clock::now(); + stoppedFlag.wait(); + stop = std::chrono::system_clock::now(); + + ASSERT_LT(stop - start, std::chrono::milliseconds{10}); + + t.join(); +} + +TEST(OnceFlag, waitForTimeout) +{ + OnceFlag startedFlag; + OnceFlag startedAckFlag; + OnceFlag stoppedFlag; + + std::thread t([&] { + startedFlag.set(); + startedAckFlag.wait(); + std::this_thread::sleep_for(std::chrono::milliseconds{100}); + stoppedFlag.set(); + }); + + startedFlag.wait(); + startedAckFlag.set(); + + ASSERT_FALSE(stoppedFlag.waitFor(std::chrono::milliseconds{25})); + stoppedFlag.wait(); + + t.join(); +} From 2a38e39e24200ae32facd83c1b3301ae113a994b Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 21 Nov 2024 11:08:33 +0100 Subject: [PATCH 5/8] chore(deps): bump codecov/codecov-action from 5.0.2 to 5.0.7 (#5722) Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 5.0.2 to 5.0.7. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/codecov/codecov-action/compare/v5.0.2...v5.0.7) --- updated-dependencies: - dependency-name: codecov/codecov-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4c32d569ee9..b1b6bd31e59 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -96,7 +96,7 @@ jobs: working-directory: build-test - name: Upload coverage reports to Codecov - uses: codecov/codecov-action@v5.0.2 + uses: codecov/codecov-action@v5.0.7 with: token: ${{ secrets.CODECOV_TOKEN }} plugins: gcov From a8269a5749a45febc3447c404103e4d5826c4677 Mon Sep 17 00:00:00 2001 From: pajlada Date: Sat, 23 Nov 2024 14:20:44 +0100 Subject: [PATCH 6/8] fix: output --version to stdout (#5727) --- CHANGELOG.md | 1 + src/main.cpp | 21 +++++++++++++-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7428d23d0b6..5fea06b0068 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -128,6 +128,7 @@ - Dev: Decoupled reply parsing from `MessageBuilder`. (#5660, #5668) - Dev: Refactored IRC message building. (#5663) - Dev: Fixed some compiler warnings. (#5672) +- Dev: Explicitly print output from `--version` to `stdout`. (#5727) - Dev: Unified parsing of historic and live IRC messages. (#5678) - Dev: 7TV's `entitlement.reset` is now explicitly ignored. (#5685) - Dev: Qt 6.8 and later now default to the GDI fontengine. (#5710) diff --git a/src/main.cpp b/src/main.cpp index 4ece0deb871..2b7fc924e28 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -44,13 +44,16 @@ int main(int argc, char **argv) QMessageBox box; if (Modes::instance().isPortable) { - box.setText( + auto errorMessage = error.what() + QStringLiteral( "\n\nInfo: Portable mode requires the application to " "be in a writeable location. If you don't want " "portable mode reinstall the application. " - "https://chatterino.com.")); + "https://chatterino.com."); + std::cerr << errorMessage.toLocal8Bit().constData() << '\n'; + std::cerr.flush(); + box.setText(errorMessage); } else { @@ -77,12 +80,14 @@ int main(int argc, char **argv) attachToConsole(); auto version = Version::instance(); - qInfo().noquote() << QString("%1 (commit %2%3)") - .arg(version.fullVersion()) - .arg(version.commitHash()) - .arg(Modes::instance().isNightly - ? ", " + version.dateOfBuild() - : ""); + auto versionMessage = + QString("%1 (commit %2%3)") + .arg(version.fullVersion()) + .arg(version.commitHash()) + .arg(Modes::instance().isNightly ? ", " + version.dateOfBuild() + : ""); + std::cout << versionMessage.toLocal8Bit().constData() << '\n'; + std::cout.flush(); } else { From b4ff1286c79671bd7cf5e6959599fb9eeea03d06 Mon Sep 17 00:00:00 2001 From: pajlada Date: Sat, 23 Nov 2024 15:45:01 +0100 Subject: [PATCH 7/8] fix: clear WinToast on exit (#5728) --- CHANGELOG.md | 1 + src/singletons/Toasts.cpp | 7 +++++++ src/singletons/Toasts.hpp | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fea06b0068..587e24f7868 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,7 @@ - Bugfix: Fixed grammar in the user highlight page. (#5602) - Bugfix: Fixed incorrect message being disabled in some cases upon approving or denying an automod caught message. (#5611) - Bugfix: Fixed double-click selection not working when clicking outside a message. (#5617) +- Bugfix: Fixed a potential rare crash that could occur on Windows if a toast was about to fire just as we were shutting down. (#5728) - Bugfix: Fixed emotes starting with ":" not tab-completing. (#5603) - Bugfix: Fixed 7TV emotes messing with Qt's HTML. (#5677) - Bugfix: Fixed incorrect messages getting replaced visually. (#5683) diff --git a/src/singletons/Toasts.cpp b/src/singletons/Toasts.cpp index 7e21dc2c315..efcb908ba5c 100644 --- a/src/singletons/Toasts.cpp +++ b/src/singletons/Toasts.cpp @@ -70,6 +70,13 @@ using WinToastLib::WinToast; using WinToastLib::WinToastTemplate; #endif +Toasts::~Toasts() +{ +#ifdef Q_OS_WIN + WinToast::instance()->clear(); +#endif +} + bool Toasts::isEnabled() { #ifdef Q_OS_WIN diff --git a/src/singletons/Toasts.hpp b/src/singletons/Toasts.hpp index e4c2ad23467..b57f5c3f995 100644 --- a/src/singletons/Toasts.hpp +++ b/src/singletons/Toasts.hpp @@ -3,6 +3,8 @@ #include #include +#include + namespace chatterino { enum class Platform : uint8_t; @@ -17,6 +19,8 @@ enum class ToastReaction { class Toasts final { public: + ~Toasts(); + void sendChannelNotification(const QString &channelName, const QString &channelTitle, Platform p); static QString findStringFromReaction(const ToastReaction &reaction); From 14c4bb64595d9df0c463701cb9c7075730c39235 Mon Sep 17 00:00:00 2001 From: pajlada Date: Sat, 23 Nov 2024 16:29:44 +0100 Subject: [PATCH 8/8] fix: network requests timing out too early (#5729) --- CHANGELOG.md | 1 + src/common/network/NetworkTask.cpp | 25 ++++++++++++---- tests/src/NetworkHelpers.hpp | 9 ++++++ tests/src/NetworkRequest.cpp | 46 ++++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 587e24f7868..574f3ba6288 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,7 @@ - Bugfix: Fixed global badges not showing in anonymous mode. (#5599) - Bugfix: Fixed grammar in the user highlight page. (#5602) - Bugfix: Fixed incorrect message being disabled in some cases upon approving or denying an automod caught message. (#5611) +- Bugfix: Fixed network requests timing out despite them not being in flight for that long, for Qt 6.3+ where we have the technology. (#5729) - Bugfix: Fixed double-click selection not working when clicking outside a message. (#5617) - Bugfix: Fixed a potential rare crash that could occur on Windows if a toast was about to fire just as we were shutting down. (#5728) - Bugfix: Fixed emotes starting with ":" not tab-completing. (#5603) diff --git a/src/common/network/NetworkTask.cpp b/src/common/network/NetworkTask.cpp index bebe3405123..7bc1330c6b3 100644 --- a/src/common/network/NetworkTask.cpp +++ b/src/common/network/NetworkTask.cpp @@ -30,22 +30,35 @@ NetworkTask::~NetworkTask() void NetworkTask::run() { + this->reply_ = this->createReply(); + if (!this->reply_) + { + this->deleteLater(); + return; + } + const auto &timeout = this->data_->timeout; if (timeout.has_value()) { +#if QT_VERSION >= QT_VERSION_CHECK(6, 3, 0) + QObject::connect(this->reply_, &QNetworkReply::requestSent, this, + [this]() { + const auto &timeout = this->data_->timeout; + this->timer_ = new QTimer(this); + this->timer_->setSingleShot(true); + this->timer_->start(timeout.value()); + QObject::connect(this->timer_, &QTimer::timeout, + this, &NetworkTask::timeout); + }); +#else this->timer_ = new QTimer(this); this->timer_->setSingleShot(true); this->timer_->start(timeout.value()); QObject::connect(this->timer_, &QTimer::timeout, this, &NetworkTask::timeout); +#endif } - this->reply_ = this->createReply(); - if (!this->reply_) - { - this->deleteLater(); - return; - } QObject::connect(this->reply_, &QNetworkReply::finished, this, &NetworkTask::finished); } diff --git a/tests/src/NetworkHelpers.hpp b/tests/src/NetworkHelpers.hpp index f55355e4a54..7125deb7da3 100644 --- a/tests/src/NetworkHelpers.hpp +++ b/tests/src/NetworkHelpers.hpp @@ -1,7 +1,15 @@ #pragma once + #include "Test.hpp" #include +#include +#include + +#include +#include +#include + namespace chatterino { #ifdef CHATTERINO_TEST_USE_PUBLIC_HTTPBIN @@ -52,4 +60,5 @@ class RequestWaiter std::condition_variable condition_; bool requestDone_ = false; }; + } // namespace chatterino diff --git a/tests/src/NetworkRequest.cpp b/tests/src/NetworkRequest.cpp index 44b7504c7d3..a81059e4841 100644 --- a/tests/src/NetworkRequest.cpp +++ b/tests/src/NetworkRequest.cpp @@ -231,3 +231,49 @@ TEST(NetworkRequest, FinallyCallbackOnTimeout) EXPECT_FALSE(onSuccessCalled); EXPECT_TRUE(NetworkManager::workerThread->isRunning()); } + +/// Ensure timeouts don't expire early just because their request took a bit longer to actually fire +/// +/// We need to ensure all requests are "executed" before we start waiting for them +TEST(NetworkRequest, BatchedTimeouts) +{ +#if QT_VERSION >= QT_VERSION_CHECK(6, 3, 0) + // roughly num network manager worker threads * 2 + static const auto numRequests = 10; + + struct RequestState { + RequestWaiter waiter; + bool errored = false; + }; + + EXPECT_TRUE(NetworkManager::workerThread->isRunning()); + + std::vector> states; + + for (auto i = 1; i <= numRequests; ++i) + { + auto state = std::make_shared(); + + auto url = getDelayURL(1); + + NetworkRequest(url) + .timeout(1500) + .onError([=](const NetworkResult &result) { + (void)result; + state->errored = true; + }) + .finally([=] { + state->waiter.requestDone(); + }) + .execute(); + + states.emplace_back(state); + } + + for (const auto &state : states) + { + state->waiter.waitForRequest(); + EXPECT_FALSE(state->errored); + } +#endif +}