From 74d65a345d13a22b1fe77f86522f7fea251286d2 Mon Sep 17 00:00:00 2001
From: pajlada <rasmus.karlsson@pajlada.com>
Date: Sat, 10 Aug 2024 14:24:25 +0200
Subject: [PATCH] fix: cleanly exit on shutdown (#5537)

Co-authored-by: Mm2PL <mm2pl+gh@kotmisia.pl>
Co-authored-by: Nerixyz <nerixdev@outlook.de>
---
 CHANGELOG.md                                  |  1 +
 mocks/include/mocks/DisabledStreamerMode.hpp  |  4 ++
 src/Application.cpp                           | 38 +------------------
 src/Application.hpp                           |  6 ---
 src/CMakeLists.txt                            |  1 +
 src/RunGui.cpp                                |  4 --
 src/common/Channel.cpp                        |  2 +-
 src/common/Channel.hpp                        |  2 +-
 src/providers/bttv/BttvLiveUpdates.cpp        |  6 ++-
 .../liveupdates/BasicPubSubClient.hpp         | 11 ++++++
 .../liveupdates/BasicPubSubManager.hpp        | 15 +++++++-
 src/providers/seventv/SeventvEventAPI.cpp     |  4 +-
 src/providers/seventv/eventapi/Client.cpp     | 22 +++++++----
 src/providers/seventv/eventapi/Client.hpp     |  3 ++
 src/singletons/NativeMessaging.cpp            | 17 ++++++++-
 src/singletons/NativeMessaging.hpp            |  1 +
 src/singletons/StreamerMode.cpp               | 25 +++++++++++-
 src/singletons/StreamerMode.hpp               |  4 ++
 src/util/IpcQueue.cpp                         |  5 +++
 src/util/IpcQueue.hpp                         |  2 +
 src/util/RenameThread.hpp                     | 20 ++++++++++
 src/widgets/splits/SplitInput.cpp             |  4 +-
 tests/src/BasicPubSub.cpp                     |  2 +-
 23 files changed, 134 insertions(+), 65 deletions(-)
 create mode 100644 src/util/RenameThread.hpp

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a4001d843..0be2e7c2e 100644
--- a/CHANGELOG.md
+++ b/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)
 
diff --git a/mocks/include/mocks/DisabledStreamerMode.hpp b/mocks/include/mocks/DisabledStreamerMode.hpp
index 96c03b2b2..747fdd0ad 100644
--- a/mocks/include/mocks/DisabledStreamerMode.hpp
+++ b/mocks/include/mocks/DisabledStreamerMode.hpp
@@ -9,4 +9,8 @@ public:
     {
         return false;
     }
+
+    void start() override
+    {
+    }
 };
diff --git a/src/Application.cpp b/src/Application.cpp
index 5495d3af0..94f38dc54 100644
--- a/src/Application.cpp
+++ b/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)
diff --git a/src/Application.hpp b/src/Application.hpp
index ff9863851..7afaf27f6 100644
--- a/src/Application.hpp
+++ b/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();
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 42823b259..7c68aef35 100644
--- a/src/CMakeLists.txt
+++ b/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
diff --git a/src/RunGui.cpp b/src/RunGui.cpp
index 086077454..4863f4da7 100644
--- a/src/RunGui.cpp
+++ b/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
diff --git a/src/common/Channel.cpp b/src/common/Channel.cpp
index bbf58f0a7..4e5d19e7d 100644
--- a/src/common/Channel.cpp
+++ b/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)
diff --git a/src/common/Channel.hpp b/src/common/Channel.hpp
index b85fb83cb..c2da9e67b 100644
--- a/src/common/Channel.hpp
+++ b/src/common/Channel.hpp
@@ -123,7 +123,7 @@ public:
 
     static std::shared_ptr<Channel> getEmpty();
 
-    TabCompletionModel completionModel;
+    TabCompletionModel *completionModel;
     QDate lastDate_;
 
 protected:
diff --git a/src/providers/bttv/BttvLiveUpdates.cpp b/src/providers/bttv/BttvLiveUpdates.cpp
index f9128b5c7..b2658937c 100644
--- a/src/providers/bttv/BttvLiveUpdates.cpp
+++ b/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)
 {
 }
 
