Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanly exit Chatterino instead of force exiting #4993

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
- Dev: Move `clang-tidy` checker to its own CI job. (#4996)
- 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)
- 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)
Expand Down
7 changes: 1 addition & 6 deletions src/Application.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>())
Expand All @@ -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>())
Expand All @@ -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);
Expand Down
8 changes: 1 addition & 7 deletions src/Application.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,6 @@ class Application : public IApplication
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();
Expand All @@ -117,6 +111,7 @@ class Application : public IApplication
Emotes *const emotes{};
AccountController *const accounts{};
HotkeyController *const hotkeys{};
TwitchIrcServer *const twitch{};
WindowManager *const windows{};
Toasts *const toasts{};
ImageUploader *const imageUploader{};
Expand All @@ -126,7 +121,6 @@ class Application : public IApplication
CommandController *const commands{};
NotificationController *const notifications{};
HighlightController *const highlights{};
TwitchIrcServer *const twitch{};
ChatterinoBadges *const chatterinoBadges{};
FfzBadges *const ffzBadges{};
SeventvBadges *const seventvBadges{};
Expand Down
1 change: 1 addition & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,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
Expand Down
12 changes: 8 additions & 4 deletions src/RunGui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <QtConcurrent>

#include <csignal>
#include <thread>
#include <tuple>

#ifdef USEWINSDK
Expand Down Expand Up @@ -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))
Expand All @@ -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.
Expand Down Expand Up @@ -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
11 changes: 11 additions & 0 deletions src/providers/liveupdates/BasicPubSubClient.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,15 @@ class BasicPubSubClient
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:
Expand All @@ -164,6 +173,8 @@ class BasicPubSubClient
{
assert(this->isStarted());
this->started_.store(false, std::memory_order_release);

this->stopImpl();
}

liveupdates::WebsocketHandle handle_;
Expand Down
8 changes: 7 additions & 1 deletion src/providers/liveupdates/BasicPubSubManager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -94,7 +95,10 @@ class BasicPubSubManager
.toStdString());
}

virtual ~BasicPubSubManager() = default;
virtual ~BasicPubSubManager()
{
this->stop();
};

BasicPubSubManager(const BasicPubSubManager &) = delete;
BasicPubSubManager(const BasicPubSubManager &&) = delete;
Expand All @@ -115,6 +119,8 @@ class BasicPubSubManager
this->mainThread_.reset(new std::thread([this] {
runThread();
}));

renameThread(*this->mainThread_.get(), "BPSM");
}

void stop()
Expand Down
22 changes: 14 additions & 8 deletions src/providers/seventv/eventapi/Client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
{
}

Expand All @@ -23,6 +25,11 @@ void Client::onConnectionEstablished()
this->checkHeartbeat();
}

void Client::stopImpl()
{
this->heartbeatTimer_->cancel();
}

void Client::setHeartbeatInterval(int intervalMs)
{
qCDebug(chatterinoSeventvEventAPI)
Expand Down Expand Up @@ -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
2 changes: 2 additions & 0 deletions src/providers/seventv/eventapi/Client.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class Client : public BasicPubSubClient<Subscription>

protected:
void onConnectionEstablished() override;
void stopImpl() override;

private:
void checkHeartbeat();
Expand All @@ -32,6 +33,7 @@ class Client : public BasicPubSubClient<Subscription>
lastHeartbeat_;
// This will be set once on the welcome message.
std::chrono::milliseconds heartbeatInterval_;
std::shared_ptr<boost::asio::steady_timer> heartbeatTimer_;

friend SeventvEventAPI;
};
Expand Down
11 changes: 10 additions & 1 deletion src/singletons/WindowManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,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()
{
Expand Down
19 changes: 19 additions & 0 deletions src/util/RenameThread.hpp
Original file line number Diff line number Diff line change
@@ -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
8 changes: 5 additions & 3 deletions src/widgets/splits/SplitInput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

Expand All @@ -76,6 +76,8 @@ SplitInput::SplitInput(QWidget *parent, Split *_chatWidget,
});
}

SplitInput::~SplitInput() = default;

void SplitInput::initLayout()
{
auto app = getApp();
Expand Down
3 changes: 2 additions & 1 deletion src/widgets/splits/SplitInput.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,15 @@ class ResizingTextEdit;
class ChannelView;
enum class CompletionKind;

class SplitInput : public BaseWidget
class SplitInput final : public BaseWidget
pajlada marked this conversation as resolved.
Show resolved Hide resolved
{
Q_OBJECT

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;
Expand Down
Loading