From 865dd130da250d91c8093b2464374f31b2d5b475 Mon Sep 17 00:00:00 2001 From: Mm2PL Date: Fri, 1 Dec 2023 14:46:37 +0100 Subject: [PATCH 01/11] Get rid of the pesky _exit call --- src/RunGui.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/RunGui.cpp b/src/RunGui.cpp index 62f1d6a43..19ac9413c 100644 --- a/src/RunGui.cpp +++ b/src/RunGui.cpp @@ -20,6 +20,7 @@ #include #include +#include #include #ifdef USEWINSDK @@ -244,7 +245,8 @@ void runGui(QApplication &a, Paths &paths, Settings &settings) restartOnSignal = value; }); - auto thread = std::thread([dir = paths.miscDirectory] { + auto thread = std::jthread([dir = paths.miscDirectory] { +#ifdef Q_OS_WIN32 { auto path = combinePath(dir, "Update.exe"); if (QFile::exists(path)) @@ -259,6 +261,7 @@ void runGui(QApplication &a, Paths &paths, Settings &settings) QFile::remove(path); } } +#endif }); // Clear the cache 1 minute after start. @@ -314,7 +317,5 @@ void runGui(QApplication &a, Paths &paths, Settings &settings) // flushing windows clipboard to keep copied messages flushClipboard(); #endif - - _exit(0); } } // namespace chatterino From 2dcd6be804ba6611d4dd06bd84ff24c28ce31384 Mon Sep 17 00:00:00 2001 From: Mm2PL Date: Fri, 1 Dec 2023 14:53:29 +0100 Subject: [PATCH 02/11] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 845d117b4..2655cbb6d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -75,6 +75,7 @@ - Dev: Added Tests for Windows and MacOS in CI. (#4970) - Dev: Refactored the Image Uploader feature. (#4971) - Dev: Fixed deadlock and use-after-free in tests. (#4981) +- Dev: Cleanly exit Chatterino instead of force exiting. (#4993) ## 2.4.6 From 76acef0f209641fbbcc87c1fe029420da95db363 Mon Sep 17 00:00:00 2001 From: Mm2PL Date: Sat, 2 Dec 2023 14:47:24 +0100 Subject: [PATCH 03/11] Don't use jthread because we can't --- src/RunGui.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/RunGui.cpp b/src/RunGui.cpp index 19ac9413c..9932055a9 100644 --- a/src/RunGui.cpp +++ b/src/RunGui.cpp @@ -245,7 +245,7 @@ void runGui(QApplication &a, Paths &paths, Settings &settings) restartOnSignal = value; }); - auto thread = std::jthread([dir = paths.miscDirectory] { + auto thread = std::thread([dir = paths.miscDirectory] { #ifdef Q_OS_WIN32 { auto path = combinePath(dir, "Update.exe"); @@ -311,6 +311,7 @@ void runGui(QApplication &a, Paths &paths, Settings &settings) pajlada::Settings::SettingManager::gSave(); } + thread.join(); chatterino::NetworkManager::deinit(); #ifdef USEWINSDK From 4e37e833ba774267e516dd3c4f29049b0c4f6b51 Mon Sep 17 00:00:00 2001 From: Mm2PL Date: Sat, 2 Dec 2023 14:57:53 +0100 Subject: [PATCH 04/11] Update src/RunGui.cpp Co-authored-by: pajlada --- src/RunGui.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/RunGui.cpp b/src/RunGui.cpp index 9932055a9..1fe6ada4e 100644 --- a/src/RunGui.cpp +++ b/src/RunGui.cpp @@ -311,7 +311,9 @@ void runGui(QApplication &a, Paths &paths, Settings &settings) pajlada::Settings::SettingManager::gSave(); } - thread.join(); + if (thread.joinable()) { + thread.join(); + } chatterino::NetworkManager::deinit(); #ifdef USEWINSDK From bf9fde66643aea89905ea395a28d644619cbb49b Mon Sep 17 00:00:00 2001 From: Mm2PL Date: Sat, 2 Dec 2023 14:59:31 +0100 Subject: [PATCH 05/11] Update src/RunGui.cpp --- src/RunGui.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/RunGui.cpp b/src/RunGui.cpp index 1fe6ada4e..d533b6102 100644 --- a/src/RunGui.cpp +++ b/src/RunGui.cpp @@ -311,7 +311,8 @@ void runGui(QApplication &a, Paths &paths, Settings &settings) pajlada::Settings::SettingManager::gSave(); } - if (thread.joinable()) { + if (thread.joinable()) + { thread.join(); } chatterino::NetworkManager::deinit(); From 51e5b6690dd9cbebbe938c93f022a2ce8d6f5967 Mon Sep 17 00:00:00 2001 From: Mm2PL Date: Sun, 3 Dec 2023 00:10:01 +0100 Subject: [PATCH 06/11] Call stop() when destroying BasicPubSubManager --- src/providers/liveupdates/BasicPubSubManager.hpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/providers/liveupdates/BasicPubSubManager.hpp b/src/providers/liveupdates/BasicPubSubManager.hpp index f82f70363..fc0781147 100644 --- a/src/providers/liveupdates/BasicPubSubManager.hpp +++ b/src/providers/liveupdates/BasicPubSubManager.hpp @@ -94,7 +94,10 @@ public: .toStdString()); } - virtual ~BasicPubSubManager() = default; + virtual ~BasicPubSubManager() + { + this->stop(); + }; BasicPubSubManager(const BasicPubSubManager &) = delete; BasicPubSubManager(const BasicPubSubManager &&) = delete; From fbf13c7a9e0de691cc54eb13ffe7d82f7fb780c0 Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sun, 3 Dec 2023 12:51:49 +0100 Subject: [PATCH 07/11] Destroy the window manager before TwitchIrcServer TwitchIrcServer contains the channels, and they now live longer than the windows themselves. I made it so the dtor of WindowManager *actually* deletes the windows now - i.e. them and everything in them are destroyed. --- src/Application.cpp | 2 +- src/Application.hpp | 2 +- src/singletons/WindowManager.cpp | 11 ++++++++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Application.cpp b/src/Application.cpp index 0a2f8b4de..c1d87af0c 100644 --- a/src/Application.cpp +++ b/src/Application.cpp @@ -109,6 +109,7 @@ Application::Application(Settings &_settings, Paths &_paths) , emotes(&this->emplace()) , accounts(&this->emplace()) , hotkeys(&this->emplace()) + , twitch(&this->emplace()) , windows(&this->emplace()) , toasts(&this->emplace()) , imageUploader(&this->emplace()) @@ -117,7 +118,6 @@ Application::Application(Settings &_settings, Paths &_paths) , commands(&this->emplace()) , notifications(&this->emplace()) , highlights(&this->emplace()) - , twitch(&this->emplace()) , chatterinoBadges(&this->emplace()) , ffzBadges(&this->emplace()) , seventvBadges(&this->emplace()) diff --git a/src/Application.hpp b/src/Application.hpp index 5dec1e906..063043618 100644 --- a/src/Application.hpp +++ b/src/Application.hpp @@ -98,6 +98,7 @@ public: Emotes *const emotes{}; AccountController *const accounts{}; HotkeyController *const hotkeys{}; + TwitchIrcServer *const twitch{}; WindowManager *const windows{}; Toasts *const toasts{}; ImageUploader *const imageUploader{}; @@ -106,7 +107,6 @@ public: CommandController *const commands{}; NotificationController *const notifications{}; HighlightController *const highlights{}; - TwitchIrcServer *const twitch{}; ChatterinoBadges *const chatterinoBadges{}; FfzBadges *const ffzBadges{}; SeventvBadges *const seventvBadges{}; diff --git a/src/singletons/WindowManager.cpp b/src/singletons/WindowManager.cpp index a86fdbc5a..1484e980a 100644 --- a/src/singletons/WindowManager.cpp +++ b/src/singletons/WindowManager.cpp @@ -127,7 +127,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() { From 295b38a8f1ac873ab67909d6854d607579d0c009 Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sun, 3 Dec 2023 12:58:11 +0100 Subject: [PATCH 08/11] Add a parent to SplitInput's QCompleter mini memory leak fix --- src/widgets/splits/SplitInput.cpp | 8 +++++--- src/widgets/splits/SplitInput.hpp | 3 ++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/widgets/splits/SplitInput.cpp b/src/widgets/splits/SplitInput.cpp index 2d94314ac..ba8a0418b 100644 --- a/src/widgets/splits/SplitInput.cpp +++ b/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(); diff --git a/src/widgets/splits/SplitInput.hpp b/src/widgets/splits/SplitInput.hpp index 56504437a..96c4e2b82 100644 --- a/src/widgets/splits/SplitInput.hpp +++ b/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; From 934e518e18665445a88b198f26de5273c2e64687 Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sun, 3 Dec 2023 13:45:37 +0100 Subject: [PATCH 09/11] Stop the 7tv heartbeat timer on shutdown request --- .../liveupdates/BasicPubSubClient.hpp | 11 ++++++++++ src/providers/seventv/eventapi/Client.cpp | 22 ++++++++++++------- src/providers/seventv/eventapi/Client.hpp | 2 ++ 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/providers/liveupdates/BasicPubSubClient.hpp b/src/providers/liveupdates/BasicPubSubClient.hpp index 31532c1dd..27b1a798c 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 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_; diff --git a/src/providers/seventv/eventapi/Client.cpp b/src/providers/seventv/eventapi/Client.cpp index f266478ce..465cbc03d 100644 --- a/src/providers/seventv/eventapi/Client.cpp +++ b/src/providers/seventv/eventapi/Client.cpp @@ -13,6 +13,8 @@ Client::Client(liveupdates::WebsocketClient &websocketClient, : BasicPubSubClient(websocketClient, std::move(handle)) , lastHeartbeat_(std::chrono::steady_clock::now()) , heartbeatInterval_(heartbeatInterval) + , heartbeatTimer_(std::make_shared( + 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(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..511488327 100644 --- a/src/providers/seventv/eventapi/Client.hpp +++ b/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 heartbeatTimer_; friend SeventvEventAPI; }; From 06cb7d5259c1ffdfecebd724793b3b71eb9942c5 Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sun, 3 Dec 2023 13:51:58 +0100 Subject: [PATCH 10/11] Rename the BasicPubSubManager thread --- src/CMakeLists.txt | 1 + .../liveupdates/BasicPubSubManager.hpp | 3 +++ src/util/RenameThread.hpp | 19 +++++++++++++++++++ 3 files changed, 23 insertions(+) create mode 100644 src/util/RenameThread.hpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 86c24c15b..7f9c9146f 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -486,6 +486,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/providers/liveupdates/BasicPubSubManager.hpp b/src/providers/liveupdates/BasicPubSubManager.hpp index fc0781147..58c4bbd75 100644 --- a/src/providers/liveupdates/BasicPubSubManager.hpp +++ b/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 #include @@ -118,6 +119,8 @@ public: this->mainThread_.reset(new std::thread([this] { runThread(); })); + + renameThread(*this->mainThread_.get(), "BPSM"); } void stop() diff --git a/src/util/RenameThread.hpp b/src/util/RenameThread.hpp new file mode 100644 index 000000000..0cc1c6506 --- /dev/null +++ b/src/util/RenameThread.hpp @@ -0,0 +1,19 @@ +#pragma once + +#include + +#ifdef Q_OS_LINUX +# include +#endif + +namespace chatterino { + +template +void renameThread(T &&thread, const char *threadName) +{ +#ifdef Q_OS_LINUX + pthread_setname_np(thread.native_handle(), threadName); +#endif +} + +} // namespace chatterino From b62d2577cb7d1169fd1a89aa52ca4fb9c6fac3e7 Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sun, 7 Jan 2024 14:38:27 +0100 Subject: [PATCH 11/11] Remove now-unneccessary fakeDtor --- src/Application.cpp | 5 ----- src/Application.hpp | 6 ------ src/RunGui.cpp | 2 +- 3 files changed, 1 insertion(+), 12 deletions(-) diff --git a/src/Application.cpp b/src/Application.cpp index 7ce1c67c3..7b3e9a0ad 100644 --- a/src/Application.cpp +++ b/src/Application.cpp @@ -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); diff --git a/src/Application.hpp b/src/Application.hpp index 737989169..66eb79fa5 100644 --- a/src/Application.hpp +++ b/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(); diff --git a/src/RunGui.cpp b/src/RunGui.cpp index 50ca92731..a1283ab7b 100644 --- a/src/RunGui.cpp +++ b/src/RunGui.cpp @@ -294,6 +294,6 @@ void runGui(QApplication &a, Paths &paths, Settings &settings, const Args &args) // flushing windows clipboard to keep copied messages flushClipboard(); #endif - app.fakeDtor(); } + } // namespace chatterino