Merge 56ba1ca2ee into eeb797c03f

.CI/full-ubuntu-build.sh

@@ -29,6 +29,7 @@ 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,6 +19,7 @@ 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,6 +20,7 @@ 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,6 +3,7 @@ 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
@@ -47,6 +48,7 @@ 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,6 +62,7 @@ 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
@@ -86,6 +87,7 @@ 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)"
@@ -161,7 +163,7 @@ jobs:
 
       - name: Install Qt5
         if: startsWith(matrix.qt-version, '5.')
-        uses: jurplel/install-qt-action@v4.1.1
+        uses: jurplel/install-qt-action@v4.0.0
         with:
           cache: true
           cache-key-prefix: ${{ runner.os }}-QtCache-${{ matrix.qt-version }}-v2
@@ -169,7 +171,7 @@ jobs:
 
       - name: Install Qt6
         if: startsWith(matrix.qt-version, '6.')
-        uses: jurplel/install-qt-action@v4.1.1
+        uses: jurplel/install-qt-action@v4.0.0
         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.1.1
+        uses: jurplel/install-qt-action@v4.0.0
         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.1
+        uses: ZedThree/clang-tidy-review@v0.20.0
         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.1
+        uses: ZedThree/clang-tidy-review/upload@v0.20.0

.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.1
+      - uses: ZedThree/clang-tidy-review/post@v0.20.0
         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.1.1
+        uses: jurplel/install-qt-action@v4.0.0
         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.1.1
+        uses: jurplel/install-qt-action@v4.0.0
         with:
           cache: true
           cache-key-prefix: ${{ runner.os }}-QtCache-${{ matrix.qt-version }}-v2

.github/workflows/test.yml

@@ -33,6 +33,7 @@ jobs:
       fail-fast: false
     env:
       C2_PLUGINS: ${{ matrix.plugins }}
+      C2_BUILD_WITH_QT6: ${{ startsWith(matrix.qt-version, '6.') }}
 
     steps:
       - uses: actions/checkout@v4
@@ -56,6 +57,7 @@ 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 \
@@ -96,9 +98,9 @@ jobs:
         working-directory: build-test
 
       - name: Upload coverage reports to Codecov
-        uses: codecov/codecov-action@v5.0.7
+        uses: codecov/codecov-action@v4.6.0
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
-          plugins: gcov
+          plugin: 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 ..
+   cmake -DBUILD_WITH_QT6=ON ..
    ```
 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_QTKEYCHAIN=OFF ..
+   cmake -DBUILD_WITH_QT6=ON -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@3 rapidjson cmake qt@6`
+   `brew install boost openssl@1.1 rapidjson cmake qt@5`
 
 ## 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@6 ..`
+   `cmake -DCMAKE_PREFIX_PATH=/opt/homebrew/opt/qt@5 -DOPENSSL_ROOT_DIR=/opt/homebrew/opt/openssl@1.1 ..`
 1. Build:  
    `make`
 

CHANGELOG.md

@@ -73,7 +73,6 @@
 - 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)
@@ -131,7 +130,6 @@
 - 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,7 +21,11 @@ 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)
-option(BUILD_WITH_QT6 "Build with Qt6" 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(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 46eb2509eb37a1c8b593d37bc51359ec2f31a541
+Subproject commit 4a0a1e599377cdcdc91b0fbbefc312936b48730c

src/CMakeLists.txt

@@ -514,8 +514,6 @@ 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,12 +8,10 @@
 #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>
@@ -122,11 +120,6 @@ 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();
         }));
 
@@ -149,34 +142,22 @@ public:
 
         this->work_.reset();
 
-        if (!this->mainThread_->joinable())
+        if (this->mainThread_->joinable())
         {
-            return;
+            // 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();
+            }
         }
-
-        // 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:
@@ -413,7 +394,6 @@ 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,11 +560,6 @@ 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");
@@ -583,36 +578,23 @@ void PubSub::stop()
 
     this->work.reset();
 
-    if (!this->thread->joinable())
+    if (this->thread->joinable())
     {
-        return;
+        // 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();
+        }
     }
-
-    // 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,7 +3,6 @@
 #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>
@@ -268,8 +267,6 @@ private:
     const QString host_;
     const PubSubClientOptions clientOptions_;
 
-    OnceFlag stoppedFlag_;
-
     bool stopping_{false};
 
 #ifdef FRIEND_TEST

src/util/OnceFlag.cpp

@@ -1,33 +0,0 @@
-#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

@@ -1,41 +0,0 @@
-#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,7 +51,6 @@ 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

@@ -1,88 +0,0 @@
-#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();
-}