fix: resolve PulseAudio/WirePlumber deadlock and freeze issues
- Fix AudioBackend destructor: properly lock the PA mainloop before disconnecting the context to prevent race conditions with PA callbacks - Fix context leak on reconnect: call pa_context_unref() when the old context is replaced after PA_CONTEXT_FAILED to avoid resource leaks - Fix PA mainloop killed on reconnect (critical): PA_CONTEXT_TERMINATED was unconditionally calling quit() on the mainloop, even during reconnection when the old context fires TERMINATED after the new one was created. This was killing the new context and preventing successful reconnection, causing Waybar to appear frozen. The fix only quits the mainloop when the terminating context is still the active one. - Fix Wireplumber use-after-free: explicitly disconnect GObject signal handlers for mixer_api_, def_nodes_api_, and om_ before clearing the object references in the destructor to prevent callbacks from firing with a destroyed self pointer. - Fix GVariant memory leak in Wireplumber::handleScroll: unref the GVariant created for the set-volume signal after the emit call. Co-authored-by: Alexays <13947260+Alexays@users.noreply.github.com>
This commit is contained in:
@@ -54,6 +54,15 @@ waybar::modules::Wireplumber::Wireplumber(const std::string& id, const Json::Val
|
||||
|
||||
waybar::modules::Wireplumber::~Wireplumber() {
|
||||
waybar::modules::Wireplumber::modules.remove(this);
|
||||
if (mixer_api_ != nullptr) {
|
||||
g_signal_handlers_disconnect_by_data(mixer_api_, this);
|
||||
}
|
||||
if (def_nodes_api_ != nullptr) {
|
||||
g_signal_handlers_disconnect_by_data(def_nodes_api_, this);
|
||||
}
|
||||
if (om_ != nullptr) {
|
||||
g_signal_handlers_disconnect_by_data(om_, this);
|
||||
}
|
||||
wp_core_disconnect(wp_core_);
|
||||
g_clear_pointer(&apis_, g_ptr_array_unref);
|
||||
g_clear_object(&om_);
|
||||
@@ -528,6 +537,7 @@ bool waybar::modules::Wireplumber::handleScroll(GdkEventScroll* e) {
|
||||
GVariant* variant = g_variant_new_double(newVol);
|
||||
gboolean ret;
|
||||
g_signal_emit_by_name(mixer_api_, "set-volume", node_id_, variant, &ret);
|
||||
g_variant_unref(variant);
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -40,12 +40,16 @@ AudioBackend::AudioBackend(std::function<void()> on_updated_cb, private_construc
|
||||
}
|
||||
|
||||
AudioBackend::~AudioBackend() {
|
||||
if (context_ != nullptr) {
|
||||
pa_context_disconnect(context_);
|
||||
}
|
||||
|
||||
if (mainloop_ != nullptr) {
|
||||
mainloop_api_->quit(mainloop_api_, 0);
|
||||
// Lock the mainloop so we can safely disconnect the context.
|
||||
// This must be done before stopping the thread.
|
||||
pa_threaded_mainloop_lock(mainloop_);
|
||||
if (context_ != nullptr) {
|
||||
pa_context_disconnect(context_);
|
||||
pa_context_unref(context_);
|
||||
context_ = nullptr;
|
||||
}
|
||||
pa_threaded_mainloop_unlock(mainloop_);
|
||||
pa_threaded_mainloop_stop(mainloop_);
|
||||
pa_threaded_mainloop_free(mainloop_);
|
||||
}
|
||||
@@ -73,7 +77,14 @@ void AudioBackend::contextStateCb(pa_context* c, void* data) {
|
||||
auto* backend = static_cast<AudioBackend*>(data);
|
||||
switch (pa_context_get_state(c)) {
|
||||
case PA_CONTEXT_TERMINATED:
|
||||
backend->mainloop_api_->quit(backend->mainloop_api_, 0);
|
||||
// Only quit the mainloop if this is still the active context.
|
||||
// During reconnection, the old context fires TERMINATED after the new one
|
||||
// has already been created; quitting in that case would kill the new context.
|
||||
// Note: context_ is only written from PA callbacks (while the mainloop lock is
|
||||
// held), so this comparison is safe within any PA callback.
|
||||
if (backend->context_ == nullptr || backend->context_ == c) {
|
||||
backend->mainloop_api_->quit(backend->mainloop_api_, 0);
|
||||
}
|
||||
break;
|
||||
case PA_CONTEXT_READY:
|
||||
pa_context_get_server_info(c, serverInfoCb, data);
|
||||
@@ -93,6 +104,8 @@ void AudioBackend::contextStateCb(pa_context* c, void* data) {
|
||||
// So there is no need to lock it again.
|
||||
if (backend->context_ != nullptr) {
|
||||
pa_context_disconnect(backend->context_);
|
||||
pa_context_unref(backend->context_);
|
||||
backend->context_ = nullptr;
|
||||
}
|
||||
backend->connectContext();
|
||||
break;
|
||||
|
||||
Reference in New Issue
Block a user