From 0b88b4ef0c5c7e1e04ebd43846baf3a593cafa83 Mon Sep 17 00:00:00 2001 From: Alex Date: Sat, 4 Jul 2026 13:41:29 +0200 Subject: [PATCH 1/3] fix(wireplumber): reconnect when PipeWire/WirePlumber restarts Previously the wireplumber module connected to PipeWire once in its constructor and had no handling for the connection being lost. When PipeWire or the wireplumber service restarted (or crashed), the module went stale/blank and never recovered until Waybar itself was restarted. Connect to the WpCore "disconnected" signal and, on disconnect, schedule a bounded main-loop retry (Glib::signal_timeout) that tears down the now invalid core/object-manager/mixer-api references and rebuilds the whole connection from scratch, re-running the async API and object-manager setup. Connection setup/teardown is factored into setupConnection() and teardownConnection() so startup and reconnect share one code path. The reconnect timer is cancelled in the destructor and the existing isModuleAlive() registry guard still protects in-flight async callbacks, so teardown during a pending reconnect stays safe. Fixes #2882. --- include/modules/wireplumber.hpp | 8 +++ src/modules/wireplumber.cpp | 92 +++++++++++++++++++++++++++++---- 2 files changed, 91 insertions(+), 9 deletions(-) diff --git a/include/modules/wireplumber.hpp b/include/modules/wireplumber.hpp index f1650e92..d32fbeec 100644 --- a/include/modules/wireplumber.hpp +++ b/include/modules/wireplumber.hpp @@ -17,6 +17,11 @@ class Wireplumber : public ALabel { auto update() -> void override; private: + bool setupConnection(); + void teardownConnection(); + void scheduleReconnect(); + bool onReconnectTimeout(); + static void onCoreDisconnected(waybar::modules::Wireplumber* self); void asyncLoadRequiredApiModules(); void prepare(waybar::modules::Wireplumber* self); void activatePlugins(); @@ -66,6 +71,9 @@ class Wireplumber : public ALabel { bool only_physical_; bool resolved_physical_; std::string form_factor_; + // Timer used to retry connecting to PipeWire after it goes away; disconnected in the destructor + // so a pending attempt can't outlive the module. See #2882. + sigc::connection reconnect_timer_; }; } // namespace waybar::modules diff --git a/src/modules/wireplumber.cpp b/src/modules/wireplumber.cpp index a2391071..11c2c60e 100644 --- a/src/modules/wireplumber.cpp +++ b/src/modules/wireplumber.cpp @@ -10,6 +10,11 @@ bool isValidNodeId(uint32_t id) { return id > 0 && id < G_MAXUINT32; } std::list waybar::modules::Wireplumber::modules; +// Interval between reconnect attempts after PipeWire/WirePlumber goes away. Fixed (rather than +// growing) so the module recovers promptly whenever the service comes back, while still being +// a bounded, main-loop-friendly poll rather than a busy loop. +static constexpr unsigned kReconnectIntervalMs = 2000; + // Async load/activation callbacks (onDefaultNodesApiLoaded, onMixerApiLoaded, onPluginActivated) // are handed a raw `self` pointer with no GCancellable, and WirePlumber has no way to withdraw an // in-flight callback. If the module is destroyed before such a callback fires (e.g. an output/bar @@ -46,21 +51,37 @@ waybar::modules::Wireplumber::Wireplumber(const std::string& id, const Json::Val waybar::modules::Wireplumber::modules.push_back(this); wp_init(WP_INIT_PIPEWIRE); - wp_core_ = wp_core_new(nullptr, nullptr, nullptr); - apis_ = g_ptr_array_new_with_free_func(g_object_unref); - om_ = wp_object_manager_new(); type_ = g_strdup(config_["node-type"].isString() ? config_["node-type"].asString().c_str() : "Audio/Sink"); only_physical_ = config_["only-physical"].isBool() ? config_["only-physical"].asBool() : false; + if (!setupConnection()) { + spdlog::error("[{}]: Could not connect to PipeWire: '{}'", name_, type_); + throw std::runtime_error("Could not connect to PipeWire\n"); + } +} + +// Creates a fresh WpCore/object manager, connects to PipeWire and kicks off async API loading. +// Used both at startup and when reconnecting after a PipeWire/WirePlumber restart, so all +// connection-scoped state is (re)built here. Returns false if the connection could not be +// initiated. See https://github.com/Alexays/Waybar/issues/2882. +bool waybar::modules::Wireplumber::setupConnection() { + wp_core_ = wp_core_new(nullptr, nullptr, nullptr); + apis_ = g_ptr_array_new_with_free_func(g_object_unref); + om_ = wp_object_manager_new(); + pending_plugins_ = 0; + prepare(this); + // Recover when PipeWire/WirePlumber goes away (service restart, crash). The "disconnected" + // signal fires on the GTK main loop; from there we schedule a reconnect attempt. + g_signal_connect_swapped(wp_core_, "disconnected", (GCallback)onCoreDisconnected, this); + spdlog::debug("[{}]: connecting to pipewire: '{}'...", name_, type_); if (wp_core_connect(wp_core_) == 0) { - spdlog::error("[{}]: Could not connect to PipeWire: '{}'", name_, type_); - throw std::runtime_error("Could not connect to PipeWire\n"); + return false; } spdlog::debug("[{}]: {} connected!", name_, type_); @@ -68,10 +89,13 @@ waybar::modules::Wireplumber::Wireplumber(const std::string& id, const Json::Val g_signal_connect_swapped(om_, "installed", (GCallback)onObjectManagerInstalled, this); asyncLoadRequiredApiModules(); + return true; } -waybar::modules::Wireplumber::~Wireplumber() { - waybar::modules::Wireplumber::modules.remove(this); +// Disconnects signal handlers and releases all connection-scoped WirePlumber objects. Safe to call +// when already partially/fully torn down (every pointer is null-checked and cleared), so it doubles +// as the reconnect reset and the destructor's cleanup. +void waybar::modules::Wireplumber::teardownConnection() { if (mixer_api_ != nullptr) { g_signal_handlers_disconnect_by_data(mixer_api_, this); } @@ -81,12 +105,62 @@ waybar::modules::Wireplumber::~Wireplumber() { if (om_ != nullptr) { g_signal_handlers_disconnect_by_data(om_, this); } - wp_core_disconnect(wp_core_); + if (wp_core_ != nullptr) { + g_signal_handlers_disconnect_by_data(wp_core_, this); + wp_core_disconnect(wp_core_); + } g_clear_pointer(&apis_, g_ptr_array_unref); g_clear_object(&om_); - g_clear_object(&wp_core_); g_clear_object(&mixer_api_); g_clear_object(&def_nodes_api_); + g_clear_object(&wp_core_); + // onObjectManagerInstalled re-populates these via out-params (which don't free the previous + // value), so clear them here to avoid leaking the old strings across a reconnect. + g_clear_pointer(&default_node_name_, g_free); + g_clear_pointer(&default_source_name_, g_free); + pending_plugins_ = 0; +} + +// "disconnected" signal handler on wp_core_. Runs during the core's own signal emission, so it must +// not tear down the core here; it only schedules a reconnect, which performs the teardown/rebuild +// once control has returned to the main loop. +void waybar::modules::Wireplumber::onCoreDisconnected(waybar::modules::Wireplumber* self) { + if (!isModuleAlive(self)) { + return; + } + spdlog::warn("[{}]: PipeWire connection lost; will attempt to reconnect", self->name_); + self->scheduleReconnect(); +} + +void waybar::modules::Wireplumber::scheduleReconnect() { + if (reconnect_timer_.connected()) { + return; // a reconnect attempt is already pending + } + reconnect_timer_ = Glib::signal_timeout().connect( + sigc::mem_fun(*this, &Wireplumber::onReconnectTimeout), kReconnectIntervalMs); +} + +// Runs on the GTK main loop. Rebuilds the connection from scratch; returns true to keep retrying at +// the fixed interval until PipeWire is back, or false to stop once reconnected (a future +// "disconnected" signal will re-arm the timer if needed). +bool waybar::modules::Wireplumber::onReconnectTimeout() { + teardownConnection(); + spdlog::info("[{}]: attempting to reconnect to PipeWire...", name_); + if (setupConnection()) { + spdlog::info("[{}]: reconnected to PipeWire", name_); + return false; + } + teardownConnection(); + spdlog::debug("[{}]: reconnect failed; retrying in {} ms", name_, kReconnectIntervalMs); + return true; +} + +waybar::modules::Wireplumber::~Wireplumber() { + // Remove from the live-module registry first so any in-flight async callback bails out (#3974), + // then cancel a pending reconnect so onReconnectTimeout can't fire on a half-destroyed module. + waybar::modules::Wireplumber::modules.remove(this); + reconnect_timer_.disconnect(); + teardownConnection(); g_free(default_node_name_); g_free(default_source_name_); g_free(type_); From acc7060be6f78f36a30a9baeb147815efb80cdd8 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 5 Jul 2026 09:53:11 +0200 Subject: [PATCH 2/3] fix(wireplumber): guard async callbacks by connection generation; wire scroll once Addresses review on #5168: - Generational aliasing (blocking): setupConnection()/onReconnectTimeout() rebuild wp_core_/om_/pending_plugins_ in place on the same self, but the async load/activate callbacks carried no generation, and isModuleAlive() only proves self still exists. If PipeWire dropped again while a previous connection's async chain was still in flight, a stale completion would run against the rebuilt connection (a stray --pending_plugins_, an out-of-order install_object_manager), re-creating #2882's stale/blank state. Each async call now carries an AsyncCall{self, generation}; connection_generation_ is bumped in setupConnection(), and every callback drops out when its generation no longer matches (checked after isModuleAlive short-circuits). - Duplicate scroll handlers: onMixerApiLoaded re-runs on every reconnect and connected a new scroll handler each time (dead but accumulating). Moved the one-time wiring to the constructor; handleScroll no-ops while mixer_api_ is null, so wiring it before the first connect is safe. --- include/modules/wireplumber.hpp | 11 ++++--- src/modules/wireplumber.cpp | 52 ++++++++++++++++++++++++--------- 2 files changed, 46 insertions(+), 17 deletions(-) diff --git a/include/modules/wireplumber.hpp b/include/modules/wireplumber.hpp index d32fbeec..dc96b520 100644 --- a/include/modules/wireplumber.hpp +++ b/include/modules/wireplumber.hpp @@ -29,10 +29,9 @@ class Wireplumber : public ALabel { static void updateNodeName(waybar::modules::Wireplumber* self, uint32_t id); static void updateSourceVolume(waybar::modules::Wireplumber* self, uint32_t id); static void updateSourceName(waybar::modules::Wireplumber* self, uint32_t id); // NEW - static void onPluginActivated(WpObject* p, GAsyncResult* res, waybar::modules::Wireplumber* self); - static void onDefaultNodesApiLoaded(WpObject* p, GAsyncResult* res, - waybar::modules::Wireplumber* self); - static void onMixerApiLoaded(WpObject* p, GAsyncResult* res, waybar::modules::Wireplumber* self); + static void onPluginActivated(WpObject* p, GAsyncResult* res, gpointer data); + static void onDefaultNodesApiLoaded(WpObject* p, GAsyncResult* res, gpointer data); + static void onMixerApiLoaded(WpObject* p, GAsyncResult* res, gpointer data); static void onObjectManagerInstalled(waybar::modules::Wireplumber* self); static void onMixerChanged(waybar::modules::Wireplumber* self, uint32_t id); static void onDefaultNodesApiChanged(waybar::modules::Wireplumber* self); @@ -57,6 +56,10 @@ class Wireplumber : public ALabel { WpPlugin* def_nodes_api_; gchar* default_node_name_; uint32_t pending_plugins_; + // Bumped on every (re)connection. The async load/activate callbacks capture the generation they + // were scheduled under (via their user_data) and no-op if it no longer matches, so a completion + // from a connection that was already torn down cannot corrupt the new generation's state (#2882). + uint32_t connection_generation_{0}; bool muted_; double volume_; double min_step_; diff --git a/src/modules/wireplumber.cpp b/src/modules/wireplumber.cpp index 11c2c60e..8f9bd961 100644 --- a/src/modules/wireplumber.cpp +++ b/src/modules/wireplumber.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include @@ -26,6 +27,17 @@ bool waybar::modules::Wireplumber::isModuleAlive(waybar::modules::Wireplumber* s return std::find(modules.begin(), modules.end(), self) != modules.end(); } +namespace { +// user_data for the async load/activate callbacks. Pairs the module with the connection generation +// the call was scheduled under so a completion belonging to a torn-down connection can be dropped +// (see Wireplumber::connection_generation_ and #2882). Heap-allocated per call; the callback takes +// ownership and frees it. +struct AsyncCall { + waybar::modules::Wireplumber* self; + uint32_t generation; +}; +} // namespace + waybar::modules::Wireplumber::Wireplumber(const std::string& id, const Json::Value& config) : ALabel(config, "wireplumber", id, "{volume}%"), wp_core_(nullptr), @@ -56,6 +68,12 @@ waybar::modules::Wireplumber::Wireplumber(const std::string& id, const Json::Val : "Audio/Sink"); only_physical_ = config_["only-physical"].isBool() ? config_["only-physical"].asBool() : false; + // Wire the scroll handler once, here, rather than in onMixerApiLoaded: the latter now re-runs on + // every reconnect and would accumulate duplicate handlers. handleScroll no-ops while mixer_api_ + // is null, so wiring it before the first successful connect is safe. + event_box_.add_events(Gdk::SCROLL_MASK | Gdk::SMOOTH_SCROLL_MASK); + event_box_.signal_scroll_event().connect(sigc::mem_fun(*this, &Wireplumber::handleScroll)); + if (!setupConnection()) { spdlog::error("[{}]: Could not connect to PipeWire: '{}'", name_, type_); throw std::runtime_error("Could not connect to PipeWire\n"); @@ -67,6 +85,10 @@ waybar::modules::Wireplumber::Wireplumber(const std::string& id, const Json::Val // connection-scoped state is (re)built here. Returns false if the connection could not be // initiated. See https://github.com/Alexays/Waybar/issues/2882. bool waybar::modules::Wireplumber::setupConnection() { + // New connection generation: any async load/activate callback still in flight from a previous + // connection will see a mismatched generation and bail out instead of mutating this one's state. + ++connection_generation_; + wp_core_ = wp_core_new(nullptr, nullptr, nullptr); apis_ = g_ptr_array_new_with_free_func(g_object_unref); om_ = wp_object_manager_new(); @@ -471,8 +493,10 @@ void waybar::modules::Wireplumber::onObjectManagerInstalled(waybar::modules::Wir } void waybar::modules::Wireplumber::onPluginActivated(WpObject* p, GAsyncResult* res, - waybar::modules::Wireplumber* self) { - if (!isModuleAlive(self)) { + gpointer data) { + std::unique_ptr call(static_cast(data)); + auto* self = call->self; + if (!isModuleAlive(self) || call->generation != self->connection_generation_) { return; } @@ -496,7 +520,8 @@ void waybar::modules::Wireplumber::activatePlugins() { WpPlugin* plugin = static_cast(g_ptr_array_index(apis_, i)); pending_plugins_++; wp_object_activate(WP_OBJECT(plugin), WP_PLUGIN_FEATURE_ENABLED, nullptr, - (GAsyncReadyCallback)onPluginActivated, this); + (GAsyncReadyCallback)onPluginActivated, + new AsyncCall{this, connection_generation_}); } } @@ -520,8 +545,10 @@ void waybar::modules::Wireplumber::prepare(waybar::modules::Wireplumber* self) { } void waybar::modules::Wireplumber::onDefaultNodesApiLoaded(WpObject* p, GAsyncResult* res, - waybar::modules::Wireplumber* self) { - if (!isModuleAlive(self)) { + gpointer data) { + std::unique_ptr call(static_cast(data)); + auto* self = call->self; + if (!isModuleAlive(self) || call->generation != self->connection_generation_) { return; } @@ -541,12 +568,14 @@ void waybar::modules::Wireplumber::onDefaultNodesApiLoaded(WpObject* p, GAsyncRe spdlog::debug("[{}]: loading mixer api module", self->name_); wp_core_load_component(self->wp_core_, "libwireplumber-module-mixer-api", "module", nullptr, - "mixer-api", nullptr, (GAsyncReadyCallback)onMixerApiLoaded, self); + "mixer-api", nullptr, (GAsyncReadyCallback)onMixerApiLoaded, + new AsyncCall{self, call->generation}); } -void waybar::modules::Wireplumber::onMixerApiLoaded(WpObject* p, GAsyncResult* res, - waybar::modules::Wireplumber* self) { - if (!isModuleAlive(self)) { +void waybar::modules::Wireplumber::onMixerApiLoaded(WpObject* p, GAsyncResult* res, gpointer data) { + std::unique_ptr call(static_cast(data)); + auto* self = call->self; + if (!isModuleAlive(self) || call->generation != self->connection_generation_) { return; } @@ -570,16 +599,13 @@ void waybar::modules::Wireplumber::onMixerApiLoaded(WpObject* p, GAsyncResult* r self->activatePlugins(); self->dp.emit(); - - self->event_box_.add_events(Gdk::SCROLL_MASK | Gdk::SMOOTH_SCROLL_MASK); - self->event_box_.signal_scroll_event().connect(sigc::mem_fun(*self, &Wireplumber::handleScroll)); } void waybar::modules::Wireplumber::asyncLoadRequiredApiModules() { spdlog::debug("[{}]: loading default nodes api module", name_); wp_core_load_component(wp_core_, "libwireplumber-module-default-nodes-api", "module", nullptr, "default-nodes-api", nullptr, (GAsyncReadyCallback)onDefaultNodesApiLoaded, - this); + new AsyncCall{this, connection_generation_}); } static const std::array ports = { From 9b093c53e90cd194c3da8fe89d4105042bb98fad Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 5 Jul 2026 10:19:51 +0200 Subject: [PATCH 3/3] fix(wireplumber): read gboolean into a gboolean, not a 1-byte bool (OOB write) g_variant_lookup with the "b" format writes a gboolean (gint, 4 bytes), but muted_ and source_muted_ are C++ bool members (1 byte). Passing their addresses caused a 3-byte out-of-bounds write past the member (undefined behavior). Read into a gboolean temporary and assign back to the bool, preserving the prior value when "mute" is absent. --- src/modules/wireplumber.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/modules/wireplumber.cpp b/src/modules/wireplumber.cpp index 8f9bd961..ddcdda6e 100644 --- a/src/modules/wireplumber.cpp +++ b/src/modules/wireplumber.cpp @@ -304,7 +304,12 @@ void waybar::modules::Wireplumber::updateVolume(waybar::modules::Wireplumber* se g_variant_lookup(variant, "volume", "d", &self->volume_); g_variant_lookup(variant, "step", "d", &self->min_step_); - g_variant_lookup(variant, "mute", "b", &self->muted_); + // GVariant "b" writes a gboolean (4 bytes); reading directly into the 1-byte bool member is an + // out-of-bounds write. Read into a gboolean temporary and assign back. + gboolean mute = FALSE; + if (g_variant_lookup(variant, "mute", "b", &mute)) { + self->muted_ = mute; + } g_clear_pointer(&variant, g_variant_unref); self->dp.emit(); @@ -329,7 +334,11 @@ void waybar::modules::Wireplumber::updateSourceVolume(waybar::modules::Wireplumb } g_variant_lookup(variant, "volume", "d", &self->source_volume_); - g_variant_lookup(variant, "mute", "b", &self->source_muted_); + // See updateVolume: GVariant "b" writes a gboolean (4 bytes), not a 1-byte bool. + gboolean mute = FALSE; + if (g_variant_lookup(variant, "mute", "b", &mute)) { + self->source_muted_ = mute; + } g_clear_pointer(&variant, g_variant_unref); self->dp.emit();