From 36517a2c10d206bbde30f6a43e0002b3c3ce139f Mon Sep 17 00:00:00 2001 From: bbedward Date: Fri, 13 Feb 2026 17:54:43 -0500 Subject: [PATCH] services/pipewire: manage default objs using normal qt properties Fixes use after free bugs due to pointer mismatches in destructors. Drops SimpleObjectHandle. --- changelog/next.md | 1 + src/core/util.hpp | 31 -------- src/services/pipewire/defaults.cpp | 121 +++++++++++++++++++---------- src/services/pipewire/defaults.hpp | 5 +- 4 files changed, 83 insertions(+), 75 deletions(-) diff --git a/changelog/next.md b/changelog/next.md index 7180d53..b9000c2 100644 --- a/changelog/next.md +++ b/changelog/next.md @@ -48,6 +48,7 @@ set shell id. - Fixed memory leak in IPC handlers. - Fixed ClippingRectangle related crashes. - Fixed crashes when monitors are unplugged. +- Fixed crashes when default pipewire devices are lost. ## Packaging Changes diff --git a/src/core/util.hpp b/src/core/util.hpp index 3b86d28..bb8dd85 100644 --- a/src/core/util.hpp +++ b/src/core/util.hpp @@ -251,37 +251,6 @@ public: GuardedEmitBlocker block() { return GuardedEmitBlocker(&this->blocked); } }; -template -class SimpleObjectHandleOps { - using Traits = MemberPointerTraits; - -public: - static bool setObject(Traits::Class* parent, Traits::Type value) { - if (value == parent->*member) return false; - - if (parent->*member != nullptr) { - QObject::disconnect(parent->*member, &QObject::destroyed, parent, destroyedSlot); - } - - parent->*member = value; - - if (value != nullptr) { - QObject::connect(parent->*member, &QObject::destroyed, parent, destroyedSlot); - } - - if constexpr (changedSignal != nullptr) { - emit(parent->*changedSignal)(); - } - - return true; - } -}; - -template -bool setSimpleObjectHandle(auto* parent, auto* value) { - return SimpleObjectHandleOps::setObject(parent, value); -} - template class MethodFunctor { using PtrMeta = MemberPointerTraits; diff --git a/src/services/pipewire/defaults.cpp b/src/services/pipewire/defaults.cpp index 02463f4..7a24a65 100644 --- a/src/services/pipewire/defaults.cpp +++ b/src/services/pipewire/defaults.cpp @@ -12,7 +12,6 @@ #include #include "../../core/logcat.hpp" -#include "../../core/util.hpp" #include "metadata.hpp" #include "node.hpp" #include "registry.hpp" @@ -138,32 +137,6 @@ void PwDefaultTracker::onNodeAdded(PwNode* node) { } } -void PwDefaultTracker::onNodeDestroyed(QObject* node) { - if (node == this->mDefaultSink) { - qCInfo(logDefaults) << "Default sink destroyed."; - this->mDefaultSink = nullptr; - emit this->defaultSinkChanged(); - } - - if (node == this->mDefaultSource) { - qCInfo(logDefaults) << "Default source destroyed."; - this->mDefaultSource = nullptr; - emit this->defaultSourceChanged(); - } - - if (node == this->mDefaultConfiguredSink) { - qCInfo(logDefaults) << "Default configured sink destroyed."; - this->mDefaultConfiguredSink = nullptr; - emit this->defaultConfiguredSinkChanged(); - } - - if (node == this->mDefaultConfiguredSource) { - qCInfo(logDefaults) << "Default configured source destroyed."; - this->mDefaultConfiguredSource = nullptr; - emit this->defaultConfiguredSourceChanged(); - } -} - void PwDefaultTracker::changeConfiguredSink(PwNode* node) { if (node != nullptr) { if (!node->type.testFlags(PwNodeType::AudioSink)) { @@ -240,10 +213,23 @@ void PwDefaultTracker::setDefaultSink(PwNode* node) { if (node == this->mDefaultSink) return; qCInfo(logDefaults) << "Default sink changed to" << node; - setSimpleObjectHandle< - &PwDefaultTracker::mDefaultSink, - &PwDefaultTracker::onNodeDestroyed, - &PwDefaultTracker::defaultSinkChanged>(this, node); + if (this->mDefaultSink != nullptr) { + QObject::disconnect(this->mDefaultSink, nullptr, this, nullptr); + } + + this->mDefaultSink = node; + + if (node != nullptr) { + QObject::connect(node, &QObject::destroyed, this, &PwDefaultTracker::onDefaultSinkDestroyed); + } + + emit this->defaultSinkChanged(); +} + +void PwDefaultTracker::onDefaultSinkDestroyed() { + qCInfo(logDefaults) << "Default sink destroyed."; + this->mDefaultSink = nullptr; + emit this->defaultSinkChanged(); } void PwDefaultTracker::setDefaultSinkName(const QString& name) { @@ -257,10 +243,23 @@ void PwDefaultTracker::setDefaultSource(PwNode* node) { if (node == this->mDefaultSource) return; qCInfo(logDefaults) << "Default source changed to" << node; - setSimpleObjectHandle< - &PwDefaultTracker::mDefaultSource, - &PwDefaultTracker::onNodeDestroyed, - &PwDefaultTracker::defaultSourceChanged>(this, node); + if (this->mDefaultSource != nullptr) { + QObject::disconnect(this->mDefaultSource, nullptr, this, nullptr); + } + + this->mDefaultSource = node; + + if (node != nullptr) { + QObject::connect(node, &QObject::destroyed, this, &PwDefaultTracker::onDefaultSourceDestroyed); + } + + emit this->defaultSourceChanged(); +} + +void PwDefaultTracker::onDefaultSourceDestroyed() { + qCInfo(logDefaults) << "Default source destroyed."; + this->mDefaultSource = nullptr; + emit this->defaultSourceChanged(); } void PwDefaultTracker::setDefaultSourceName(const QString& name) { @@ -274,10 +273,28 @@ void PwDefaultTracker::setDefaultConfiguredSink(PwNode* node) { if (node == this->mDefaultConfiguredSink) return; qCInfo(logDefaults) << "Default configured sink changed to" << node; - setSimpleObjectHandle< - &PwDefaultTracker::mDefaultConfiguredSink, - &PwDefaultTracker::onNodeDestroyed, - &PwDefaultTracker::defaultConfiguredSinkChanged>(this, node); + if (this->mDefaultConfiguredSink != nullptr) { + QObject::disconnect(this->mDefaultConfiguredSink, nullptr, this, nullptr); + } + + this->mDefaultConfiguredSink = node; + + if (node != nullptr) { + QObject::connect( + node, + &QObject::destroyed, + this, + &PwDefaultTracker::onDefaultConfiguredSinkDestroyed + ); + } + + emit this->defaultConfiguredSinkChanged(); +} + +void PwDefaultTracker::onDefaultConfiguredSinkDestroyed() { + qCInfo(logDefaults) << "Default configured sink destroyed."; + this->mDefaultConfiguredSink = nullptr; + emit this->defaultConfiguredSinkChanged(); } void PwDefaultTracker::setDefaultConfiguredSinkName(const QString& name) { @@ -291,10 +308,28 @@ void PwDefaultTracker::setDefaultConfiguredSource(PwNode* node) { if (node == this->mDefaultConfiguredSource) return; qCInfo(logDefaults) << "Default configured source changed to" << node; - setSimpleObjectHandle< - &PwDefaultTracker::mDefaultConfiguredSource, - &PwDefaultTracker::onNodeDestroyed, - &PwDefaultTracker::defaultConfiguredSourceChanged>(this, node); + if (this->mDefaultConfiguredSource != nullptr) { + QObject::disconnect(this->mDefaultConfiguredSource, nullptr, this, nullptr); + } + + this->mDefaultConfiguredSource = node; + + if (node != nullptr) { + QObject::connect( + node, + &QObject::destroyed, + this, + &PwDefaultTracker::onDefaultConfiguredSourceDestroyed + ); + } + + emit this->defaultConfiguredSourceChanged(); +} + +void PwDefaultTracker::onDefaultConfiguredSourceDestroyed() { + qCInfo(logDefaults) << "Default configured source destroyed."; + this->mDefaultConfiguredSource = nullptr; + emit this->defaultConfiguredSourceChanged(); } void PwDefaultTracker::setDefaultConfiguredSourceName(const QString& name) { diff --git a/src/services/pipewire/defaults.hpp b/src/services/pipewire/defaults.hpp index 591c4fd..f31669e 100644 --- a/src/services/pipewire/defaults.hpp +++ b/src/services/pipewire/defaults.hpp @@ -44,7 +44,10 @@ private slots: void onMetadataAdded(PwMetadata* metadata); void onMetadataProperty(const char* key, const char* type, const char* value); void onNodeAdded(PwNode* node); - void onNodeDestroyed(QObject* node); + void onDefaultSinkDestroyed(); + void onDefaultSourceDestroyed(); + void onDefaultConfiguredSinkDestroyed(); + void onDefaultConfiguredSourceDestroyed(); private: void setDefaultSink(PwNode* node);