Merge b62d2577cb into 292f9b9734

CHANGELOG.md

@@ -106,6 +106,7 @@
 - Dev: Refactored the Image Uploader feature. (#4971)
 - Dev: Refactored the SplitOverlay code. (#5082)
 - Dev: Fixed deadlock and use-after-free in tests. (#4981)
+- Dev: Cleanly exit Chatterino instead of force exiting. (#4993)
 - Dev: Moved all `.clang-format` files to the root directory. (#5037)
 - Dev: Load less message history upon reconnects. (#5001, #5018)
 - Dev: Load less message history upon reconnects. (#5001)

src/Application.cpp

@@ -114,6 +114,7 @@ Application::Application(Settings &_settings, Paths &_paths, const Args &_args)
     , emotes(&this->emplace<Emotes>())
     , accounts(&this->emplace<AccountController>())
     , hotkeys(&this->emplace<HotkeyController>())
+    , twitch(&this->emplace<TwitchIrcServer>())
     , windows(&this->emplace<WindowManager>())
     , toasts(&this->emplace<Toasts>())
     , imageUploader(&this->emplace<ImageUploader>())
@@ -123,7 +124,6 @@ Application::Application(Settings &_settings, Paths &_paths, const Args &_args)
     , commands(&this->emplace<CommandController>())
     , notifications(&this->emplace<NotificationController>())
     , highlights(&this->emplace<HighlightController>())
-    , twitch(&this->emplace<TwitchIrcServer>())
     , chatterinoBadges(&this->emplace<ChatterinoBadges>())
     , ffzBadges(&this->emplace<FfzBadges>())
     , seventvBadges(&this->emplace<SeventvBadges>())
@@ -147,11 +147,6 @@ Application::Application(Settings &_settings, Paths &_paths, const Args &_args)
 
 Application::~Application() = default;
 
-void Application::fakeDtor()
-{
-    this->twitchPubSub.reset();
-}
-
 void Application::initialize(Settings &settings, Paths &paths)
 {
     assert(isAppInitialized == false);

src/Application.hpp

@@ -98,12 +98,6 @@ public:
     Application &operator=(const Application &) = delete;
     Application &operator=(Application &&) = delete;
 
-    /**
-     * 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, Paths &paths);
     void load();
     void save();
@@ -117,6 +111,7 @@ public:
     Emotes *const emotes{};
     AccountController *const accounts{};
     HotkeyController *const hotkeys{};
+    TwitchIrcServer *const twitch{};
     WindowManager *const windows{};
     Toasts *const toasts{};
     ImageUploader *const imageUploader{};
@@ -126,7 +121,6 @@ public:
     CommandController *const commands{};
     NotificationController *const notifications{};
     HighlightController *const highlights{};
-    TwitchIrcServer *const twitch{};
     ChatterinoBadges *const chatterinoBadges{};
     FfzBadges *const ffzBadges{};
     SeventvBadges *const seventvBadges{};

src/CMakeLists.txt

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

@@ -21,6 +21,7 @@
 #include <QtConcurrent>
 
 #include <csignal>
+#include <thread>
 #include <tuple>
 
 #ifdef USEWINSDK
@@ -237,6 +238,7 @@ void runGui(QApplication &a, Paths &paths, Settings &settings, const Args &args)
 #endif
 
     auto thread = std::thread([dir = paths.miscDirectory] {
+#ifdef Q_OS_WIN32
         {
             auto path = combinePath(dir, "Update.exe");
             if (QFile::exists(path))
@@ -251,6 +253,7 @@ void runGui(QApplication &a, Paths &paths, Settings &settings, const Args &args)
                 QFile::remove(path);
             }
         }
+#endif
     });
 
     // Clear the cache 1 minute after start.
@@ -281,15 +284,16 @@ void runGui(QApplication &a, Paths &paths, Settings &settings, const Args &args)
         pajlada::Settings::SettingManager::gSave();
     }
 
+    if (thread.joinable())
+    {
+        thread.join();
+    }
     chatterino::NetworkManager::deinit();
 
 #ifdef USEWINSDK
     // flushing windows clipboard to keep copied messages
     flushClipboard();
 #endif
-
-    app.fakeDtor();
-
-    _exit(0);
 }
+
 }  // namespace chatterino

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 do 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,6 +8,7 @@
 #include "providers/twitch/PubSubHelpers.hpp"
 #include "util/DebugCount.hpp"
 #include "util/ExponentialBackoff.hpp"
+#include "util/RenameThread.hpp"
 
 #include <pajlada/signals/signal.hpp>
 #include <QJsonObject>
@@ -94,7 +95,10 @@ public:
                 .toStdString());
     }
 
-    virtual ~BasicPubSubManager() = default;
+    virtual ~BasicPubSubManager()
+    {
+        this->stop();
+    };
 
     BasicPubSubManager(const BasicPubSubManager &) = delete;
     BasicPubSubManager(const BasicPubSubManager &&) = delete;
@@ -115,6 +119,8 @@ public:
         this->mainThread_.reset(new std::thread([this] {
             runThread();
         }));
+
+        renameThread(*this->mainThread_.get(), "BPSM");
     }
 
     void stop()

src/providers/seventv/eventapi/Client.cpp

@@ -13,6 +13,8 @@ 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()))
 {
 }
 
@@ -23,6 +25,11 @@ void Client::onConnectionEstablished()
     this->checkHeartbeat();
 }
 
+void Client::stopImpl()
+{
+    this->heartbeatTimer_->cancel();
+}
+
 void Client::setHeartbeatInterval(int intervalMs)
 {
     qCDebug(chatterinoSeventvEventAPI)
@@ -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

src/providers/seventv/eventapi/Client.hpp

@@ -24,6 +24,7 @@ public:
 
 protected:
     void onConnectionEstablished() override;
+    void stopImpl() override;
 
 private:
     void checkHeartbeat();
@@ -32,6 +33,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/WindowManager.cpp

@@ -125,7 +125,16 @@ WindowManager::WindowManager()
     });
 }
 
-WindowManager::~WindowManager() = default;
+WindowManager::~WindowManager()
+{
+    for (const auto &window : this->windows_)
+    {
+        // We would prefer to use window->deleteLater() here, but the timings are too tight
+        // Channel's completion model gets destroyed before the deleteLater call actually deletes the
+        // UI objects that rely on the completion model
+        delete window;
+    }
+}
 
 MessageElementFlags WindowManager::getWordFlags()
 {

src/util/RenameThread.hpp

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

src/widgets/splits/SplitInput.cpp

@@ -50,13 +50,13 @@ SplitInput::SplitInput(QWidget *parent, Split *_chatWidget,
     this->installEventFilter(this);
     this->initLayout();
 
-    auto completer =
-        new QCompleter(&this->split_->getChannel()->completionModel);
+    auto *completer =
+        new QCompleter(&this->split_->getChannel()->completionModel, this);
     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);
         this->ui_.textEdit->setCompleter(completer);
     });
 
@@ -76,6 +76,8 @@ SplitInput::SplitInput(QWidget *parent, Split *_chatWidget,
                                        });
 }
 
+SplitInput::~SplitInput() = default;
+
 void SplitInput::initLayout()
 {
     auto app = getApp();

src/widgets/splits/SplitInput.hpp

@@ -24,7 +24,7 @@ class ResizingTextEdit;
 class ChannelView;
 enum class CompletionKind;
 
-class SplitInput : public BaseWidget
+class SplitInput final : public BaseWidget
 {
     Q_OBJECT
 
@@ -32,6 +32,7 @@ public:
     SplitInput(Split *_chatWidget, bool enableInlineReplying = true);
     SplitInput(QWidget *parent, Split *_chatWidget, ChannelView *_channelView,
                bool enableInlineReplying = true);
+    ~SplitInput() override;
 
     bool hasSelection() const;
     void clearSelection() const;