diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fd01100..d2b948c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ - Bugfix: Fixed an issue where Splits could get lost by dragging it onto your Recycle Bin. (#6147) - Bugfix: Fixed some Twitch commands not getting tab-completed correctly. (#6143) - Bugfix: Fixed shared chat badges displaying pixelated when Chatterino is scaled too much. (#6146) +- Bugfix: Fixed a few crashes that could occur when Chatterino was shutting down, some related to network tasks still firing despite us shutting down. (#6187) - Bugfix: Fixed a crash that could occur when eventsub was enabled and Chatterino was attached to a conhost on Windows that was later gone. (#6161) - Bugfix: Fixed a crash that could occur an eventsub connection's keepalive timer would run after the connection was dead, causing the keepalive timer to use-itself-after-free. (#6204) - Bugfix: Fixed a crash that could occur when an image started loading mid app shutdown. (#6213) diff --git a/src/Application.cpp b/src/Application.cpp index 1de04c43..d8ae6606 100644 --- a/src/Application.cpp +++ b/src/Application.cpp @@ -70,6 +70,9 @@ using namespace chatterino; const QString BTTV_LIVE_UPDATES_URL = "wss://sockets.betterttv.net/ws"; const QString SEVENTV_EVENTAPI_URL = "wss://events.7tv.io/v3"; +std::atomic STOPPED{false}; +std::atomic ABOUT_TO_QUIT{false}; + ISoundController *makeSoundController(Settings &settings) { SoundBackend soundBackend = settings.soundBackend; @@ -200,8 +203,6 @@ Application::Application(Settings &_settings, const Paths &paths, Application::~Application() { - this->eventSub->setQuitting(); - // we do this early to ensure getApp isn't used in any dtors INSTANCE = nullptr; } @@ -594,12 +595,60 @@ eventsub::IController *Application::getEventSub() return this->eventSub.get(); } -void Application::save() +void Application::aboutToQuit() { + ABOUT_TO_QUIT.store(true); + + this->eventSub->setQuitting(); + + this->twitch->aboutToQuit(); + this->hotkeys->save(); this->windows->save(); } +void Application::stop() +{ +#ifdef CHATTERINO_HAVE_PLUGINS + this->plugins.reset(); +#endif + this->pronouns.reset(); + this->twitchUsers.reset(); + this->streamerMode.reset(); + this->linkResolver.reset(); + this->seventvEventAPI.reset(); + this->seventvEmotes.reset(); + this->ffzEmotes.reset(); + this->bttvLiveUpdates.reset(); + this->bttvEmotes.reset(); + this->chatterinoBadges.reset(); + this->twitchBadges.reset(); + this->twitchPubSub.reset(); + this->twitchLiveController.reset(); + this->sound.reset(); + this->userData.reset(); + this->seventvBadges.reset(); + this->ffzBadges.reset(); + this->twitch.reset(); + this->highlights.reset(); + this->notifications.reset(); + this->commands.reset(); + this->crashHandler.reset(); + this->seventvAPI.reset(); + this->imageUploader.reset(); + this->toasts.reset(); + this->windows.reset(); + this->hotkeys.reset(); + this->eventSub.reset(); + this->accounts.reset(); + this->emotes.reset(); + this->logging.reset(); + this->fonts.reset(); + this->themes.reset(); + + STOPPED.store(true); +} + void Application::initNm(const Paths &paths) { (void)paths; @@ -613,6 +662,7 @@ void Application::initNm(const Paths &paths) IApplication *getApp() { assert(INSTANCE != nullptr); + assert(STOPPED.load() == false); return INSTANCE; } @@ -622,4 +672,9 @@ IApplication *tryGetApp() return INSTANCE; } +bool isAppAboutToQuit() +{ + return ABOUT_TO_QUIT.load(); +} + } // namespace chatterino diff --git a/src/Application.hpp b/src/Application.hpp index c6f7147f..d00935f9 100644 --- a/src/Application.hpp +++ b/src/Application.hpp @@ -139,7 +139,8 @@ public: void initialize(Settings &settings, const Paths &paths); void load(); - void save(); + void aboutToQuit(); + void stop(); int run(); @@ -148,7 +149,7 @@ public: private: std::unique_ptr themes; std::unique_ptr fonts; - const std::unique_ptr logging; + std::unique_ptr logging; std::unique_ptr emotes; std::unique_ptr accounts; std::unique_ptr eventSub; @@ -246,4 +247,6 @@ IApplication *getApp(); /// Might return `nullptr` if the app is being destroyed IApplication *tryGetApp(); +bool isAppAboutToQuit(); + } // namespace chatterino diff --git a/src/RunGui.cpp b/src/RunGui.cpp index e49412ce..bbeb0d3b 100644 --- a/src/RunGui.cpp +++ b/src/RunGui.cpp @@ -282,13 +282,13 @@ void runGui(QApplication &a, const Paths &paths, Settings &settings, QObject::connect(qApp, &QApplication::aboutToQuit, [] { auto *app = dynamic_cast(tryGetApp()); - if (app) - { - app->save(); - } + assert(app != nullptr); + app->aboutToQuit(); getSettings()->requestSave(); getSettings()->disableSave(); + + app->stop(); }); Application app(settings, paths, args, updates); diff --git a/src/common/network/NetworkPrivate.cpp b/src/common/network/NetworkPrivate.cpp index 92615129..c7f0625e 100644 --- a/src/common/network/NetworkPrivate.cpp +++ b/src/common/network/NetworkPrivate.cpp @@ -131,6 +131,14 @@ void NetworkData::emitSuccess(NetworkResult &&result) return; } + if (isAppAboutToQuit()) + { + qCDebug(chatterinoHTTP) + << "Success callback for" << url.toString() + << "skipped because we're about to quit"; + return; + } + QElapsedTimer timer; timer.start(); cb(result); @@ -153,12 +161,21 @@ void NetworkData::emitError(NetworkResult &&result) runCallback(this->executeConcurrently, [cb = std::move(this->onError), result = std::move(result), - hasCaller = this->hasCaller, caller = this->caller]() { + url = this->request.url(), hasCaller = this->hasCaller, + caller = this->caller]() { if (hasCaller && caller.isNull()) { return; } + if (isAppAboutToQuit()) + { + qCDebug(chatterinoHTTP) + << "Error callback for" << url.toString() + << "skipped because we're about to quit"; + return; + } + cb(result); }); } @@ -172,12 +189,20 @@ void NetworkData::emitFinally() runCallback(this->executeConcurrently, [cb = std::move(this->finally), hasCaller = this->hasCaller, - caller = this->caller]() { + url = this->request.url(), caller = this->caller]() { if (hasCaller && caller.isNull()) { return; } + if (isAppAboutToQuit()) + { + qCDebug(chatterinoHTTP) + << "Finally callback for" << url.toString() + << "skipped because we're about to quit"; + return; + } + cb(); }); } diff --git a/src/messages/Image.cpp b/src/messages/Image.cpp index 8c2a8300..70f6b4db 100644 --- a/src/messages/Image.cpp +++ b/src/messages/Image.cpp @@ -282,6 +282,15 @@ Image::~Image() return; } + if (isAppAboutToQuit()) + { + if (this->frames_) + { + std::ignore = this->frames_.release(); + } + return; + } + // Ensure the destructor for our frames is called in the GUI thread // If the Image destructor is called outside of the GUI thread, move the // ownership of the frames to the GUI thread, otherwise the frames will be @@ -488,6 +497,8 @@ void Image::actuallyLoad() return; } + assert(!isAppAboutToQuit()); + QBuffer buffer; buffer.setData(result.getData()); QImageReader reader(&buffer); diff --git a/src/providers/recentmessages/Api.cpp b/src/providers/recentmessages/Api.cpp index 855e31f4..4b9227d9 100644 --- a/src/providers/recentmessages/Api.cpp +++ b/src/providers/recentmessages/Api.cpp @@ -1,5 +1,6 @@ #include "providers/recentmessages/Api.hpp" +#include "Application.hpp" #include "common/network/NetworkRequest.hpp" #include "common/network/NetworkResult.hpp" #include "common/QLogging.hpp" @@ -33,8 +34,15 @@ void load( const long delayMs = jitter ? std::rand() % 100 : 0; QTimer::singleShot(delayMs, [=] { + if (isAppAboutToQuit()) + { + return; + } + NetworkRequest(url) .onSuccess([channelPtr, onLoaded](const auto &result) { + assert(!isAppAboutToQuit()); + auto shared = channelPtr.lock(); if (!shared) { @@ -51,30 +59,33 @@ void load( auto builtMessages = buildRecentMessages(parsedMessages, shared.get()); - postToThread([shared = std::move(shared), - root = std::move(root), - messages = std::move(builtMessages), - onLoaded]() mutable { - // Notify user about a possible gap in logs if it returned some messages - // but isn't currently joined to a channel - const auto errorCode = root.value("error_code").toString(); - if (!errorCode.isEmpty()) - { - qCDebug(LOG) - << QString("Got error from API: error_code=%1, " - "channel=%2") - .arg(errorCode, shared->getName()); - if (errorCode == "channel_not_joined" && - !messages.empty()) - { - shared->addSystemMessage( - "Message history service recovering, there may " - "be gaps in the message history."); - } - } + postToThread( + [shared = std::move(shared), root = std::move(root), + messages = std::move(builtMessages), onLoaded]() mutable { + assert(!isAppAboutToQuit()); - onLoaded(messages); - }); + // Notify user about a possible gap in logs if it returned some messages + // but isn't currently joined to a channel + const auto errorCode = + root.value("error_code").toString(); + if (!errorCode.isEmpty()) + { + qCDebug(LOG) + << QString("Got error from API: error_code=%1, " + "channel=%2") + .arg(errorCode, shared->getName()); + if (errorCode == "channel_not_joined" && + !messages.empty()) + { + shared->addSystemMessage( + "Message history service recovering, there " + "may " + "be gaps in the message history."); + } + } + + onLoaded(messages); + }); }) .onError([channelPtr, onError](const NetworkResult &result) { auto shared = channelPtr.lock(); @@ -82,6 +93,7 @@ void load( { return; } + assert(!isAppAboutToQuit()); qCDebug(LOG) << "Failed to load recent messages for" << shared->getName(); diff --git a/src/providers/seventv/SeventvEmotes.cpp b/src/providers/seventv/SeventvEmotes.cpp index 1ed27984..d8846150 100644 --- a/src/providers/seventv/SeventvEmotes.cpp +++ b/src/providers/seventv/SeventvEmotes.cpp @@ -440,6 +440,8 @@ void SeventvEmotes::getEmoteSet( getApp()->getSeventvAPI()->getEmoteSet( emoteSetId, [callback = std::move(successCallback), emoteSetId](const auto &json) { + assert(!isAppAboutToQuit()); + auto parsedEmotes = json["emotes"].toArray(); auto emoteMap = parseEmotes(parsedEmotes, false); diff --git a/src/providers/twitch/ChannelPointReward.cpp b/src/providers/twitch/ChannelPointReward.cpp index 0d4ab132..fff96de4 100644 --- a/src/providers/twitch/ChannelPointReward.cpp +++ b/src/providers/twitch/ChannelPointReward.cpp @@ -106,13 +106,13 @@ ChannelPointReward::ChannelPointReward(const QJsonObject &redemption) } else { - static const ImageSet defaultImage{ + static const ImageSet *defaultImage = new ImageSet{ Image::fromUrl({twitchChannelPointRewardUrl("1.png")}, 1, baseSize), Image::fromUrl({twitchChannelPointRewardUrl("2.png")}, 0.5, baseSize * 2), Image::fromUrl({twitchChannelPointRewardUrl("4.png")}, 0.25, baseSize * 4)}; - this->image = defaultImage; + this->image = *defaultImage; } } diff --git a/src/providers/twitch/TwitchChannel.cpp b/src/providers/twitch/TwitchChannel.cpp index df2d8b1b..34393552 100644 --- a/src/providers/twitch/TwitchChannel.cpp +++ b/src/providers/twitch/TwitchChannel.cpp @@ -1381,6 +1381,7 @@ void TwitchChannel::loadRecentMessages() recentmessages::load( this->getName(), weak, [weak](const auto &messages) { + assert(!isAppAboutToQuit()); auto shared = weak.lock(); if (!shared) { diff --git a/src/providers/twitch/TwitchIrcServer.cpp b/src/providers/twitch/TwitchIrcServer.cpp index 7eca81f9..1e0f7501 100644 --- a/src/providers/twitch/TwitchIrcServer.cpp +++ b/src/providers/twitch/TwitchIrcServer.cpp @@ -254,6 +254,11 @@ void TwitchIrcServer::initialize() auto reward = ChannelPointReward(data); postToThread([chan, reward] { + if (isAppAboutToQuit()) + { + return; + } + if (auto *channel = dynamic_cast(chan.get())) { channel->addChannelPointReward(reward); @@ -262,6 +267,13 @@ void TwitchIrcServer::initialize() }); } +void TwitchIrcServer::aboutToQuit() +{ + this->signalHolder.clear(); + + this->channels.clear(); +} + void TwitchIrcServer::initializeConnection(IrcConnection *connection, ConnectionType type) { @@ -550,8 +562,8 @@ std::shared_ptr TwitchIrcServer::getCustomChannel( return this->automodChannel; } - static auto getTimer = [](ChannelPtr channel, int msBetweenMessages, - bool addInitialMessages) { + static auto getTimer = [this](ChannelPtr channel, int msBetweenMessages, + bool addInitialMessages) { if (addInitialMessages) { for (auto i = 0; i < 1000; i++) @@ -561,7 +573,7 @@ std::shared_ptr TwitchIrcServer::getCustomChannel( } auto *timer = new QTimer; - QObject::connect(timer, &QTimer::timeout, [channel] { + QObject::connect(timer, &QTimer::timeout, this, [channel] { channel->addSystemMessage(QTime::currentTime().toString()); }); timer->start(msBetweenMessages); @@ -841,37 +853,43 @@ void TwitchIrcServer::initEventAPIs(BttvLiveUpdates *bttvLiveUpdates, bttvLiveUpdates->signals_.emoteAdded, [&](const auto &data) { auto chan = this->getChannelOrEmptyByID(data.channelID); - postToThread([chan, data] { - if (auto *channel = - dynamic_cast(chan.get())) - { - channel->addBttvEmote(data); - } - }); + postToThread( + [chan, data] { + if (auto *channel = + dynamic_cast(chan.get())) + { + channel->addBttvEmote(data); + } + }, + this); }); this->signalHolder.managedConnect( bttvLiveUpdates->signals_.emoteUpdated, [&](const auto &data) { auto chan = this->getChannelOrEmptyByID(data.channelID); - postToThread([chan, data] { - if (auto *channel = - dynamic_cast(chan.get())) - { - channel->updateBttvEmote(data); - } - }); + postToThread( + [chan, data] { + if (auto *channel = + dynamic_cast(chan.get())) + { + channel->updateBttvEmote(data); + } + }, + this); }); this->signalHolder.managedConnect( bttvLiveUpdates->signals_.emoteRemoved, [&](const auto &data) { auto chan = this->getChannelOrEmptyByID(data.channelID); - postToThread([chan, data] { - if (auto *channel = - dynamic_cast(chan.get())) - { - channel->removeBttvEmote(data); - } - }); + postToThread( + [chan, data] { + if (auto *channel = + dynamic_cast(chan.get())) + { + channel->removeBttvEmote(data); + } + }, + this); }); bttvLiveUpdates->start(); @@ -886,30 +904,36 @@ void TwitchIrcServer::initEventAPIs(BttvLiveUpdates *bttvLiveUpdates, { this->signalHolder.managedConnect( seventvEventAPI->signals_.emoteAdded, [this](const auto &data) { - postToThread([this, data] { - this->forEachSeventvEmoteSet(data.emoteSetID, - [data](TwitchChannel &chan) { - chan.addSeventvEmote(data); - }); - }); + postToThread( + [this, data] { + this->forEachSeventvEmoteSet( + data.emoteSetID, [data](TwitchChannel &chan) { + chan.addSeventvEmote(data); + }); + }, + this); }); this->signalHolder.managedConnect( seventvEventAPI->signals_.emoteUpdated, [this](const auto &data) { - postToThread([this, data] { - this->forEachSeventvEmoteSet( - data.emoteSetID, [data](TwitchChannel &chan) { - chan.updateSeventvEmote(data); - }); - }); + postToThread( + [this, data] { + this->forEachSeventvEmoteSet( + data.emoteSetID, [data](TwitchChannel &chan) { + chan.updateSeventvEmote(data); + }); + }, + this); }); this->signalHolder.managedConnect( seventvEventAPI->signals_.emoteRemoved, [this](const auto &data) { - postToThread([this, data] { - this->forEachSeventvEmoteSet( - data.emoteSetID, [data](TwitchChannel &chan) { - chan.removeSeventvEmote(data); - }); - }); + postToThread( + [this, data] { + this->forEachSeventvEmoteSet( + data.emoteSetID, [data](TwitchChannel &chan) { + chan.removeSeventvEmote(data); + }); + }, + this); }); this->signalHolder.managedConnect( seventvEventAPI->signals_.userUpdated, [this](const auto &data) { diff --git a/src/providers/twitch/TwitchIrcServer.hpp b/src/providers/twitch/TwitchIrcServer.hpp index e155fb13..b5c541b4 100644 --- a/src/providers/twitch/TwitchIrcServer.hpp +++ b/src/providers/twitch/TwitchIrcServer.hpp @@ -94,6 +94,8 @@ public: void initialize(); + void aboutToQuit(); + void forEachChannelAndSpecialChannels( std::function func) override; diff --git a/src/providers/twitch/eventsub/Connection.cpp b/src/providers/twitch/eventsub/Connection.cpp index c1361b17..09074b0c 100644 --- a/src/providers/twitch/eventsub/Connection.cpp +++ b/src/providers/twitch/eventsub/Connection.cpp @@ -75,6 +75,11 @@ void Connection::onNotification(const lib::messages::Metadata &metadata, void Connection::onClose(std::unique_ptr self, const std::optional &reconnectURL) { + if (isAppAboutToQuit()) + { + return; + } + auto *app = tryGetApp(); if (!app) { diff --git a/src/providers/twitch/eventsub/Controller.cpp b/src/providers/twitch/eventsub/Controller.cpp index ba613e48..91f140e4 100644 --- a/src/providers/twitch/eventsub/Controller.cpp +++ b/src/providers/twitch/eventsub/Controller.cpp @@ -104,7 +104,10 @@ Controller::~Controller() connection->close(); } - this->subscriptions.clear(); + { + std::lock_guard lock(this->subscriptionsMutex); + this->subscriptions.clear(); + } this->work.reset(); @@ -341,7 +344,7 @@ void Controller::subscribe(const SubscriptionRequest &request, bool isRetry) qCDebug(LOG) << "Make helix request for" << request; getHelix()->createEventSubSubscription( request, listener->getSessionID(), - [this, request, connection, + [this, request, weakConnection{std::weak_ptr(connection)}]( const auto &res) { qCDebug(LOG) << "Subscription success" << request; @@ -507,6 +510,12 @@ void Controller::registerConnection(std::weak_ptr &&connection) void Controller::retrySubscription(const SubscriptionRequest &request) { + if (isAppAboutToQuit()) + { + qCDebug(LOG) << "retrySubscription, but app is quitting" << request; + return; + } + std::lock_guard lock(this->subscriptionsMutex); auto &subscription = this->subscriptions[request]; @@ -536,6 +545,14 @@ void Controller::retrySubscription(const SubscriptionRequest &request) std::make_unique(this->ioContext); retryTimer->expires_after(subscription.backoff.next() + jitter); retryTimer->async_wait([this, request](const auto &ec) { + if (isAppAboutToQuit()) + { + qCDebug(LOG) + << "Retry was going to fire, but app is quitting so we won't." + << request; + return; + } + if (!ec) { qCDebug(LOG) << "Firing retry" << request; diff --git a/src/util/Helpers.cpp b/src/util/Helpers.cpp index 8cbfb53f..2472308b 100644 --- a/src/util/Helpers.cpp +++ b/src/util/Helpers.cpp @@ -396,7 +396,14 @@ void removeLastQS(QString &str) void writeProviderEmotesCache(const QString &id, const QString &provider, const QByteArray &bytes) { - QThreadPool::globalInstance()->start([bytes, id, provider]() { + auto *threadPool = QThreadPool::globalInstance(); + if (threadPool == nullptr) + { + // Must be exiting - do nothing + return; + } + + threadPool->start([bytes, id, provider]() { auto cacheKey = id % "." % provider; QFile responseCache(getApp()->getPaths().cacheFilePath(cacheKey)); diff --git a/src/widgets/helper/NotebookButton.cpp b/src/widgets/helper/NotebookButton.cpp index 653af872..ee6d400d 100644 --- a/src/widgets/helper/NotebookButton.cpp +++ b/src/widgets/helper/NotebookButton.cpp @@ -1,5 +1,6 @@ #include "widgets/helper/NotebookButton.hpp" +#include "Application.hpp" #include "common/QLogging.hpp" #include "singletons/Theme.hpp" #include "widgets/helper/Button.hpp" @@ -213,11 +214,21 @@ void NotebookButton::dropEvent(QDropEvent *event) void NotebookButton::hideEvent(QHideEvent *) { + if (isAppAboutToQuit()) + { + return; + } + this->parent_->refresh(); } void NotebookButton::showEvent(QShowEvent *) { + if (isAppAboutToQuit()) + { + return; + } + this->parent_->refresh(); } diff --git a/src/widgets/splits/SplitHeader.cpp b/src/widgets/splits/SplitHeader.cpp index d75712e4..764d7175 100644 --- a/src/widgets/splits/SplitHeader.cpp +++ b/src/widgets/splits/SplitHeader.cpp @@ -848,6 +848,8 @@ void SplitHeader::updateChannelText() NetworkRequest(url, NetworkRequestType::Get) .caller(this) .onSuccess([this](auto result) { + assert(!isAppAboutToQuit()); + // NOTE: We do not follow the redirects, so we need to make sure we only treat code 200 as a valid image if (result.status() == 200) {