fix: avoid reward redemption crash via buffer refactor (#4949)

Co-authored-by: nerix <nero.9@hotmail.de>
Co-authored-by: Rasmus Karlsson <rasmus.karlsson@pajlada.com>

CHANGELOG.md

@@ -32,6 +32,7 @@
 - Bugfix: Fixed tooltips appearing too large and/or away from the cursor. (#4920)
 - Bugfix: Fixed a crash when clicking `More messages below` button in a usercard and closing it quickly. (#4933)
 - Bugfix: Fixed thread popup window missing messages for nested threads. (#4923)
+- Bugfix: Fixed an occasional crash for channel point redemptions with text input. (#4949)
 - Dev: Change clang-format from v14 to v16. (#4929)
 - Dev: Fixed UTF16 encoding of `modes` file for the installer. (#4791)
 - Dev: Temporarily disable High DPI scaling on Qt6 builds on Windows. (#4767)

src/providers/twitch/IrcMessageHandler.cpp

@@ -1,6 +1,7 @@
 #include "providers/twitch/IrcMessageHandler.hpp"
 
 #include "Application.hpp"
+#include "common/Common.hpp"
 #include "common/Literals.hpp"
 #include "common/QLogging.hpp"
 #include "controllers/accounts/AccountController.hpp"
@@ -691,7 +692,7 @@ void IrcMessageHandler::handlePrivMessage(Communi::IrcPrivateMessage *message,
     // https://mm2pl.github.io/emoji_rfc.pdf for more details
 
     this->addMessage(
-        message, message->target(),
+        message, channelOrEmptyByTarget(message->target(), server),
         message->content().replace(COMBINED_FIXER, ZERO_WIDTH_JOINER), server,
         false, message->isAction());
 
@@ -1001,7 +1002,7 @@ void IrcMessageHandler::handleUserNoticeMessage(Communi::IrcMessage *message,
         // Messages are not required, so they might be empty
         if (!content.isEmpty())
         {
-            this->addMessage(message, target, content, server, true, false);
+            this->addMessage(message, chn, content, server, true, false);
         }
     }
 
@@ -1260,13 +1261,11 @@ void IrcMessageHandler::setSimilarityFlags(const MessagePtr &message,
 }
 
 void IrcMessageHandler::addMessage(Communi::IrcMessage *message,
-                                   const QString &target,
+                                   const ChannelPtr &chan,
                                    const QString &originalContent,
                                    TwitchIrcServer &server, bool isSub,
                                    bool isAction)
 {
-    auto chan = channelOrEmptyByTarget(target, server);
-
     if (chan->isEmpty())
     {
         return;
@@ -1290,27 +1289,14 @@ void IrcMessageHandler::addMessage(Communi::IrcMessage *message,
     if (const auto it = tags.find("custom-reward-id"); it != tags.end())
     {
         const auto rewardId = it.value().toString();
-        if (!channel->isChannelPointRewardKnown(rewardId))
+        if (!rewardId.isEmpty() &&
+            !channel->isChannelPointRewardKnown(rewardId))
         {
             // Need to wait for pubsub reward notification
-            auto *clone = message->clone();
             qCDebug(chatterinoTwitch) << "TwitchChannel reward added ADD "
                                          "callback since reward is not known:"
                                       << rewardId;
-            channel->channelPointRewardAdded.connect(
-                [=, this, &server](ChannelPointReward reward) {
-                    qCDebug(chatterinoTwitch)
-                        << "TwitchChannel reward added callback:" << reward.id
-                        << "-" << rewardId;
-                    if (reward.id == rewardId)
-                    {
-                        this->addMessage(clone, target, originalContent, server,
-                                         isSub, isAction);
-                        clone->deleteLater();
-                        return true;
-                    }
-                    return false;
-                });
+            channel->addQueuedRedemption(rewardId, originalContent, message);
             return;
         }
         args.channelPointRewardId = rewardId;
@@ -1319,7 +1305,7 @@ void IrcMessageHandler::addMessage(Communi::IrcMessage *message,
     QString content = originalContent;
     int messageOffset = stripLeadingReplyMention(tags, content);
 
-    TwitchMessageBuilder builder(chan.get(), message, args, content, isAction);
+    TwitchMessageBuilder builder(channel, message, args, content, isAction);
     builder.setMessageOffset(messageOffset);
 
     if (const auto it = tags.find("reply-thread-parent-msg-id");

src/providers/twitch/IrcMessageHandler.hpp

@@ -55,15 +55,15 @@ public:
     void handleJoinMessage(Communi::IrcMessage *message);
     void handlePartMessage(Communi::IrcMessage *message);
 
+    void addMessage(Communi::IrcMessage *message, const ChannelPtr &chan,
+                    const QString &originalContent, TwitchIrcServer &server,
+                    bool isSub, bool isAction);
+
 private:
     static float similarity(const MessagePtr &msg,
                             const LimitedQueueSnapshot<MessagePtr> &messages);
     static void setSimilarityFlags(const MessagePtr &message,
                                    const ChannelPtr &channel);
-
-    void addMessage(Communi::IrcMessage *message, const QString &target,
-                    const QString &originalContent, TwitchIrcServer &server,
-                    bool isSub, bool isAction);
 };
 
 }  // namespace chatterino

src/providers/twitch/TwitchChannel.cpp

@@ -348,6 +348,17 @@ void TwitchChannel::refreshSevenTVChannelEmotes(bool manualRefresh)
         manualRefresh);
 }
 
+void TwitchChannel::addQueuedRedemption(const QString &rewardId,
+                                        const QString &originalContent,
+                                        Communi::IrcMessage *message)
+{
+    this->waitingRedemptions_.push_back({
+        rewardId,
+        originalContent,
+        {message->clone(), {}},
+    });
+}
+
 void TwitchChannel::addChannelPointReward(const ChannelPointReward &reward)
 {
     assertInGuiThread();
@@ -368,25 +379,26 @@ void TwitchChannel::addChannelPointReward(const ChannelPointReward &reward)
     }
     if (result)
     {
+        const auto &channelName = this->getName();
         qCDebug(chatterinoTwitch)
-            << "[TwitchChannel" << this->getName()
+            << "[TwitchChannel" << channelName
             << "] Channel point reward added:" << reward.id << ","
             << reward.title << "," << reward.isUserInputRequired;
 
-        // TODO: There's an underlying bug here. This bug should be fixed.
-        // This only attempts to prevent a crash when invoking the signal.
-        try
-        {
-            this->channelPointRewardAdded.invoke(reward);
-        }
-        catch (const std::bad_function_call &)
-        {
-            qCWarning(chatterinoTwitch).nospace()
-                << "[TwitchChannel " << this->getName()
-                << "] Caught std::bad_function_call when adding channel point "
-                   "reward ChannelPointReward{ id: "
-                << reward.id << ", title: " << reward.title << " }.";
-        }
+        auto *server = getApp()->twitch;
+        auto it = std::remove_if(
+            this->waitingRedemptions_.begin(), this->waitingRedemptions_.end(),
+            [&](const QueuedRedemption &msg) {
+                if (reward.id == msg.rewardID)
+                {
+                    IrcMessageHandler::instance().addMessage(
+                        msg.message.get(), shared_from_this(),
+                        msg.originalContent, *server, false, false);
+                    return true;
+                }
+                return false;
+            });
+        this->waitingRedemptions_.erase(it, this->waitingRedemptions_.end());
     }
 }
 

src/providers/twitch/TwitchChannel.hpp

@@ -4,12 +4,15 @@
 #include "common/Atomic.hpp"
 #include "common/Channel.hpp"
 #include "common/ChannelChatters.hpp"
+#include "common/Common.hpp"
 #include "common/Outcome.hpp"
 #include "common/UniqueAccess.hpp"
 #include "providers/twitch/TwitchEmotes.hpp"
 #include "util/QStringHash.hpp"
 
+#include <boost/circular_buffer/space_optimized.hpp>
 #include <boost/signals2.hpp>
+#include <IrcMessage>
 #include <pajlada/signals/signalholder.hpp>
 #include <QColor>
 #include <QElapsedTimer>
@@ -67,6 +70,8 @@ struct HelixStream;
 
 class TwitchIrcServer;
 
+const int MAX_QUEUED_REDEMPTIONS = 16;
+
 class TwitchChannel final : public Channel, public ChannelChatters
 {
 public:
@@ -218,8 +223,13 @@ public:
     pajlada::Signals::NoArgSignal roomModesChanged;
 
     // Channel point rewards
-    pajlada::Signals::SelfDisconnectingSignal<ChannelPointReward>
-        channelPointRewardAdded;
+    void addQueuedRedemption(const QString &rewardId,
+                             const QString &originalContent,
+                             Communi::IrcMessage *message);
+    /**
+     * A rich & hydrated redemption from PubSub has arrived, add it to the channel.
+     * This will look at queued up partial messages, and if one is found it will add the queued up partial messages fully hydrated.
+     **/
     void addChannelPointReward(const ChannelPointReward &reward);
     bool isChannelPointRewardKnown(const QString &rewardId);
     std::optional<ChannelPointReward> channelPointReward(
@@ -246,6 +256,12 @@ private:
         QString actualDisplayName;
     } nameOptions;
 
+    struct QueuedRedemption {
+        QString rewardID;
+        QString originalContent;
+        QObjectPtr<Communi::IrcMessage> message;
+    };
+
     void refreshPubSub();
     void refreshChatters();
     void refreshBadges();
@@ -356,6 +372,8 @@ private:
         badgeSets_;  // "subscribers": { "0": ... "3": ... "6": ...
     UniqueAccess<std::vector<CheerEmoteSet>> cheerEmoteSets_;
     UniqueAccess<std::map<QString, ChannelPointReward>> channelPointRewards_;
+    boost::circular_buffer_space_optimized<QueuedRedemption>
+        waitingRedemptions_{MAX_QUEUED_REDEMPTIONS};
 
     bool mod_ = false;
     bool vip_ = false;