fix: avoid crash from channelPointRewardAdded callback

[iProdigy]

Closes #2432
Closes #3942

Problem

It is possible for channelPointRewardAdded connections to be created that have a long lifetime (take this as a given, I explain how this happens in the "Follow-up" section). Upon callback lambda invocation, the captured variables will be released. However, if the invocation is unrelated to the desired rewardId, releasing the captured variables will cause a seg fault when a matching invocation occurs later on. (Technically this matching invocation will take the form of empty reward ids because of the earlier variable destruction)

Solution

Clone objects used in lambda scope to avoid early destruction (also ensure reward is properly copied for each signal connection)

Follow-up

While this PR avoids a common crash, there are remaining edge cases to be tackled in future PRs.

1. It is possible that channelPointRewardAdded is invoked before a signal connection is made in IrcMessageHandler (when isChannelPointRewardKnown yields false). This is the scenario I described before. When this happens, now the first redemption associated with the reward will never be posted to chat (and the cloned objects are not freed). To solve this, there should be a mutex that prevents channelPointRewardAdded.invoke when we are in the !channel->isChannelPointRewardKnown(rewardId) block (and after acquiring the mutex, we should check isChannelPointRewardKnown once more) Scratch this, I assumed multiple threads were at play

2. It is possible that the pubsub socket is not connected when the text redemption occurs. As a result, a signal connection will be made, but invoke would not be called. Again, the cloned objects would not be freed (until the next redemption for the same reward occurs, while pubsub is connected). So, we ought to implement a timeout where if x seconds pass without signal invocation, we release the message regardless.

[iProdigy]

I just received chatterino.twitch: TwitchChannel reward added ADD callback since reward is not known: "" (not sure which channel), so bad payloads from twitch are not out of the question (in contributing to the edge cases that trigger the crashes)

[Nerixyz]

These don't need to be heap allocated, as they're effectively on the heap already (in the case of Qt5, you'll have three layers of indirection) and copied into the lambda.

[iProdigy]

Are you sure? Here's some logs from a recent crash I captured:

chatterino.twitch: [TwitchChannel "removed" ] Channel point reward added: "b37475ab-5bb3-4787-bdf9-d8c3582b62f5" , "Ask me anything" , true
chatterino.twitch: TwitchChannel reward added callback: "b37475ab-5bb3-4787-bdf9-d8c3582b62f5" - ""
chatterino.twitch: TwitchChannel reward added callback: "" -
Segmentation fault (core dumped)

Notice that rewardId has become an empty QString (and seemingly isn't even properly printed in the last callback log)

[Nerixyz]

Maybe I'm reading this wrong, but I think the use of SelfDisconnectingSignal is wrong here, since it uses std::remove_if which says that calling the callback shouldn't modify the value, which we kind-of do here by deleting the clone in the callback.

We might want to use a QPointer for the message clone, to avoid use-after-free there.

[iProdigy]

Working on full refactor à la #4943 (review)