From d40b0a6c1da6c837319133ef059a241ecf1e1b05 Mon Sep 17 00:00:00 2001 From: iProdigy <8106344+iProdigy@users.noreply.github.com> Date: Wed, 8 Nov 2023 09:14:48 -0800 Subject: [PATCH] fix: avoid reward redemption crash via buffer refactor (#4949) Co-authored-by: nerix Co-authored-by: Rasmus Karlsson --- CHANGELOG.md | 1 + src/providers/twitch/IrcMessageHandler.cpp | 30 +++++----------- src/providers/twitch/IrcMessageHandler.hpp | 8 ++--- src/providers/twitch/TwitchChannel.cpp | 42 ++++++++++++++-------- src/providers/twitch/TwitchChannel.hpp | 22 ++++++++++-- 5 files changed, 60 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 63a699619..1673fb09e 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/src/providers/twitch/IrcMessageHandler.cpp b/src/providers/twitch/IrcMessageHandler.cpp index 10df44359..2be9d9621 100644 --- a/src/providers/twitch/IrcMessageHandler.cpp +++ b/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"); diff --git a/src/providers/twitch/IrcMessageHandler.hpp b/src/providers/twitch/IrcMessageHandler.hpp index 44773b8f3..26c21f6da 100644 --- a/src/providers/twitch/IrcMessageHandler.hpp +++ b/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 &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 diff --git a/src/providers/twitch/TwitchChannel.cpp b/src/providers/twitch/TwitchChannel.cpp index 6dc19510b..bf5762574 100644 --- a/src/providers/twitch/TwitchChannel.cpp +++ b/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()); } } diff --git a/src/providers/twitch/TwitchChannel.hpp b/src/providers/twitch/TwitchChannel.hpp index 28b8c5d23..d3636c9c9 100644 --- a/src/providers/twitch/TwitchChannel.hpp +++ b/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 #include +#include #include #include #include @@ -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 - 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( @@ -246,6 +256,12 @@ private: QString actualDisplayName; } nameOptions; + struct QueuedRedemption { + QString rewardID; + QString originalContent; + QObjectPtr message; + }; + void refreshPubSub(); void refreshChatters(); void refreshBadges(); @@ -356,6 +372,8 @@ private: badgeSets_; // "subscribers": { "0": ... "3": ... "6": ... UniqueAccess> cheerEmoteSets_; UniqueAccess> channelPointRewards_; + boost::circular_buffer_space_optimized + waitingRedemptions_{MAX_QUEUED_REDEMPTIONS}; bool mod_ = false; bool vip_ = false;