fix some crashes, mostly related to network requests (#6187)

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)

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<bool> STOPPED{false};
+std::atomic<bool> 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

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<Theme> themes;
     std::unique_ptr<Fonts> fonts;
-    const std::unique_ptr<Logging> logging;
+    std::unique_ptr<Logging> logging;
     std::unique_ptr<Emotes> emotes;
     std::unique_ptr<AccountController> accounts;
     std::unique_ptr<eventsub::IController> eventSub;
@@ -246,4 +247,6 @@ IApplication *getApp();
 /// Might return `nullptr` if the app is being destroyed
 IApplication *tryGetApp();
 
+bool isAppAboutToQuit();
+
 }  // namespace chatterino

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<Application *>(tryGetApp());
-        if (app)
+        assert(app != nullptr);
-        {
+        app->aboutToQuit();
-            app->save();
-        }
 
         getSettings()->requestSave();
         getSettings()->disableSave();
+
+        app->stop();
     });
 
     Application app(settings, paths, args, updates);

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();
                 });
 }

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);

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),
+                postToThread(
-                              root = std::move(root),
+                    [shared = std::move(shared), root = std::move(root),
-                              messages = std::move(builtMessages),
+                     messages = std::move(builtMessages), onLoaded]() mutable {
-                              onLoaded]() mutable {
+                        assert(!isAppAboutToQuit());
-                    // 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);
+                        // 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();

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);

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;
     }
 }
 

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)
             {

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<TwitchChannel *>(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<Channel> TwitchIrcServer::getCustomChannel(
         return this->automodChannel;
     }
 
-    static auto getTimer = [](ChannelPtr channel, int msBetweenMessages,
+    static auto getTimer = [this](ChannelPtr channel, int msBetweenMessages,
-                              bool addInitialMessages) {
+                                  bool addInitialMessages) {
         if (addInitialMessages)
         {
             for (auto i = 0; i < 1000; i++)
@@ -561,7 +573,7 @@ std::shared_ptr<Channel> 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] {
+                postToThread(
-                    if (auto *channel =
+                    [chan, data] {
-                            dynamic_cast<TwitchChannel *>(chan.get()))
+                        if (auto *channel =
-                    {
+                                dynamic_cast<TwitchChannel *>(chan.get()))
-                        channel->addBttvEmote(data);
+                        {
-                    }
+                            channel->addBttvEmote(data);
-                });
+                        }
+                    },
+                    this);
             });
         this->signalHolder.managedConnect(
             bttvLiveUpdates->signals_.emoteUpdated, [&](const auto &data) {
                 auto chan = this->getChannelOrEmptyByID(data.channelID);
 
-                postToThread([chan, data] {
+                postToThread(
-                    if (auto *channel =
+                    [chan, data] {
-                            dynamic_cast<TwitchChannel *>(chan.get()))
+                        if (auto *channel =
-                    {
+                                dynamic_cast<TwitchChannel *>(chan.get()))
-                        channel->updateBttvEmote(data);
+                        {
-                    }
+                            channel->updateBttvEmote(data);
-                });
+                        }
+                    },
+                    this);
             });
         this->signalHolder.managedConnect(
             bttvLiveUpdates->signals_.emoteRemoved, [&](const auto &data) {
                 auto chan = this->getChannelOrEmptyByID(data.channelID);
 
-                postToThread([chan, data] {
+                postToThread(
-                    if (auto *channel =
+                    [chan, data] {
-                            dynamic_cast<TwitchChannel *>(chan.get()))
+                        if (auto *channel =
-                    {
+                                dynamic_cast<TwitchChannel *>(chan.get()))
-                        channel->removeBttvEmote(data);
+                        {
-                    }
+                            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] {
+                postToThread(
-                    this->forEachSeventvEmoteSet(data.emoteSetID,
+                    [this, data] {
-                                                 [data](TwitchChannel &chan) {
+                        this->forEachSeventvEmoteSet(
-                                                     chan.addSeventvEmote(data);
+                            data.emoteSetID, [data](TwitchChannel &chan) {
-                                                 });
+                                chan.addSeventvEmote(data);
-                });
+                            });
+                    },
+                    this);
             });
         this->signalHolder.managedConnect(
             seventvEventAPI->signals_.emoteUpdated, [this](const auto &data) {
-                postToThread([this, data] {
+                postToThread(
-                    this->forEachSeventvEmoteSet(
+                    [this, data] {
-                        data.emoteSetID, [data](TwitchChannel &chan) {
+                        this->forEachSeventvEmoteSet(
-                            chan.updateSeventvEmote(data);
+                            data.emoteSetID, [data](TwitchChannel &chan) {
-                        });
+                                chan.updateSeventvEmote(data);
-                });
+                            });
+                    },
+                    this);
             });
         this->signalHolder.managedConnect(
             seventvEventAPI->signals_.emoteRemoved, [this](const auto &data) {
-                postToThread([this, data] {
+                postToThread(
-                    this->forEachSeventvEmoteSet(
+                    [this, data] {
-                        data.emoteSetID, [data](TwitchChannel &chan) {
+                        this->forEachSeventvEmoteSet(
-                            chan.removeSeventvEmote(data);
+                            data.emoteSetID, [data](TwitchChannel &chan) {
-                        });
+                                chan.removeSeventvEmote(data);
-                });
+                            });
+                    },
+                    this);
             });
         this->signalHolder.managedConnect(
             seventvEventAPI->signals_.userUpdated, [this](const auto &data) {

src/providers/twitch/TwitchIrcServer.hpp

@@ -94,6 +94,8 @@ public:
 
     void initialize();
 
+    void aboutToQuit();
+
     void forEachChannelAndSpecialChannels(
         std::function<void(ChannelPtr)> func) override;
 

src/providers/twitch/eventsub/Connection.cpp

@@ -75,6 +75,11 @@ void Connection::onNotification(const lib::messages::Metadata &metadata,
 void Connection::onClose(std::unique_ptr<lib::Listener> self,
                          const std::optional<std::string> &reconnectURL)
 {
+    if (isAppAboutToQuit())
+    {
+        return;
+    }
+
     auto *app = tryGetApp();
     if (!app)
     {

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<lib::Session>(connection)}](
                 const auto &res) {
                 qCDebug(LOG) << "Subscription success" << request;
@@ -507,6 +510,12 @@ void Controller::registerConnection(std::weak_ptr<lib::Session> &&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<boost::asio::system_timer>(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;

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));
 

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();
 }
 

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)
                         {