diff --git a/src/providers/liveupdates/BasicPubSubClient.hpp b/src/providers/liveupdates/BasicPubSubClient.hpp
index 31532c1dd..aa65e619a 100644
--- a/src/providers/liveupdates/BasicPubSubClient.hpp
+++ b/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_;
diff --git a/src/providers/liveupdates/BasicPubSubManager.hpp b/src/providers/liveupdates/BasicPubSubManager.hpp
index f82f70363..d7cb0253b 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/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};
 };
 
diff --git a/src/providers/seventv/SeventvEventAPI.cpp b/src/providers/seventv/SeventvEventAPI.cpp
index 294f95ee2..c38d2a223 100644
--- a/src/providers/seventv/SeventvEventAPI.cpp
+++ b/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)
 {
 }
diff --git a/src/providers/seventv/eventapi/Client.cpp b/src/providers/seventv/eventapi/Client.cpp
index f266478ce..84533a066 100644
--- a/src/providers/seventv/eventapi/Client.cpp
+++ b/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,14 +61,13 @@ void Client::checkHeartbeat()
 
     auto self = std::dynamic_pointer_cast<Client>(this->shared_from_this());
 
-    runAfter(this->websocketClient_.get_io_service(), this->heartbeatInterval_,
-             [self](auto) {
-                 if (!self->isStarted())
-                 {
-                     return;
-                 }
-                 self->checkHeartbeat();
-             });
+    runAfter(this->heartbeatTimer_, this->heartbeatInterval_, [self](auto) {
+        if (!self->isStarted())
+        {
+            return;
+        }
+        self->checkHeartbeat();
+    });
 }
 
 }  // namespace chatterino::seventv::eventapi
diff --git a/src/providers/seventv/eventapi/Client.hpp b/src/providers/seventv/eventapi/Client.hpp
index 11683edcf..ecbc7cb73 100644
--- a/src/providers/seventv/eventapi/Client.hpp
+++ b/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;
 };
diff --git a/src/singletons/NativeMessaging.cpp b/src/singletons/NativeMessaging.cpp
index 58dfd1b2b..1dfc93a5f 100644
--- a/src/singletons/NativeMessaging.cpp
+++ b/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())
diff --git a/src/singletons/NativeMessaging.hpp b/src/singletons/NativeMessaging.hpp
index 5d0633358..0d3ba454f 100644
--- a/src/singletons/NativeMessaging.hpp
+++ b/src/singletons/NativeMessaging.hpp
@@ -36,6 +36,7 @@ public:
     NativeMessagingServer(NativeMessagingServer &&) = delete;
     NativeMessagingServer &operator=(const NativeMessagingServer &) = delete;
     NativeMessagingServer &operator=(NativeMessagingServer &&) = delete;
+    ~NativeMessagingServer();
 
     void start();
 
diff --git a/src/singletons/StreamerMode.cpp b/src/singletons/StreamerMode.cpp
index eb79be9a1..fac58aaad 100644
--- a/src/singletons/StreamerMode.cpp
+++ b/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
diff --git a/src/singletons/StreamerMode.hpp b/src/singletons/StreamerMode.hpp
index 5b7b6ef80..58ce43be6 100644
--- a/src/singletons/StreamerMode.hpp
+++ b/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);
 
diff --git a/src/util/IpcQueue.cpp b/src/util/IpcQueue.cpp
index 2e2e3df54..397386974 100644
--- a/src/util/IpcQueue.cpp
+++ b/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
diff --git a/src/util/IpcQueue.hpp b/src/util/IpcQueue.hpp
index e56cf2ba3..059d15c6e 100644
--- a/src/util/IpcQueue.hpp
+++ b/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.
diff --git a/src/util/RenameThread.hpp b/src/util/RenameThread.hpp
new file mode 100644
index 000000000..6f57bbb46
--- /dev/null
+++ b/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
diff --git a/src/widgets/splits/SplitInput.cpp b/src/widgets/splits/SplitInput.cpp
index 85c421be9..b45928a82 100644
--- a/src/widgets/splits/SplitInput.cpp
+++ b/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);
     });
 
diff --git a/tests/src/BasicPubSub.cpp b/tests/src/BasicPubSub.cpp
index 6315970ff..3b5c6b8f8 100644
--- a/tests/src/BasicPubSub.cpp
+++ b/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")
     {
     }