Merge remote-tracking branch 'upstream/master' into chatterino7

.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

.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

.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.7
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
           plugins: gcov

CHANGELOG.md

@@ -67,7 +67,9 @@
 - 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)
 - Bugfix: Fixed 7TV emotes messing with Qt's HTML. (#5677)
 - Bugfix: Fixed incorrect messages getting replaced visually. (#5683)
@@ -128,9 +130,11 @@
 - 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)
+- Dev: Moved to condition variables when shutting down worker threads. (#5721)
 
 ## 2.5.1
 

src/CMakeLists.txt

@@ -533,6 +533,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

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);
 }

src/main.cpp

@@ -49,13 +49,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
         {
@@ -82,12 +85,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
     {

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 <pajlada/signals/signal.hpp>
 #include <QJsonObject>
+#include <QScopeGuard>
 #include <QString>
 #include <QStringBuilder>
 #include <websocketpp/client.hpp>
@@ -120,6 +122,11 @@ class BasicPubSubManager
         this->work_ = std::make_shared<boost::asio::io_service::work>(
             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<std::thread> mainThread_;
+    OnceFlag stoppedFlag_;
 
     const QString host_;
 

src/providers/twitch/PubSubManager.cpp

@@ -15,10 +15,10 @@
 #include "util/RenameThread.hpp"
 
 #include <QJsonArray>
+#include <QScopeGuard>
 
 #include <algorithm>
 #include <exception>
-#include <future>
 #include <iostream>
 #include <memory>
 #include <thread>
@@ -560,6 +560,11 @@ void PubSub::start()
     this->work = std::make_shared<boost::asio::io_service::work>(
         this->websocketClient.get_io_service());
     this->thread = std::make_unique<std::thread>([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()

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 <boost/asio/io_service.hpp>
 #include <boost/asio/ssl/context.hpp>
@@ -267,6 +268,8 @@ class PubSub
     const QString host_;
     const PubSubClientOptions clientOptions_;
 
+    OnceFlag stoppedFlag_;
+
     bool stopping_{false};
 
 #ifdef FRIEND_TEST

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

src/singletons/Toasts.hpp

@@ -3,6 +3,8 @@
 #include <pajlada/settings/setting.hpp>
 #include <QString>
 
+#include <cstdint>
+
 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);

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