fix: cleanly exit on shutdown (#5537)

Co-authored-by: Mm2PL <mm2pl+gh@kotmisia.pl>
Co-authored-by: Nerixyz <nerixdev@outlook.de>

CHANGELOG.md

@@ -60,6 +60,7 @@
 - Dev: Documented and added tests to RTL handling. (#5473)
 - Dev: Refactored 7TV/BTTV definitions out of `TwitchIrcServer` into `Application`. (#5532)
 - Dev: Refactored code that's responsible for deleting old update files. (#5535)
+- Dev: Cleanly exit on shutdown. (#5537)
 - Dev: Refactored a few `#define`s into `const(expr)` and cleaned includes. (#5527)
 - Dev: Prepared for Qt 6.8 by addressing some deprecations. (#5529)
 

mocks/include/mocks/DisabledStreamerMode.hpp

@@ -9,4 +9,8 @@ public:
     {
         return false;
     }
+
+    void start() override
+    {
+    }
 };

src/Application.cpp

@@ -196,42 +196,6 @@ Application::~Application()
     Application::instance = nullptr;
 }
 
-void Application::fakeDtor()
-{
-#ifdef CHATTERINO_HAVE_PLUGINS
-    this->plugins.reset();
-#endif
-    this->twitchPubSub.reset();
-    this->twitchBadges.reset();
-    this->twitchLiveController.reset();
-    this->chatterinoBadges.reset();
-    // this->bttvLiveUpdates.reset();
-    this->bttvEmotes.reset();
-    this->ffzEmotes.reset();
-    // this->seventvEventAPI.reset();
-    this->seventvEmotes.reset();
-    this->notifications.reset();
-    this->commands.reset();
-    // If a crash happens after crashHandler has been reset, we'll assert
-    // This isn't super different from before, where if the app is already killed, the getApp() portion of it is already dead
-    this->crashHandler.reset();
-    this->seventvAPI.reset();
-    this->highlights.reset();
-    this->seventvBadges.reset();
-    this->ffzBadges.reset();
-    // this->twitch.reset();
-    this->imageUploader.reset();
-    this->hotkeys.reset();
-    this->fonts.reset();
-    this->sound.reset();
-    this->userData.reset();
-    this->toasts.reset();
-    this->accounts.reset();
-    this->emotes.reset();
-    this->themes.reset();
-    this->streamerMode.reset();
-}
-
 void Application::initialize(Settings &settings, const Paths &paths)
 {
     assert(isAppInitialized == false);
@@ -316,6 +280,8 @@ void Application::initialize(Settings &settings, const Paths &paths)
 
     this->initBttvLiveUpdates();
     this->initSeventvEventAPI();
+
+    this->streamerMode->start();
 }
 
 int Application::run(QApplication &qtApp)

src/Application.hpp

@@ -131,12 +131,6 @@ public:
         return false;
     }
 
-    /**
-     * In the interim, before we remove _exit(0); from RunGui.cpp,
-     * this will destroy things we know can be destroyed
-     */
-    void fakeDtor();
-
     void initialize(Settings &settings, const Paths &paths);
     void load();
     void save();

src/CMakeLists.txt

@@ -511,6 +511,7 @@ set(SOURCE_FILES
         util/RapidjsonHelpers.hpp
         util/RatelimitBucket.cpp
         util/RatelimitBucket.hpp
+        util/RenameThread.hpp
         util/SampleData.cpp
         util/SampleData.hpp
         util/SharedPtrElementLess.hpp

src/RunGui.cpp

@@ -277,10 +277,6 @@ void runGui(QApplication &a, const Paths &paths, Settings &settings,
     // flushing windows clipboard to keep copied messages
     flushClipboard();
 #endif
-
-    app.fakeDtor();
-
-    _exit(0);
 }
 
 }  // namespace chatterino

src/common/Channel.cpp

@@ -26,7 +26,7 @@ namespace chatterino {
 // Channel
 //
 Channel::Channel(const QString &name, Type type)
-    : completionModel(*this, nullptr)
+    : completionModel(new TabCompletionModel(*this, nullptr))
     , lastDate_(QDate::currentDate())
     , name_(name)
     , messages_(getSettings()->scrollbackSplitLimit)

src/common/Channel.hpp

@@ -123,7 +123,7 @@ public:
 
     static std::shared_ptr<Channel> getEmpty();
 
-    TabCompletionModel completionModel;
+    TabCompletionModel *completionModel;
     QDate lastDate_;
 
 protected:

src/providers/bttv/BttvLiveUpdates.cpp

@@ -1,13 +1,17 @@
 #include "providers/bttv/BttvLiveUpdates.hpp"
 
+#include "common/Literals.hpp"
+
 #include <QJsonDocument>
 
 #include <utility>
 
 namespace chatterino {
 
+using namespace chatterino::literals;
+
 BttvLiveUpdates::BttvLiveUpdates(QString host)
-    : BasicPubSubManager(std::move(host))
+    : BasicPubSubManager(std::move(host), u"BTTV"_s)
 {
 }
 

src/providers/liveupdates/BasicPubSubClient.hpp

@@ -150,6 +150,15 @@ protected:
         return this->started_.load(std::memory_order_acquire);
     }
 
+    /**
+     * @brief Will be called when the clients has been requested to stop
+     *
+     * Derived classes can override this to implement their own shutdown behaviour
+     */
+    virtual void stopImpl()
+    {
+    }
+
     liveupdates::WebsocketClient &websocketClient_;
 
 private:
@@ -164,6 +173,8 @@ private:
     {
         assert(this->isStarted());
         this->started_.store(false, std::memory_order_release);
+
+        this->stopImpl();
     }
 
     liveupdates::WebsocketHandle handle_;

src/providers/liveupdates/BasicPubSubManager.hpp

@@ -8,10 +8,12 @@
 #include "providers/twitch/PubSubHelpers.hpp"
 #include "util/DebugCount.hpp"
 #include "util/ExponentialBackoff.hpp"
+#include "util/RenameThread.hpp"
 
 #include <pajlada/signals/signal.hpp>
 #include <QJsonObject>
 #include <QString>
+#include <QStringBuilder>
 #include <websocketpp/client.hpp>
 
 #include <algorithm>
@@ -59,8 +61,9 @@ template <typename Subscription>
 class BasicPubSubManager
 {
 public:
-    BasicPubSubManager(QString host)
+    BasicPubSubManager(QString host, QString shortName)
         : host_(std::move(host))
+        , shortName_(std::move(shortName))
     {
         this->websocketClient_.set_access_channels(
             websocketpp::log::alevel::all);
@@ -94,7 +97,10 @@ public:
                 .toStdString());
     }
 
-    virtual ~BasicPubSubManager() = default;
+    virtual ~BasicPubSubManager()
+    {
+        this->stop();
+    }
 
     BasicPubSubManager(const BasicPubSubManager &) = delete;
     BasicPubSubManager(const BasicPubSubManager &&) = delete;
@@ -115,6 +121,8 @@ public:
         this->mainThread_.reset(new std::thread([this] {
             runThread();
         }));
+
+        renameThread(*this->mainThread_.get(), "BPSM-" % this->shortName_);
     }
 
     void stop()
@@ -373,6 +381,9 @@ private:
 
     const QString host_;
 
+    /// Short name of the service (e.g. "7TV" or "BTTV")
+    const QString shortName_;
+
     bool stopping_{false};
 };
 

src/providers/seventv/SeventvEventAPI.cpp

@@ -1,6 +1,7 @@
 #include "providers/seventv/SeventvEventAPI.hpp"
 
 #include "Application.hpp"
+#include "common/Literals.hpp"
 #include "providers/seventv/eventapi/Client.hpp"
 #include "providers/seventv/eventapi/Dispatch.hpp"
 #include "providers/seventv/eventapi/Message.hpp"
@@ -16,10 +17,11 @@ namespace chatterino {
 
 using namespace seventv;
 using namespace seventv::eventapi;
+using namespace chatterino::literals;
 
 SeventvEventAPI::SeventvEventAPI(
     QString host, std::chrono::milliseconds defaultHeartbeatInterval)
-    : BasicPubSubManager(std::move(host))
+    : BasicPubSubManager(std::move(host), u"7TV"_s)
     , heartbeatInterval_(defaultHeartbeatInterval)
 {
 }

src/providers/seventv/eventapi/Client.cpp

@@ -13,9 +13,16 @@ Client::Client(liveupdates::WebsocketClient &websocketClient,
     : BasicPubSubClient<Subscription>(websocketClient, std::move(handle))
     , lastHeartbeat_(std::chrono::steady_clock::now())
     , heartbeatInterval_(heartbeatInterval)
+    , heartbeatTimer_(std::make_shared<boost::asio::steady_timer>(
+          this->websocketClient_.get_io_service()))
 {
 }
 
+void Client::stopImpl()
+{
+    this->heartbeatTimer_->cancel();
+}
+
 void Client::onConnectionEstablished()
 {
     this->lastHeartbeat_.store(std::chrono::steady_clock::now(),
@@ -54,8 +61,7 @@ void Client::checkHeartbeat()
 
     auto self = std::dynamic_pointer_cast<Client>(this->shared_from_this());
 
-    runAfter(this->websocketClient_.get_io_service(), this->heartbeatInterval_,
-             [self](auto) {
+    runAfter(this->heartbeatTimer_, this->heartbeatInterval_, [self](auto) {
         if (!self->isStarted())
         {
             return;

src/providers/seventv/eventapi/Client.hpp

@@ -19,6 +19,8 @@ public:
            liveupdates::WebsocketHandle handle,
            std::chrono::milliseconds heartbeatInterval);
 
+    void stopImpl() override;
+
     void setHeartbeatInterval(int intervalMs);
     void handleHeartbeat();
 
@@ -32,6 +34,7 @@ private:
         lastHeartbeat_;
     // This will be set once on the welcome message.
     std::chrono::milliseconds heartbeatInterval_;
+    std::shared_ptr<boost::asio::steady_timer> heartbeatTimer_;
 
     friend SeventvEventAPI;
 };

src/singletons/NativeMessaging.cpp

@@ -136,6 +136,21 @@ NativeMessagingServer::NativeMessagingServer()
 {
 }
 
+NativeMessagingServer::~NativeMessagingServer()
+{
+    if (!ipc::IpcQueue::remove("chatterino_gui"))
+    {
+        qCWarning(chatterinoNativeMessage) << "Failed to remove message queue";
+    }
+    this->thread.requestInterruption();
+    this->thread.quit();
+    // Most likely, the receiver thread will still wait for a message
+    if (!this->thread.wait(250))
+    {
+        this->thread.terminate();
+    }
+}
+
 void NativeMessagingServer::start()
 {
     this->thread.start();
@@ -161,7 +176,7 @@ void NativeMessagingServer::ReceiverThread::run()
         return;
     }
 
-    while (true)
+    while (!this->isInterruptionRequested())
     {
         auto buf = messageQueue->receive();
         if (buf.isEmpty())

src/singletons/NativeMessaging.hpp

@@ -36,6 +36,7 @@ public:
     NativeMessagingServer(NativeMessagingServer &&) = delete;
     NativeMessagingServer &operator=(const NativeMessagingServer &) = delete;
     NativeMessagingServer &operator=(NativeMessagingServer &&) = delete;
+    ~NativeMessagingServer();
 
     void start();
 

src/singletons/StreamerMode.cpp

@@ -105,6 +105,12 @@ bool isBroadcasterSoftwareActive()
         break;
     }
 
+    if (!p.waitForFinished(1000))
+    {
+        qCWarning(chatterinoStreamerMode) << "Force-killing pgrep";
+        p.kill();
+    }
+
     return false;
 #elif defined(Q_OS_WIN)
     if (!IsWindowsVistaOrGreater())
@@ -160,6 +166,8 @@ public:
 
     [[nodiscard]] bool isEnabled() const;
 
+    void start();
+
 private:
     void settingChanged(StreamerModeSetting value);
     void setEnabled(bool enabled);
@@ -194,9 +202,15 @@ bool StreamerMode::isEnabled() const
     return this->private_->isEnabled();
 }
 
+void StreamerMode::start()
+{
+    this->private_->start();
+}
+
 StreamerModePrivate::StreamerModePrivate(StreamerMode *parent)
     : parent_(parent)
 {
+    this->thread_.setObjectName("StreamerMode");
     this->timer_.moveToThread(&this->thread_);
     QObject::connect(&this->timer_, &QTimer::timeout, [this] {
         auto timeouts =
@@ -221,13 +235,22 @@ StreamerModePrivate::StreamerModePrivate(StreamerMode *parent)
     QObject::connect(&this->thread_, &QThread::started, [this] {
         this->settingChanged(getSettings()->enableStreamerMode.getEnum());
     });
+}
+
+void StreamerModePrivate::start()
+{
     this->thread_.start();
 }
 
 StreamerModePrivate::~StreamerModePrivate()
 {
     this->thread_.quit();
-    this->thread_.wait(50);
+    if (!this->thread_.wait(500))
+    {
+        qCWarning(chatterinoStreamerMode)
+            << "Failed waiting for thread, terminating it";
+        this->thread_.terminate();
+    }
 }
 
 bool StreamerModePrivate::isEnabled() const

src/singletons/StreamerMode.hpp

@@ -20,6 +20,8 @@ public:
 
     [[nodiscard]] virtual bool isEnabled() const = 0;
 
+    virtual void start() = 0;
+
 signals:
     void changed(bool enabled);
 };
@@ -37,6 +39,8 @@ public:
 
     bool isEnabled() const override;
 
+    void start() override;
+
 private:
     void updated(bool enabled);
 

src/util/IpcQueue.cpp

@@ -101,6 +101,11 @@ std::pair<std::unique_ptr<IpcQueue>, QString> IpcQueue::tryReplaceOrCreate(
     }
 }
 
+bool IpcQueue::remove(const char *name)
+{
+    return boost_ipc::message_queue::remove(name);
+}
+
 QByteArray IpcQueue::receive()
 {
     try

src/util/IpcQueue.hpp

@@ -27,6 +27,8 @@ public:
     static std::pair<std::unique_ptr<IpcQueue>, QString> tryReplaceOrCreate(
         const char *name, size_t maxMessages, size_t maxMessageSize);
 
+    static bool remove(const char *name);
+
     // TODO: use std::expected
     /// Try to receive a message.
     /// In the case of an error, the buffer is empty.

src/util/RenameThread.hpp

@@ -0,0 +1,20 @@
+#pragma once
+
+#include <QString>
+#include <QtGlobal>
+
+#ifdef Q_OS_LINUX
+#    include <pthread.h>
+#endif
+
+namespace chatterino {
+
+template <typename T>
+void renameThread(T &&thread, const QString &threadName)
+{
+#ifdef Q_OS_LINUX
+    pthread_setname_np(thread.native_handle(), threadName.toLocal8Bit());
+#endif
+}
+
+}  // namespace chatterino

src/widgets/splits/SplitInput.cpp

@@ -50,12 +50,12 @@ SplitInput::SplitInput(QWidget *parent, Split *_chatWidget,
     this->initLayout();
 
     auto *completer =
-        new QCompleter(&this->split_->getChannel()->completionModel);
+        new QCompleter(this->split_->getChannel()->completionModel);
     this->ui_.textEdit->setCompleter(completer);
 
     this->signalHolder_.managedConnect(this->split_->channelChanged, [this] {
         auto channel = this->split_->getChannel();
-        auto *completer = new QCompleter(&channel->completionModel);
+        auto *completer = new QCompleter(channel->completionModel);
         this->ui_.textEdit->setCompleter(completer);
     });
 

tests/src/BasicPubSub.cpp

@@ -68,7 +68,7 @@ class MyManager : public BasicPubSubManager<DummySubscription>
 {
 public:
     MyManager(QString host)
-        : BasicPubSubManager(std::move(host))
+        : BasicPubSubManager(std::move(host), "Test")
     {
     }