Merge pull request #5165 from Alexays/fix/crash-batch3

fix: mpris resume SIGSEGV (#5124), wireplumber async UAF (#3974), stale reload batch state (#4129)

include/modules/wireplumber.hpp

@@ -36,6 +36,9 @@ class Wireplumber : public ALabel {
   std::vector<std::string> getWPIcon();
 
   static std::list<waybar::modules::Wireplumber*> 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);

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);

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();
   }
 }

src/modules/wireplumber.cpp

@@ -10,6 +10,17 @@ bool isValidNodeId(uint32_t id) { return id > 0 && id < G_MAXUINT32; }
 
 std::list<waybar::modules::Wireplumber*> 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;