diff --git a/include/modules/wireplumber.hpp b/include/modules/wireplumber.hpp index 66900501..f1650e92 100644 --- a/include/modules/wireplumber.hpp +++ b/include/modules/wireplumber.hpp @@ -36,6 +36,9 @@ class Wireplumber : public ALabel { std::vector getWPIcon(); static std::list modules; + // Returns true while `self` is still a live module. Async load/activation callbacks use this to + // avoid dereferencing a `self` that was destroyed before the callback fired (see #3974). + static bool isModuleAlive(waybar::modules::Wireplumber* self); uint32_t resolvePhysicalSink(uint32_t start_id); uint32_t findPlaybackNodeId(const gchar* description); diff --git a/src/client.cpp b/src/client.cpp index 511e35af..d09de5bd 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -286,6 +286,15 @@ void waybar::Client::bindInterfaces() { // Clear stale outputs from previous run outputs_.clear(); + // Also drop any batch state that was left pending from the previous run. On + // reload the GApplication is swapped but the default main context (and its + // queued PRIORITY_HIGH_IDLE createBarsBatch source) survives; pending_outputs_ + // would then hold dangling waybar_output* into the just-cleared outputs_ list, + // which createBarsBatch's address comparison can mis-match if the freed slot is + // reused. Reset so the next run schedules its batch from a clean state (#4129). + pending_outputs_.clear(); + bars_scheduled_ = false; + // add existing outputs and subscribe to updates for (auto i = 0; i < gdk_display->get_n_monitors(); ++i) { auto monitor = gdk_display->get_monitor(i); diff --git a/src/modules/mpris/mpris.cpp b/src/modules/mpris/mpris.cpp index 7eb11594..387e0597 100644 --- a/src/modules/mpris/mpris.cpp +++ b/src/modules/mpris/mpris.cpp @@ -406,8 +406,11 @@ auto Mpris::onPlayerNameVanished(PlayerctlPlayerManager* manager, PlayerctlPlaye if (mpris->player_ == "playerctld") { mpris->dp.emit(); } else if (mpris->player_ == player_name->name) { + // Don't touch GTK widgets directly from the playerctl callback: on resume + // from suspend this can run in a re-entrant / torn-down state and crash in + // Gtk::Widget::set_visible. Only update state + emit; update() (on the main + // thread) hides the module when there is no player. See #5124. mpris->player = nullptr; - mpris->event_box_.set_visible(false); mpris->dp.emit(); } } diff --git a/src/modules/wireplumber.cpp b/src/modules/wireplumber.cpp index b3167381..a2391071 100644 --- a/src/modules/wireplumber.cpp +++ b/src/modules/wireplumber.cpp @@ -10,6 +10,17 @@ bool isValidNodeId(uint32_t id) { return id > 0 && id < G_MAXUINT32; } std::list waybar::modules::Wireplumber::modules; +// 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 +// is removed while a component load is still pending, or during an audio route transition), the +// callback would dereference a freed `self`. The destructor removes `this` from this registry +// before any teardown, so a missing entry means `self` is dangling and the callback must bail out +// without touching it. See https://github.com/Alexays/Waybar/issues/3974. +bool waybar::modules::Wireplumber::isModuleAlive(waybar::modules::Wireplumber* self) { + return std::find(modules.begin(), modules.end(), self) != modules.end(); +} + waybar::modules::Wireplumber::Wireplumber(const std::string& id, const Json::Value& config) : ALabel(config, "wireplumber", id, "{volume}%"), wp_core_(nullptr), @@ -387,6 +398,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)) { + return; + } + const auto* pluginName = wp_plugin_get_name(WP_PLUGIN(p)); spdlog::debug("[{}]: onPluginActivated: {}", self->name_, pluginName); g_autoptr(GError) error = nullptr; @@ -432,6 +447,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)) { + return; + } + gboolean success = FALSE; g_autoptr(GError) error = nullptr; @@ -453,6 +472,10 @@ void waybar::modules::Wireplumber::onDefaultNodesApiLoaded(WpObject* p, GAsyncRe void waybar::modules::Wireplumber::onMixerApiLoaded(WpObject* p, GAsyncResult* res, waybar::modules::Wireplumber* self) { + if (!isModuleAlive(self)) { + return; + } + gboolean success = FALSE; g_autoptr(GError) error = nullptr;