From a691bb4c51acc0de7c4ef0b03fc29eee953c639b Mon Sep 17 00:00:00 2001 From: pajlada Date: Fri, 18 Jul 2025 22:07:22 +0200 Subject: [PATCH] fix: don't detach threads (#6333) Previously, we would detach threads that did not exit in a reasonable time, meaning they'd continue running and potentially accessing data that had been freed, causing us to crash. With this change, we _dont_ detach the thread, and let the thread destructor do as it pleases (which will be terminating). Neither solution is clean - old solution would sometimes work, but this should give us a better indicator of where we're doing things wrong. --- CHANGELOG.md | 1 + src/controllers/sound/MiniaudioBackend.cpp | 1 - src/providers/liveupdates/BasicPubSubManager.hpp | 4 +--- src/providers/twitch/PubSubManager.cpp | 5 +---- 4 files changed, 3 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d63db450..f07fe826 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,7 @@ - Dev: Implemented customizable display names for enums. (#6238) - Dev: Refactored event API initialization away from Application and into TwitchIrcServer. (#6198) - Dev: Updated GoogleTest to v1.17.0. (#6180) +- Dev: Don't detach threads. (#6333) - Dev: Mini refactor of `TwitchAccount`. (#6182) - Dev: Refactored away some `getApp` usages in `WindowManager`. (#6194) - Dev: Simplified string literals to be a re-export of Qt functions. (#6175) diff --git a/src/controllers/sound/MiniaudioBackend.cpp b/src/controllers/sound/MiniaudioBackend.cpp index 673a4eeb..b7a672a8 100644 --- a/src/controllers/sound/MiniaudioBackend.cpp +++ b/src/controllers/sound/MiniaudioBackend.cpp @@ -235,7 +235,6 @@ MiniaudioBackend::~MiniaudioBackend() qCWarning(chatterinoSound) << "Audio thread did not stop within 1 second"; - this->audioThread->detach(); } } diff --git a/src/providers/liveupdates/BasicPubSubManager.hpp b/src/providers/liveupdates/BasicPubSubManager.hpp index 401e6faf..7c3ca147 100644 --- a/src/providers/liveupdates/BasicPubSubManager.hpp +++ b/src/providers/liveupdates/BasicPubSubManager.hpp @@ -175,9 +175,7 @@ public: } qCWarning(chatterinoLiveupdates) - << "Thread didn't finish after stopping, discard it"; - // detach the thread so the destructor doesn't attempt any joining - this->mainThread_->detach(); + << "Thread didn't finish after stopping"; } protected: diff --git a/src/providers/twitch/PubSubManager.cpp b/src/providers/twitch/PubSubManager.cpp index 99d9d3a1..5eb4d782 100644 --- a/src/providers/twitch/PubSubManager.cpp +++ b/src/providers/twitch/PubSubManager.cpp @@ -143,10 +143,7 @@ void PubSub::stop() return; } - qCWarning(chatterinoLiveupdates) - << "Thread didn't finish after stopping, discard it"; - // detach the thread so the destructor doesn't attempt any joining - this->thread->detach(); + qCWarning(chatterinoLiveupdates) << "Thread didn't finish after stopping"; } void PubSub::listenToChannelPointRewards(const QString &channelID)