Merge 56ba1ca2ee into 2a38e39e24

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] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

.CI/full-ubuntu-build.sh

@@ -29,7 +29,6 @@ cmake \
     -DCMAKE_EXPORT_COMPILE_COMMANDS=On \
     -DCHATTERINO_PLUGINS="$C2_PLUGINS" \
     -DCMAKE_PREFIX_PATH="$Qt6_DIR/lib/cmake" \
-    -DBUILD_WITH_QT6="$C2_BUILD_WITH_QT6" \
     -DCHATTERINO_STATIC_QT_BUILD=On \
     -DCMAKE_CXX_FLAGS="-fno-sized-deallocation" \
     .

.CI/setup-clang-tidy.sh

@@ -19,7 +19,6 @@ cmake -S. -Bbuild-clang-tidy \
     -DCMAKE_EXPORT_COMPILE_COMMANDS=On \
     -DCHATTERINO_LTO=Off \
     -DCHATTERINO_PLUGINS=On \
-    -DBUILD_WITH_QT6=On \
     -DBUILD_TESTS=On \
     -DBUILD_BENCHMARKS=On
 

.cirrus.yml

@@ -20,7 +20,6 @@ task:
         -DUSE_SYSTEM_QTKEYCHAIN="ON" \
         -DCMAKE_BUILD_TYPE="release" \
         -DCMAKE_EXPORT_COMPILE_COMMANDS="ON" \
-        -DBUILD_WITH_QT6="ON" \
         ..
     cat compile_commands.json
     make -j $(getconf _NPROCESSORS_ONLN)

.docker/Dockerfile-ubuntu-22.04-qt6-build

@@ -3,7 +3,6 @@ ARG UBUNTU_VERSION=22.04
 FROM ubuntu:$UBUNTU_VERSION
 
 ARG QT_VERSION=6.2.4
-ARG BUILD_WITH_QT6=ON
 
 ENV TZ=UTC
 RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone
@@ -48,7 +47,6 @@ RUN mkdir /src/build
 # cmake
 RUN cd /src/build && \
         CXXFLAGS=-fno-sized-deallocation cmake \
-        -DBUILD_WITH_QT6=$BUILD_WITH_QT6 \
         -DCMAKE_INSTALL_PREFIX=appdir/usr/ \
         -DCMAKE_PREFIX_PATH=/opt/qt/$QT_VERSION/gcc_64/lib/cmake \
         -DBUILD_WITH_QTKEYCHAIN=OFF \

.github/workflows/build.yml

@@ -62,7 +62,6 @@ jobs:
       C2_ENABLE_LTO: ${{ matrix.force-lto }}
       C2_PLUGINS: ${{ matrix.plugins }}
       C2_ENABLE_CRASHPAD: ${{ matrix.skip-crashpad == false }}
-      C2_BUILD_WITH_QT6: ${{ startsWith(matrix.qt-version, '6.') }}
 
     steps:
       - uses: actions/checkout@v4
@@ -87,7 +86,6 @@ jobs:
             -DCHATTERINO_LTO="$C2_ENABLE_LTO" \
             -DCHATTERINO_PLUGINS="$C2_PLUGINS" \
             -DCMAKE_PREFIX_PATH="$Qt6_DIR/lib/cmake" \
-            -DBUILD_WITH_QT6="$C2_BUILD_WITH_QT6" \
             -DCHATTERINO_STATIC_QT_BUILD=On \
             ..
           make -j"$(nproc)"
@@ -163,7 +161,7 @@ jobs:
 
       - name: Install Qt5
         if: startsWith(matrix.qt-version, '5.')
-        uses: jurplel/install-qt-action@v4.0.0
+        uses: jurplel/install-qt-action@v4.1.1
         with:
           cache: true
           cache-key-prefix: ${{ runner.os }}-QtCache-${{ matrix.qt-version }}-v2
@@ -171,7 +169,7 @@ jobs:
 
       - name: Install Qt6
         if: startsWith(matrix.qt-version, '6.')
-        uses: jurplel/install-qt-action@v4.0.0
+        uses: jurplel/install-qt-action@v4.1.1
         with:
           cache: true
           cache-key-prefix: ${{ runner.os }}-QtCache-${{ matrix.qt-version }}-v2

.github/workflows/clang-tidy.yml

@@ -30,7 +30,7 @@ jobs:
 
       - name: Install Qt6
         if: startsWith(matrix.qt-version, '6.')
-        uses: jurplel/install-qt-action@v4.0.0
+        uses: jurplel/install-qt-action@v4.1.1
         with:
           cache: true
           cache-key-prefix: ${{ runner.os }}-QtCache-${{ matrix.qt-version }}-v2
@@ -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-macos.yml

@@ -48,7 +48,7 @@ jobs:
           fetch-depth: 0 # allows for tags access
 
       - name: Install Qt
-        uses: jurplel/install-qt-action@v4.0.0
+        uses: jurplel/install-qt-action@v4.1.1
         with:
           cache: true
           cache-key-prefix: ${{ runner.os }}-QtCache-${{ matrix.qt-version }}-v2

.github/workflows/test-windows.yml

@@ -58,7 +58,7 @@ jobs:
           fetch-depth: 0 # allows for tags access
 
       - name: Install Qt
-        uses: jurplel/install-qt-action@v4.0.0
+        uses: jurplel/install-qt-action@v4.1.1
         with:
           cache: true
           cache-key-prefix: ${{ runner.os }}-QtCache-${{ matrix.qt-version }}-v2

.github/workflows/test.yml

@@ -33,7 +33,6 @@ jobs:
       fail-fast: false
     env:
       C2_PLUGINS: ${{ matrix.plugins }}
