fix: deadlock and use-after-free in tests (#4981)

* fix: use-after-free in settings

* refactor: put seventv api into a singleton

* chore: add changelog entry

* Add warning for when the 7TV load fails

CHANGELOG.md

@@ -69,6 +69,7 @@
 - Dev: Removed `Outcome` from network requests. (#4959)
 - 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)
 
 ## 2.4.6
 

mocks/include/mocks/EmptyApplication.hpp

@@ -94,6 +94,11 @@ class EmptyApplication : public IApplication
     {
         return nullptr;
     }
+
+    SeventvAPI *getSeventvAPI() override
+    {
+        return nullptr;
+    }
 };
 
 }  // namespace chatterino::mock

src/Application.cpp

@@ -10,6 +10,7 @@
 #include "controllers/hotkeys/HotkeyController.hpp"
 #include "controllers/ignores/IgnoreController.hpp"
 #include "controllers/notifications/NotificationController.hpp"
+#include "providers/seventv/SeventvAPI.hpp"
 #include "singletons/ImageUploader.hpp"
 #ifdef CHATTERINO_HAVE_PLUGINS
 #    include "controllers/plugins/PluginController.hpp"
@@ -81,6 +82,7 @@ Application::Application(Settings &_settings, Paths &_paths)
     , windows(&this->emplace<WindowManager>())
     , toasts(&this->emplace<Toasts>())
     , imageUploader(&this->emplace<ImageUploader>())
+    , seventvAPI(&this->emplace<SeventvAPI>())
 
     , commands(&this->emplace<CommandController>())
     , notifications(&this->emplace<NotificationController>())

src/Application.hpp

@@ -42,6 +42,7 @@ class ChatterinoBadges;
 class FfzBadges;
 class SeventvBadges;
 class ImageUploader;
+class SeventvAPI;
 
 class IApplication
 {
@@ -68,6 +69,7 @@ class IApplication
     virtual IUserDataController *getUserData() = 0;
     virtual ITwitchLiveController *getTwitchLiveController() = 0;
     virtual ImageUploader *getImageUploader() = 0;
+    virtual SeventvAPI *getSeventvAPI() = 0;
 };
 
 class Application : public IApplication
@@ -97,6 +99,7 @@ class Application : public IApplication
     WindowManager *const windows{};
     Toasts *const toasts{};
     ImageUploader *const imageUploader{};
+    SeventvAPI *const seventvAPI{};
 
     CommandController *const commands{};
     NotificationController *const notifications{};
@@ -174,6 +177,10 @@ class Application : public IApplication
     {
         return this->imageUploader;
     }
+    SeventvAPI *getSeventvAPI() override
+    {
+        return this->seventvAPI;
+    }
 
     pajlada::Signals::NoArgSignal streamerModeChanged;
 

src/providers/seventv/SeventvAPI.cpp

@@ -78,11 +78,5 @@ void SeventvAPI::updatePresence(const QString &twitchChannelID,
         .execute();
 }
 
-SeventvAPI &getSeventvAPI()
-{
-    static SeventvAPI instance;
-    return instance;
-}
-
 }  // namespace chatterino
 // NOLINTEND(readability-convert-member-functions-to-static)

src/providers/seventv/SeventvAPI.hpp

@@ -1,5 +1,7 @@
 #pragma once
 
+#include "common/Singleton.hpp"
+
 #include <functional>
 
 class QString;
@@ -9,25 +11,33 @@ namespace chatterino {
 
 class NetworkResult;
 
-class SeventvAPI
+class SeventvAPI : public Singleton
 {
     using ErrorCallback = std::function<void(const NetworkResult &)>;
     template <typename... T>
     using SuccessCallback = std::function<void(T...)>;
 
 public:
-    void getUserByTwitchID(const QString &twitchID,
-                           SuccessCallback<const QJsonObject &> &&onSuccess,
-                           ErrorCallback &&onError);
-    void getEmoteSet(const QString &emoteSet,
-                     SuccessCallback<const QJsonObject &> &&onSuccess,
-                     ErrorCallback &&onError);
-
-    void updatePresence(const QString &twitchChannelID,
-                        const QString &seventvUserID,
-                        SuccessCallback<> &&onSuccess, ErrorCallback &&onError);
+    SeventvAPI() = default;
+    ~SeventvAPI() override = default;
+
+    SeventvAPI(const SeventvAPI &) = delete;
+    SeventvAPI(SeventvAPI &&) = delete;
+    SeventvAPI &operator=(const SeventvAPI &) = delete;
+    SeventvAPI &operator=(SeventvAPI &&) = delete;
+
+    virtual void getUserByTwitchID(
+        const QString &twitchID,
+        SuccessCallback<const QJsonObject &> &&onSuccess,
+        ErrorCallback &&onError);
+    virtual void getEmoteSet(const QString &emoteSet,
+                             SuccessCallback<const QJsonObject &> &&onSuccess,
+                             ErrorCallback &&onError);
+
+    virtual void updatePresence(const QString &twitchChannelID,
+                                const QString &seventvUserID,
+                                SuccessCallback<> &&onSuccess,
+                                ErrorCallback &&onError);
 };
 
-SeventvAPI &getSeventvAPI();
-
 }  // namespace chatterino

