diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a2691d36..2b97e7241 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/src/Application.cpp b/src/Application.cpp index e13624c7e..7b3e9a0ad 100644 --- a/src/Application.cpp +++ b/src/Application.cpp @@ -114,6 +114,7 @@ Application::Application(Settings &_settings, Paths &_paths, const Args &_args) , emotes(&this->emplace()) , accounts(&this->emplace()) , hotkeys(&this->emplace()) + , twitch(&this->emplace()) , windows(&this->emplace()) , toasts(&this->emplace()) , imageUploader(&this->emplace()) @@ -123,7 +124,6 @@ Application::Application(Settings &_settings, Paths &_paths, const Args &_args) , commands(&this->emplace()) , notifications(&this->emplace()) , highlights(&this->emplace()) - , twitch(&this->emplace()) , chatterinoBadges(&this->emplace()) , ffzBadges(&this->emplace()) , seventvBadges(&this->emplace()) @@ -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 fb84d7f50..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(); @@ -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{}; diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 8919ebb7c..6ab73561c 100644 --- a/src/CMakeLists.txt +++ b/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 diff --git a/src/RunGui.cpp b/src/RunGui.cpp index 994115961..a1283ab7b 100644 --- a/src/RunGui.cpp +++ b/src/RunGui.cpp @@ -21,6 +21,7 @@ #include #include +#include #include #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 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/liveupdates/BasicPubSubManager.hpp b/src/providers/liveupdates/BasicPubSubManager.hpp index f82f70363..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 @@ -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() 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; }; diff --git a/src/singletons/WindowManager.cpp b/src/singletons/WindowManager.cpp index 038c45f8f..4499db5d7 100644 --- a/src/singletons/WindowManager.cpp +++ b/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() { 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 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;