-      C2_BUILD_WITH_QT6: ${{ startsWith(matrix.qt-version, '6.') }}
 
     steps:
       - uses: actions/checkout@v4
@@ -57,7 +56,6 @@ jobs:
             -DBUILD_APP=OFF \
             -DCHATTERINO_PLUGINS="$C2_PLUGINS" \
             -DCMAKE_PREFIX_PATH="$Qt6_DIR/lib/cmake" \
-            -DBUILD_WITH_QT6="$C2_BUILD_WITH_QT6" \
             -DCHATTERINO_STATIC_QT_BUILD=On \
             -DCHATTERINO_GENERATE_COVERAGE=On \
             -DCMAKE_BUILD_TYPE=Debug \
@@ -98,9 +96,9 @@ jobs:
         working-directory: build-test
 
       - name: Upload coverage reports to Codecov
-        uses: codecov/codecov-action@v4.6.0
+        uses: codecov/codecov-action@v5.0.7
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
-          plugin: gcov
+          plugins: gcov
           fail_ci_if_error: true
           verbose: true

BUILDING_ON_FREEBSD.md

@@ -17,7 +17,7 @@ FreeBSD 15.0-SNAP.
    ```
 1. Generate build files. To enable Lua plugins in your build add `-DCHATTERINO_PLUGINS=ON` to this command.
    ```sh
-   cmake -DBUILD_WITH_QT6=ON ..
+   cmake ..
    ```
 1. Build the project
    ```sh

BUILDING_ON_LINUX.md

@@ -53,7 +53,7 @@ nix-shell -p openssl boost qt6.full pkg-config cmake
    ```
 1. Generate build files. To enable Lua plugins in your build add `-DCHATTERINO_PLUGINS=ON` to this command.
    ```sh
-   cmake -DBUILD_WITH_QT6=ON -DBUILD_WITH_QTKEYCHAIN=OFF ..
+   cmake -DBUILD_WITH_QTKEYCHAIN=OFF ..
    ```
 1. Build the project
    ```sh

BUILDING_ON_MAC.md

@@ -10,7 +10,7 @@ Local dev machines for testing are available on Apple Silicon on macOS 13.
 1. Install [Homebrew](https://brew.sh/#install)  
    We use this for dependency management on macOS
 1. Install all dependencies:  
-   `brew install boost openssl@1.1 rapidjson cmake qt@5`
+   `brew install boost openssl@3 rapidjson cmake qt@6`
 
 ## Building
 
@@ -21,7 +21,7 @@ Local dev machines for testing are available on Apple Silicon on macOS 13.
 1. Create a build directory and go into it:  
    `mkdir build && cd build`
 1. Run CMake. To enable Lua plugins in your build add `-DCHATTERINO_PLUGINS=ON` to this command.  
-   `cmake -DCMAKE_PREFIX_PATH=/opt/homebrew/opt/qt@5 -DOPENSSL_ROOT_DIR=/opt/homebrew/opt/openssl@1.1 ..`
+   `cmake -DCMAKE_PREFIX_PATH=/opt/homebrew/opt/qt@6 ..`
 1. Build:  
    `make`
 

CHANGELOG.md

@@ -73,6 +73,7 @@
 - Bugfix: Fixed incorrect messages getting replaced visually. (#5683)
 - Bugfix: Fixed rendering of multi-line selection that starts at a trailing space. (#5691)
 - Bugfix: Fixed pause indicator not appearing in certain cases. (#5707)
+- Dev: Default build with Qt6 on all platforms. (#5716)
 - Dev: Update Windows build from Qt 6.5.0 to Qt 6.7.1. (#5420)
 - Dev: Update vcpkg build Qt from 6.5.0 to 6.7.0, boost from 1.83.0 to 1.85.0, openssl from 3.1.3 to 3.3.0. (#5422)
 - Dev: Unsingletonize `ISoundController`. (#5462)
@@ -130,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
 

CMakeLists.txt

@@ -21,11 +21,7 @@ option(BUILD_WITH_QTKEYCHAIN "Build Chatterino with support for your system key
 option(USE_SYSTEM_MINIAUDIO "Build Chatterino with your system miniaudio" OFF)
 option(BUILD_WITH_CRASHPAD "Build chatterino with crashpad" OFF)
 option(USE_PRECOMPILED_HEADERS "Use precompiled headers" ON)
-if(WIN32)
-    option(BUILD_WITH_QT6 "Build with Qt6, default on for Windows" On)
-else()
-    option(BUILD_WITH_QT6 "Use Qt6 instead of default Qt5" OFF)
-endif()
+option(BUILD_WITH_QT6 "Build with Qt6" On)
 option(CHATTERINO_GENERATE_COVERAGE "Generate coverage files" OFF)
 # We don't use translations, and we don't want qtkeychain to build translations
 option(BUILD_TRANSLATIONS "" OFF)

lib/settings

@@ -1 +1 @@
-Subproject commit 4a0a1e599377cdcdc91b0fbbefc312936b48730c
+Subproject commit 46eb2509eb37a1c8b593d37bc51359ec2f31a541

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

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 @@ public:
         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 @@ public:
 
         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 @@ private:
 
     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 @@ private:
     const QString host_;
     const PubSubClientOptions clientOptions_;
 
+    OnceFlag stoppedFlag_;
+
     bool stopping_{false};
 
 #ifdef FRIEND_TEST

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

src/util/OnceFlag.hpp

@@ -0,0 +1,41 @@
+#pragma once
+
+#include <chrono>
+#include <condition_variable>
+#include <mutex>
+
+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

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!

tests/src/OnceFlag.cpp

@@ -0,0 +1,88 @@
+#include "util/OnceFlag.hpp"
+
+#include "Test.hpp"
+
+#include <thread>
+
+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();
+}