src/providers/seventv/SeventvBadges.cpp

@@ -2,15 +2,12 @@
 
 #include "messages/Emote.hpp"
 #include "messages/Image.hpp"
-#include "providers/seventv/SeventvAPI.hpp"
 #include "providers/seventv/SeventvEmotes.hpp"
 
 #include <QJsonArray>
 #include <QUrl>
 #include <QUrlQuery>
 
-#include <map>
-
 namespace chatterino {
 
 std::optional<EmotePtr> SeventvBadges::getBadge(const UserId &id) const

src/providers/seventv/SeventvEmotes.cpp

@@ -1,5 +1,6 @@
 #include "providers/seventv/SeventvEmotes.hpp"
 
+#include "Application.hpp"
 #include "common/Literals.hpp"
 #include "common/NetworkResult.hpp"
 #include "common/QLogging.hpp"
@@ -214,7 +215,7 @@ void SeventvEmotes::loadGlobalEmotes()
 
     qCDebug(chatterinoSeventv) << "Loading 7TV Global Emotes";
 
-    getSeventvAPI().getEmoteSet(
+    getIApp()->getSeventvAPI()->getEmoteSet(
         u"global"_s,
         [this](const auto &json) {
             QJsonArray parsedEmotes = json["emotes"].toArray();
@@ -243,7 +244,7 @@ void SeventvEmotes::loadChannelEmotes(
     qCDebug(chatterinoSeventv)
         << "Reloading 7TV Channel Emotes" << channelId << manualRefresh;
 
-    getSeventvAPI().getUserByTwitchID(
+    getIApp()->getSeventvAPI()->getUserByTwitchID(
         channelId,
         [callback = std::move(callback), channel, channelId,
          manualRefresh](const auto &json) {
@@ -405,7 +406,7 @@ void SeventvEmotes::getEmoteSet(
 {
     qCDebug(chatterinoSeventv) << "Loading 7TV Emote Set" << emoteSetId;
 
-    getSeventvAPI().getEmoteSet(
+    getIApp()->getSeventvAPI()->getEmoteSet(
         emoteSetId,
         [callback = std::move(successCallback), emoteSetId](const auto &json) {
             auto parsedEmotes = json["emotes"].toArray();

src/providers/twitch/TwitchAccount.cpp

@@ -491,7 +491,15 @@ void TwitchAccount::loadSeventvUserID()
         return;
     }
 
-    getSeventvAPI().getUserByTwitchID(
+    auto *seventv = getIApp()->getSeventvAPI();
+    if (!seventv)
+    {
+        qCWarning(chatterinoSeventv)
+            << "Not loading 7TV User ID because the 7TV API is not initialized";
+        return;
+    }
+
+    seventv->getUserByTwitchID(
         this->getUserId(),
         [this](const auto &json) {
             const auto id = json["user"]["id"].toString();

src/providers/twitch/TwitchChannel.cpp

@@ -1646,7 +1646,7 @@ void TwitchChannel::updateSevenTVActivity()
 
     qCDebug(chatterinoSeventv) << "Sending activity in" << this->getName();
 
-    getSeventvAPI().updatePresence(
+    getIApp()->getSeventvAPI()->updatePresence(
         this->roomId(), currentSeventvUserID,
         [chan = weakOf<Channel>(this)]() {
             const auto self =

src/singletons/Settings.cpp

@@ -137,6 +137,7 @@ bool Settings::toggleMutedChannel(const QString &channelName)
 Settings *Settings::instance_ = nullptr;
 
 Settings::Settings(const QString &settingsDirectory)
+    : prevInstance_(Settings::instance_)
 {
     QString settingsPath = settingsDirectory + "/settings.json";
 
@@ -188,7 +189,10 @@ Settings::Settings(const QString &settingsDirectory)
         false);
 }
 
-Settings::~Settings() = default;
+Settings::~Settings()
+{
+    Settings::instance_ = this->prevInstance_;
+}
 
 void Settings::saveSnapshot()
 {

src/singletons/Settings.hpp

@@ -66,6 +66,7 @@ enum UsernameRightClickBehavior : int {
 class Settings
 {
     static Settings *instance_;
+    Settings *prevInstance_ = nullptr;
 
 public:
     Settings(const QString &settingsDirectory);