Skip to content

Commit

Permalink
fix: ensure liveupdate pubsubs exit (#5557)
Browse files Browse the repository at this point in the history
Previously, the derived class (i.e. BttvLiveUpdates or SeventvEventAPI)
    would have their destructor ran before BasicPubSubManager called
    stop, meaning there was a time wherein messages could still flow
    through, attempting to call `onMessage` on a pure virtual, causing a
    crash.
  • Loading branch information
pajlada authored Aug 24, 2024
1 parent 175afa8 commit aa048b3
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
- Bugfix: Fixed windows on Windows not saving correctly when snapping them to the edges. (#5478)
- Bugfix: Fixed user info card popups adding duplicate line to log files. (#5499)
- Bugfix: Fixed tooltips and input completion popups not working after moving a split. (#5541)
- Bugfix: Fixed rare issue on shutdown where the client would hang. (#5557)
- Bugfix: Fixed `/clearmessages` not working with more than one window. (#5489)
- Bugfix: Fixed splits staying paused after unfocusing Chatterino in certain configurations. (#5504)
- Bugfix: Links with invalid characters in the domain are no longer detected. (#5509)
Expand Down
5 changes: 5 additions & 0 deletions src/providers/bttv/BttvLiveUpdates.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ BttvLiveUpdates::BttvLiveUpdates(QString host)
{
}

BttvLiveUpdates::~BttvLiveUpdates()
{
this->stop();
}

void BttvLiveUpdates::joinChannel(const QString &channelID,
const QString &userName)
{
Expand Down
1 change: 1 addition & 0 deletions src/providers/bttv/BttvLiveUpdates.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class BttvLiveUpdates : public BasicPubSubManager<BttvLiveUpdateSubscription>

public:
BttvLiveUpdates(QString host);
~BttvLiveUpdates() override;

struct {
Signal<BttvLiveUpdateEmoteUpdateAddMessage> emoteAdded;
Expand Down
24 changes: 20 additions & 4 deletions src/providers/liveupdates/BasicPubSubManager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ class BasicPubSubManager

virtual ~BasicPubSubManager()
{
this->stop();
// The derived class must call stop in its destructor
assert(this->stopping_);
}

BasicPubSubManager(const BasicPubSubManager &) = delete;
Expand Down Expand Up @@ -127,6 +128,11 @@ class BasicPubSubManager

void stop()
{
if (this->stopping_)
{
return;
}

this->stopping_ = true;

for (const auto &client : this->clients_)
Expand All @@ -138,10 +144,20 @@ class BasicPubSubManager

if (this->mainThread_->joinable())
{
this->mainThread_->join();
// NOTE: We spawn a new thread to join the websocket thread.
// There is a case where a new client was initiated but not added to the clients list.
// We just don't join the thread & let the operating system nuke the thread if joining fails
// within 1s.
auto joiner = std::async(std::launch::async, &std::thread::join,
this->mainThread_.get());
if (joiner.wait_for(std::chrono::seconds(1)) ==
std::future_status::timeout)
{
qCWarning(chatterinoLiveupdates)
<< "Thread didn't join within 1 second, rip it out";
this->websocketClient_.stop();
}
}

assert(this->clients_.empty());
}

protected:
Expand Down
5 changes: 5 additions & 0 deletions src/providers/seventv/SeventvEventAPI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ SeventvEventAPI::SeventvEventAPI(
{
}

SeventvEventAPI::~SeventvEventAPI()
{
this->stop();
}

void SeventvEventAPI::subscribeUser(const QString &userID,
const QString &emoteSetID)
{
Expand Down
2 changes: 2 additions & 0 deletions src/providers/seventv/SeventvEventAPI.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class SeventvEventAPI
std::chrono::milliseconds defaultHeartbeatInterval =
std::chrono::milliseconds(25000));

~SeventvEventAPI() override;

struct {
Signal<seventv::eventapi::EmoteAddDispatch> emoteAdded;
Signal<seventv::eventapi::EmoteUpdateDispatch> emoteUpdated;
Expand Down
2 changes: 0 additions & 2 deletions src/providers/twitch/PubSubManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -581,8 +581,6 @@ void PubSub::stop()
this->websocketClient.stop();
}
}

assert(this->clients.empty());
}

bool PubSub::listenToWhispers()
Expand Down

0 comments on commit aa048b3

Please sign in to comment.