Merge pull request #5175 from Alexays/fix/post-0150-review

fix: batch of bugs found reviewing the 0.15.0..master diff

include/group.hpp

@@ -14,7 +14,7 @@ class Group : public AModule {
 
  public:
   Group(const std::string&, const std::string&, const Json::Value&, bool);
-  ~Group() override = default;
+  ~Group() override;
   auto update() -> void override;
   operator Gtk::Widget&() override;
 

include/modules/hyprland/workspaces.hpp

@@ -227,8 +227,9 @@ class Workspaces : public AModule, public EventHandler {
   sigc::connection m_scrollEventConnection_;
   IPC& m_ipc;
 
+  // Coalesces bursts of Hyprland events into a single UI refresh. Armed and
+  // disconnected only on the GTK main thread (see Workspaces::update).
   sigc::connection m_debounceTimer;
-  bool m_updatePending = false;
 };
 
 }  // namespace waybar::modules::hyprland

include/modules/mango/backend.hpp

@@ -1,6 +1,7 @@
 // include/modules/mango/backend.hpp
 #pragma once
 
+#include <atomic>
 #include <list>
 #include <mutex>
 #include <string>
@@ -52,6 +53,7 @@ class IPC {
 
   static Json::Value sendCommand(const std::string& cmd);
 
+  std::atomic<bool> running_ = true;
   int sockfd_ = -1;
   std::thread ipc_thread_;
   mutable std::mutex data_mutex_;

include/modules/sni/host.hpp

@@ -16,7 +16,8 @@ class Host {
  public:
   Host(std::size_t id, const Json::Value&, const Bar&, const std::vector<std::string>&,
        const std::function<void(std::unique_ptr<Item>&)>&,
-       const std::function<void(std::unique_ptr<Item>&)>&, const std::function<void()>&);
+       const std::function<void(std::unique_ptr<Item>&)>&, const std::function<void()>&,
+       const std::function<void()>&);
   ~Host();
 
   void checkIgnoreList(const std::vector<std::string>& ignore_list,
@@ -55,6 +56,10 @@ class Host {
   const std::vector<std::string> ignore_list_;
   const std::function<void(std::unique_ptr<Item>&)> on_add_;
   const std::function<void(std::unique_ptr<Item>&)> on_remove_;
+  // Re-applies the configured ordering to the already-added tray widgets. This
+  // must NOT re-run the add path (which would re-parent widgets and reconnect
+  // signals); it only reorders existing children.
+  const std::function<void()> on_reorder_;
 
   ItemOrderMap orders_;
   const std::function<void()> on_update_;

include/modules/sni/tray.hpp

@@ -1,6 +1,10 @@
 #pragma once
 
 #include <fmt/format.h>
+#include <sigc++/connection.h>
+
+#include <unordered_map>
+#include <utility>
 
 #include "AModule.hpp"
 #include "bar.hpp"
@@ -19,6 +23,9 @@ class Tray : public AModule {
  private:
   void onAdd(std::unique_ptr<Item>& item);
   void onRemove(std::unique_ptr<Item>& item);
+  // Reorders the already-added tray widgets by their configured order. Does not
+  // add or remove any widget.
+  void reorderBox();
   void checkIgnoreList(std::unique_ptr<Item>* item);
   std::vector<std::string> parseIgnoreList(const Json::Value& config);
   void queueUpdate();
@@ -29,6 +36,9 @@ class Tray : public AModule {
   std::vector<std::string> ignore_list_;
   SNI::Host host_;
   std::vector<Item*> items_;
+  // signal_show/signal_hide connections owned per added item, so they can be
+  // disconnected on removal instead of leaking and accumulating.
+  std::unordered_map<Item*, std::pair<sigc::connection, sigc::connection>> item_connections_;
 };
 
 }  // namespace waybar::modules::SNI

src/AModule.cpp

@@ -238,9 +238,23 @@ bool AModule::handleUserEvent(GdkEventButton* const& e) {
   if (!format.empty()) {
     const int width = gdk_window_get_width(e->window);
     const int height = gdk_window_get_height(e->window);
-    const std::string cmd =
+    // Substitute {x}/{y} with the click position. The configured command is
-        fmt::format(fmt::runtime(format), fmt::arg("x", (int)round(100. * e->x / width)),
+    // arbitrary user input that may contain literal braces which are not {x}/{y}
+    // (e.g. `echo ${HOME}`, `awk '{print $1}'`, brace expansions). Those make
+    // libfmt throw fmt::format_error; since we run inside a GTK signal handler an
+    // uncaught exception aborts the whole bar. Only format when a placeholder is
+    // actually present, and fall back to the raw command if formatting throws.
+    std::string cmd = format;
+    if (format.find("{x}") != std::string::npos || format.find("{y}") != std::string::npos) {
+      try {
+        cmd = fmt::format(fmt::runtime(format), fmt::arg("x", (int)round(100. * e->x / width)),
                           fmt::arg("y", (int)round(100. * e->y / height)));
+      } catch (const fmt::format_error& err) {
+        spdlog::warn("Failed to format command '{}': {}. Running it unformatted.", format,
+                     err.what());
+        cmd = format;
+      }
+    }
     pid_children_.push_back(util::command::forkExec(cmd));
   }
   dp.emit();

src/bar.cpp

@@ -331,12 +331,14 @@ waybar::Bar::Bar(struct waybar_output* w_output, const Json::Value& w_config)
    * returned to the main loop, when any late initial configure has been dispatched and widgets have
    * had a chance to allocate/draw.
    */
-  Glib::signal_idle().connect(sigc::track_obj([this] {
+  Glib::signal_idle().connect(sigc::track_obj(
+      [this] {
         window.queue_resize();
         window.queue_draw();
         forceLayerCommit();
         return false;
-  }, *this));
+      },
+      *this));
 
   if (spdlog::should_log(spdlog::level::debug)) {
     // Unfortunately, this function isn't in the C++ bindings, so we have to call the C version.
@@ -750,9 +752,12 @@ void waybar::Bar::onOutputGeometryChanged() {
 }
 
 void waybar::Bar::toggleSuspend(bool suspend) {
-  auto process_modules = [suspend](Gtk::Box& module_box) {
+  // Iterate the actual module objects. Modules are packed into the Gtk::Box via
-    for (auto* widget : module_box.get_children()) {
+  // AModule::operator Gtk::Widget&(), which returns the member event_box_, so the
-      auto* module = dynamic_cast<waybar::AModule*>(widget);
+  // box children are Gtk::EventBox, never AModule -- a dynamic_cast over them is
+  // always null and suspend()/resume() would never fire. modules_all_ holds the
+  // real module pointers (including group children), so use it instead.
+  for (auto const& module : modules_all_) {
     if (module && module->shouldSuspend()) {
       if (suspend) {
         module->suspend();
@@ -761,9 +766,4 @@ void waybar::Bar::toggleSuspend(bool suspend) {
       }
     }
   }
-  };
-
-  process_modules(left_);
-  process_modules(center_);
-  process_modules(right_);
 }

src/config.cpp

@@ -240,12 +240,33 @@ bool isValidOutput(const Json::Value& config, const std::string& name,
         continue;
       }
       std::string str = config_output_dimension.asString();
-      int i = str.find(" ");
+      auto first_space = str.find(' ');
-      std::string dimension = str.substr(0, i);
+      if (first_space == std::string::npos) {
-      str = str.substr(i + 1);
+        spdlog::warn(
-      i = str.find(" ");
+            "Ignoring malformed 'output-dimensions' entry (expected '<dimension> <comparator> "
-      std::string comparator = str.substr(0, i);
+            "<value>'): '{}'",
-      int value = std::stoi(str.substr(i));
+            str);
+        continue;
+      }
+      std::string dimension = str.substr(0, first_space);
+      str = str.substr(first_space + 1);
+      auto second_space = str.find(' ');
+      if (second_space == std::string::npos) {
+        spdlog::warn(
+            "Ignoring malformed 'output-dimensions' entry (expected '<dimension> <comparator> "
+            "<value>'): '{}'",
+            config_output_dimension.asString());
+        continue;
+      }
+      std::string comparator = str.substr(0, second_space);
+      int value;
+      try {
+        value = std::stoi(str.substr(second_space + 1));
+      } catch (const std::exception& e) {
+        spdlog::warn("Ignoring 'output-dimensions' entry with non-integer value: '{}'",
+                     config_output_dimension.asString());
+        continue;
+      }
 
       int comparison_value;
       if (dimension == "height") {

src/group.cpp

@@ -105,6 +105,14 @@ Group::Group(const std::string& name, const std::string& id, const Json::Value&
   event_box_.add(box);
 }
 
+Group::~Group() {
+  // Disconnect any pending reveal timeout so it cannot fire on a destroyed
+  // instance (the GLib source is not removed by sigc::connection's destructor).
+  if (reveal_timeout_.connected()) {
+    reveal_timeout_.disconnect();
+  }
+}
+
 void Group::show_group() {
   box.set_state_flags(Gtk::StateFlags::STATE_FLAG_PRELIGHT);
   revealer.set_reveal_child(true);

src/modules/clock.cpp

@@ -220,6 +220,7 @@ auto waybar::modules::Clock::update() -> void {
     if (tzInTooltip_) tzText_ = getTZtext(now.get_sys_time());
     if (cldInTooltip_) cldText_ = get_calendar(today, shiftedDay, tz);
     if (ordInTooltip_) ordText_ = get_ordinal_date(shiftedDay);
+    try {
       if (tzInTooltip_ || cldInTooltip_ || ordInTooltip_) {
         // std::vformat doesn't support named arguments.
         m_tlpText_ =
@@ -234,6 +235,20 @@ auto waybar::modules::Clock::update() -> void {
       }
 
       m_tlpText_ = fmt_lib::vformat(m_locale_, m_tlpText_, fmt_lib::make_format_args(now));
+    } catch (const std::exception& e) {
+      // An unsupported/invalid specifier (e.g. %-I / %OI) in the tooltip-format or the
+      // calendar format must not take the whole module down every tick. Warn once and skip
+      // the tooltip for this update so the bar keeps working.
+      static bool tlpWarned = false;
+      if (!tlpWarned) {
+        spdlog::warn(
+            "Clock: could not format tooltip \"{}\": {}. Skipping tooltip; check your "
+            "tooltip-format/calendar format specifiers.",
+            m_tlpFmt_, e.what());
+        tlpWarned = true;
+      }
+      m_tlpText_.clear();
+    }
 
     // Pango doesn't support CSS classes but to continue using it while staying
     // backwards compatible this approach uses post-posting to replace fake

src/modules/custom_graph.cpp

@@ -99,7 +99,13 @@ void waybar::modules::CustomGraph::continuousWorker() {
         thread_.sleep_for(std::chrono::seconds(config_["restart-interval"].asUInt()));
         fp_ = util::command::open(cmd, pid_, output_name_);
         if (!fp_) {
-          throw std::runtime_error("Unable to open " + cmd);
+          // Letting this exception escape the SleeperThread would call
+          // std::terminate and kill all of Waybar. Degrade gracefully instead.
+          output_ = {1, ""};
+          dp.emit();
+          spdlog::error("Unable to restart {}: unable to open {}", name_, cmd);
+          thread_.stop();
+          return;
         }
       } else {
         thread_.stop();
@@ -139,9 +145,11 @@ void waybar::modules::CustomGraph::waitingWorker() {
 }
 
 void waybar::modules::CustomGraph::refresh(int sig) {
+#ifdef SIGRTMIN
   if (config_["signal"].isInt() && sig == SIGRTMIN + config_["signal"].asInt()) {
     thread_.wake_up();
   }
+#endif
 }
 
 void waybar::modules::CustomGraph::handleEvent() {
@@ -268,22 +276,32 @@ void waybar::modules::CustomGraph::parseOutputJson() {
   std::istringstream output(output_.out);
   std::string line;
   class_.clear();
+  // A script can emit invalid UTF-8; passing it unchecked to Pango/GTK aborts
+  // the whole bar in g_utf8_* (see parseOutputRaw, which validates the same way).
+  auto sanitize = [](const std::string& s) -> Glib::ustring {
+    Glib::ustring value = s;
+    if (!value.validate()) {
+      value = value.make_valid();
+    }
+    return value;
+  };
   while (getline(output, line)) {
     auto parsed = parser_.parse(line);
-    if (config_["escape"].isBool() && config_["escape"].asBool()) {
+    const bool escape = config_["escape"].isBool() && config_["escape"].asBool();
-      text_ = Glib::Markup::escape_text(parsed["text"].asString());
+    if (escape) {
+      text_ = Glib::Markup::escape_text(sanitize(parsed["text"].asString()));
     } else {
-      text_ = parsed["text"].asString();
+      text_ = sanitize(parsed["text"].asString());
     }
-    if (config_["escape"].isBool() && config_["escape"].asBool()) {
+    if (escape) {
-      alt_ = Glib::Markup::escape_text(parsed["alt"].asString());
+      alt_ = Glib::Markup::escape_text(sanitize(parsed["alt"].asString()));
     } else {
-      alt_ = parsed["alt"].asString();
+      alt_ = sanitize(parsed["alt"].asString());
     }
-    if (config_["escape"].isBool() && config_["escape"].asBool()) {
+    if (escape) {
-      tooltip_ = Glib::Markup::escape_text(parsed["tooltip"].asString());
+      tooltip_ = Glib::Markup::escape_text(sanitize(parsed["tooltip"].asString()));
     } else {
-      tooltip_ = parsed["tooltip"].asString();
+      tooltip_ = sanitize(parsed["tooltip"].asString());
     }
     if (parsed["class"].isString()) {
       class_.push_back(parsed["class"].asString());

src/modules/hyprland/backend.cpp

@@ -297,11 +297,40 @@ bool IPC::isLuaProtocol() {
     return *s_luaProtocolDetected_;
   }
 
-  // Probe: send a harmless old-style dispatch and check the error.
+  // Detect the Lua-based dispatch protocol (Hyprland >= 0.54) via the read-only
-  // In Lua-based Hyprland (>= 0.54) the error contains "hl.dispatch".
+  // "version" query. This MUST have no side effects: an earlier probe issued a real
-  // In older versions it returns "ok" or a different error.
+  // "dispatch workspace __waybar_probe__", which on Hyprland < 0.54 actually switched
-  auto reply = getSocket1Reply("dispatch workspace __waybar_probe__");
+  // the user to a junk workspace named __waybar_probe__ on the first click/scroll.
-  bool luaProto = reply.find("hl.dispatch") != std::string::npos;
+  bool luaProto = false;
+  try {
+    util::JsonParser parser;
+    const Json::Value ver = parser.parse(getSocket1Reply("j/version"));
+
+    // Prefer the numeric "version" field ("0.54.0"); fall back to the "tag" field
+    // ("v0.54.0" or "v0.54.0-16-gdeadbee"), which is present on all releases.
+    std::string versionStr = ver["version"].asString();
+    if (versionStr.empty()) {
+      versionStr = ver["tag"].asString();
+    }
+
+    const size_t firstDigit = versionStr.find_first_of("0123456789");
+    if (firstDigit != std::string::npos) {
+      // std::stoi parses the leading integer and stops at the first non-digit, so it
+      // tolerates the trailing ".patch-commits-ghash" suffix on the tag.
+      const int major = std::stoi(versionStr.substr(firstDigit));
+      int minor = 0;
+      const size_t dot = versionStr.find('.', firstDigit);
+      if (dot != std::string::npos && dot + 1 < versionStr.size()) {
+        minor = std::stoi(versionStr.substr(dot + 1));
+      }
+      luaProto = major > 0 || (major == 0 && minor >= 54);
+    } else {
+      spdlog::warn("Hyprland IPC: could not parse version '{}', assuming legacy protocol",
+                   versionStr);
+    }
+  } catch (const std::exception& e) {
+    spdlog::warn("Hyprland IPC: version detection failed ({}), assuming legacy protocol", e.what());
+  }
 
   if (luaProto) {
     spdlog::info("Hyprland IPC: detected Lua-based dispatch protocol (Hyprland >= 0.54)");

src/modules/hyprland/workspaces.cpp

@@ -37,6 +37,11 @@ Workspaces::~Workspaces() {
   if (m_scrollEventConnection_.connected()) {
     m_scrollEventConnection_.disconnect();
   }
+  // Cancel any pending debounce timeout so it cannot fire on a freed `this`.
+  // Runs on the main thread, same as where the timer is armed.
+  if (m_debounceTimer.connected()) {
+    m_debounceTimer.disconnect();
+  }
   m_ipc.unregisterForIPC(this);
   // wait for possible event handler to finish
   std::lock_guard<std::mutex> lg(m_mutex);
@@ -332,23 +337,11 @@ void Workspaces::onEvent(const std::string& ev) {
     }
   }
 
-  if (m_debounceTimer.connected()) {
+  // Notify the main thread. dp (Glib::Dispatcher) is the only thread-safe way to
-    m_debounceTimer.disconnect();
+  // hand off to the GTK main loop; GLib timer state must never be touched from the
-    m_updatePending = false;
+  // IPC listener thread. The debounce timer is owned entirely by the main-thread
-  }
+  // update() path (see Workspaces::update).
-
-  m_updatePending = true;
-  m_debounceTimer = Glib::signal_timeout().connect(
-      [this]() {
-        if (!m_updatePending) return false;
-        std::lock_guard<std::mutex> lock(m_mutex);
-        if (m_updatePending) {
   dp.emit();
-          m_updatePending = false;
-        }
-        return false;
-      },
-      7);
 }
 
 void Workspaces::onWorkspaceActivated(std::string const& payload) {
@@ -1041,8 +1034,20 @@ void Workspaces::setUrgentWorkspace(std::string const& windowaddress) {
 }
 
 auto Workspaces::update() -> void {
+  // Debounce rapid events (e.g. out-of-order create/destroy workspace events from
+  // Hyprland) to prevent workspace button flicker. This runs on the GTK main thread
+  // (invoked via the dp dispatcher), so arming/disconnecting the GLib timer here is
+  // thread-safe. Each event re-arms the timer, coalescing bursts into one refresh.
+  if (m_debounceTimer.connected()) {
+    m_debounceTimer.disconnect();
+  }
+  m_debounceTimer = Glib::signal_timeout().connect(
+      [this]() {
         doUpdate();
         AModule::update();
+        return false;
+      },
+      7);
 }
 
 void Workspaces::updateWindowCount() {

src/modules/keyboard_state.cpp

@@ -320,7 +320,14 @@ auto waybar::modules::KeyboardState::update() -> void {
   for (const auto& dev_path : dev_paths) {
     try {
       int fd = openFile(dev_path, O_NONBLOCK | O_CLOEXEC | O_RDONLY);
-      auto dev = openDevice(fd);
+      libevdev* dev;
+      try {
+        dev = openDevice(fd);
+      } catch (...) {
+        // openDevice does not close the fd if libevdev_new_from_fd fails.
+        closeFile(fd);
+        throw;
+      }
       numl |= libevdev_get_event_value(dev, EV_LED, LED_NUML);
       capsl |= libevdev_get_event_value(dev, EV_LED, LED_CAPSL);
       scrolll |= libevdev_get_event_value(dev, EV_LED, LED_SCROLLL);
@@ -376,7 +383,14 @@ auto waybar::modules::KeyboardState::update() -> void {
 auto waybar::modules ::KeyboardState::tryAddDevice(const std::string& dev_path) -> void {
   try {
     int fd = openFile(dev_path, O_NONBLOCK | O_CLOEXEC | O_RDONLY);
-    auto dev = openDevice(fd);
+    libevdev* dev;
+    try {
+      dev = openDevice(fd);
+    } catch (...) {
+      // openDevice does not close the fd if libevdev_new_from_fd fails.
+      closeFile(fd);
+      throw;
+    }
     if (supportsLockStates(dev)) {
       spdlog::info("Found device {} at '{}'", libevdev_get_name(dev), dev_path);
       std::lock_guard<std::mutex> lock(devices_mutex_);

src/modules/mango/backend.cpp

@@ -8,6 +8,7 @@
 #include <sys/un.h>
 #include <unistd.h>
 
+#include <chrono>
 #include <sstream>
 #include <thread>
 #include <vector>
@@ -100,42 +101,77 @@ void IPC::sendAsync(const Json::Value& request) {
 IPC::IPC() : sockfd_(-1), active_client_(Json::nullValue) { startIPC(); }
 
 IPC::~IPC() {
+  running_ = false;
   if (sockfd_ != -1) close(sockfd_);
   if (ipc_thread_.joinable()) ipc_thread_.join();
 }
 
 void IPC::startIPC() {
+  // Connect synchronously so a missing socket (this WM isn't the active
+  // compositor) throws here and lets the module constructor fail, instead of
+  // the module always attaching with a permanently empty widget.
   sockfd_ = IPC::connectToSocket();
 
   ipc_thread_ = std::thread([this]() {
     spdlog::info("Mango IPC thread started");
 
-    struct pollfd pfd;
+    char buf[4096];
-    pfd.fd = sockfd_;
+    std::string buffer;
-    pfd.events = POLLIN;
+    bool have_initial_fd = true;
 
+    // Reconnect loop: if the event stream drops (POLLHUP/POLLERR, read()==0 or
+    // an error) we back off briefly and re-establish the socket instead of
+    // leaving every mango module frozen forever with stale content.
+    while (running_) {
+      if (!have_initial_fd) {
+        try {
+          sockfd_ = IPC::connectToSocket();
+        } catch (const std::exception& e) {
+          spdlog::error("Mango IPC: failed to reconnect: {}", e.what());
+          std::this_thread::sleep_for(std::chrono::seconds(2));
+          continue;
+        }
+      }
+      have_initial_fd = false;
+
+      bool subscribed = true;
       const std::vector<std::string> subs = {"watch all-monitors"};
       for (const auto& cmd : subs) {
         if (write(sockfd_, cmd.c_str(), cmd.size()) != (ssize_t)cmd.size() ||
             write(sockfd_, "\n", 1) != 1) {
           spdlog::error("Failed to subscribe to {}", cmd);
-        return;
+          subscribed = false;
+          break;
         }
       }
+      if (!subscribed) {
+        if (sockfd_ != -1) {
+          close(sockfd_);
+          sockfd_ = -1;
+        }
+        std::this_thread::sleep_for(std::chrono::seconds(2));
+        continue;
+      }
 
-    char buf[4096];
+      struct pollfd pfd;
-    std::string buffer;
+      pfd.fd = sockfd_;
-    while (true) {
+      pfd.events = POLLIN;
+      buffer.clear();
+
+      bool connected = true;
+      while (running_ && connected) {
         int ret = poll(&pfd, 1, 1000);
         if (ret == 0) continue;
         if (ret < 0) {
           if (errno == EINTR) continue;
           spdlog::error("IPC poll error: {}", strerror(errno));
+          connected = false;
           break;
         }
 
         if (pfd.revents & (POLLERR | POLLHUP | POLLNVAL)) {
           spdlog::info("Mango IPC socket closed or invalid");
+          connected = false;
           break;
         }
 
@@ -143,11 +179,13 @@ void IPC::startIPC() {
           ssize_t n = read(sockfd_, buf, sizeof(buf));
           if (n == 0) {
             spdlog::info("Mango IPC connection closed");
+            connected = false;
             break;
           }
           if (n < 0) {
             if (errno == EINTR) continue;
             spdlog::error("IPC read error: {}", strerror(errno));
+            connected = false;
             break;
           }
           buffer.append(buf, n);
@@ -165,6 +203,19 @@ void IPC::startIPC() {
           }
         }
       }
+
+      // On shutdown leave the socket for the destructor to close (avoids a
+      // double close); on a genuine disconnect close it before reconnecting.
+      if (!running_) break;
+      if (sockfd_ != -1) {
+        close(sockfd_);
+        sockfd_ = -1;
+      }
+      spdlog::warn("Mango IPC: event stream closed, reconnecting");
+      std::this_thread::sleep_for(std::chrono::seconds(2));
+    }
+
+    spdlog::info("Mango IPC thread stopping");
   });
 }
 

src/modules/mango/language.cpp

@@ -110,8 +110,6 @@ Language::Layout Language::getLayout(const std::string& fullName) {
           short_description = short_name;
         }
 
-        short_description = short_name;
-
         Layout info{desc, short_name, variant, short_description};
         return info;
       }

src/modules/mpris/mpris.cpp

@@ -491,7 +491,13 @@ auto Mpris::getPlayerInfo() -> std::optional<PlayerInfo> {
         continue;
       }
       auto* tmp = playerctl_player_new_from_name(pn, &error);
-      if (error || !tmp) continue;
+      if (error || !tmp) {
+        // Discard any error from this candidate so it doesn't leak into the next
+        // playerctl_player_new_from_name() call or the post-loop metadata calls, which
+        // assert that the passed GError is NULL (otherwise: GLib-CRITICAL / spurious errorexit).
+        g_clear_error(&error);
+        continue;
+      }
       if (!first_valid_player) {
         first_valid_player = tmp;
         first_valid_name = name;

src/modules/network.cpp

@@ -90,13 +90,16 @@ uint32_t waybar::modules::Network::readLinkSpeed() const {
 
   if (!sysfs_speed) return 0;
 
-  uint32_t speed;
+  // Read into a signed type: /sys/class/net/<if>/speed reports -1 when there is
+  // no carrier. Extracting -1 into an unsigned type would wrap to a huge value
+  // (and would not set failbit), so use a signed type and validate the result.
+  int64_t speed = 0;
   sysfs_speed >> speed;
 
-  if (sysfs_speed.bad())  // read fails on incompatible devices
+  if (sysfs_speed.fail() || speed < 0)  // read fails on incompatible devices
     return 0;
 
-  return speed;
+  return static_cast<uint32_t>(speed);
 }
 
 waybar::modules::Network::Network(const std::string& id, const Json::Value& config)

src/modules/niri/backend.cpp

@@ -285,7 +285,7 @@ void IPC::unregisterForIPC(EventHandler* ev_handler) {
 Json::Value IPC::send(const Json::Value& request) {
   util::ScopedFd socketfd(connectToSocket());
 
-  auto unix_istream = Gio::UnixInputStream::create(socketfd, true);
+  auto unix_istream = Gio::UnixInputStream::create(socketfd, false);
   auto unix_ostream = Gio::UnixOutputStream::create(socketfd, false);
   auto istream = Gio::DataInputStream::create(unix_istream);
   auto ostream = Gio::DataOutputStream::create(unix_ostream);

src/modules/sni/host.cpp

@@ -16,7 +16,7 @@ Host::Host(std::size_t id, const Json::Value& config, const Bar& bar,
            const std::vector<std::string>& ignore_list,
            const std::function<void(std::unique_ptr<Item>&)>& on_add,
            const std::function<void(std::unique_ptr<Item>&)>& on_remove,
-           const std::function<void()>& on_update)
+           const std::function<void()>& on_reorder, const std::function<void()>& on_update)
     : bus_name_("org.kde.StatusNotifierHost-" + std::to_string(getpid()) + "-" +
                 std::to_string(id)),
       object_path_("/StatusNotifierHost/" + std::to_string(id)),
@@ -27,6 +27,7 @@ Host::Host(std::size_t id, const Json::Value& config, const Bar& bar,
       ignore_list_(ignore_list),
       on_add_(on_add),
       on_remove_(on_remove),
+      on_reorder_(on_reorder),
       on_update_(on_update) {
   auto orders = config["orders"];
   if (!orders.isNull()) {
@@ -292,11 +293,15 @@ void Host::addRegisteredItem(const std::string& service) {
 }
 
 void Host::reorderItems() {
-  std::ranges::for_each(items_, on_remove_);
+  // Re-apply the configured ordering to the tray. This is invoked while an
-  std::ranges::sort(items_, [](std::unique_ptr<Item>& item1, std::unique_ptr<Item>& item2) {
+  // item's Id/order is first resolved (from Item::setCustomIcon), which happens
-    return item1->order_ < item2->order_;
+  // *before* the item is marked ready and added. It must therefore only reorder
-  });
+  // the widgets that have already been added; re-running the full add path here
-  std::ranges::for_each(items_, on_add_);
+  // would (a) re-parent widgets and reconnect signals for every item and (b)
+  // mutate items_ from within checkIgnoreList while it is being iterated,
+  // invalidating iterators/pointers. Delegating to on_reorder_ keeps this to a
+  // pure reordering of existing children.
+  on_reorder_();
 }
 
 }  // namespace waybar::modules::SNI

src/modules/sni/tray.cpp

@@ -38,7 +38,7 @@ Tray::Tray(const std::string& id, const Bar& bar, const Json::Value& config)
       host_((initIconsConfig(config), nb_hosts_), config, bar, ignore_list_,
             std::bind(&Tray::onAdd, this, std::placeholders::_1),
             std::bind(&Tray::onRemove, this, std::placeholders::_1),
-            std::bind(&Tray::queueUpdate, this)) {
+            std::bind(&Tray::reorderBox, this), std::bind(&Tray::queueUpdate, this)) {
   box_.set_name("tray");
   event_box_.add(box_);
   if (!id.empty()) {
@@ -63,6 +63,16 @@ void Tray::onAdd(std::unique_ptr<Item>& item) {
   spdlog::info("Tray::onAdd - item bus_name='{}', category='{}', icon_name='{}', title='{}'",
                item->bus_name, item->category, item->icon_name, item->title);
 
+  // Idempotency guard: onAdd can be reached more than once for the same item
+  // (e.g. an item is processed while its Id/order is resolved and then again
+  // when it becomes ready). Re-adding would re-parent the event_box (GTK
+  // "widget already has a parent" critical), push a duplicate pointer into
+  // items_ and leak extra signal connections.
+  if (std::find(items_.begin(), items_.end(), item.get()) != items_.end()) {
+    spdlog::debug("Tray::onAdd - item already added, skipping");
+    return;
+  }
+
   if (config_["reverse-direction"].isBool() && config_["reverse-direction"].asBool()) {
     box_.pack_end(item->event_box);
   } else {
@@ -70,8 +80,13 @@ void Tray::onAdd(std::unique_ptr<Item>& item) {
   }
   items_.push_back(item.get());
 
-  item->event_box.signal_show().connect([this] { dp.emit(); });
+  auto show_conn = item->event_box.signal_show().connect([this] { dp.emit(); });
-  item->event_box.signal_hide().connect([this] { dp.emit(); });
+  auto hide_conn = item->event_box.signal_hide().connect([this] { dp.emit(); });
+  item_connections_[item.get()] = {show_conn, hide_conn};
+
+  // Position the freshly added widget according to the configured order. This
+  // must happen before the ignore-list check below, which may erase `item`.
+  reorderBox();
 
   // After this point `item` may be erased/invalidated by the ignore-list check;
   // do not touch it again below.
@@ -82,11 +97,37 @@ void Tray::onAdd(std::unique_ptr<Item>& item) {
 }
 
 void Tray::onRemove(std::unique_ptr<Item>& item) {
+  // May be called for items that were never added (e.g. the ignore-list check
+  // runs over items that are not yet ready). Only touch state we actually own.
+  auto it = std::find(items_.begin(), items_.end(), item.get());
+  if (it == items_.end()) {
+    return;
+  }
+
+  auto conn_it = item_connections_.find(item.get());
+  if (conn_it != item_connections_.end()) {
+    conn_it->second.first.disconnect();
+    conn_it->second.second.disconnect();
+    item_connections_.erase(conn_it);
+  }
+
   box_.remove(item->event_box);
-  items_.erase(std::remove(items_.begin(), items_.end(), item.get()), items_.end());
+  items_.erase(it);
   dp.emit();
 }
 
+void Tray::reorderBox() {
+  const bool reverse =
+      config_["reverse-direction"].isBool() && config_["reverse-direction"].asBool();
+  // Stable sort keeps insertion order among items sharing the same order value.
+  std::stable_sort(items_.begin(), items_.end(),
+                   [](const Item* a, const Item* b) { return a->order_ < b->order_; });
+  for (std::size_t i = 0; i < items_.size(); ++i) {
+    const int pos = reverse ? static_cast<int>(items_.size() - 1 - i) : static_cast<int>(i);
+    box_.reorder_child(items_[i]->event_box, pos);
+  }
+}
+
 auto Tray::update() -> void {
   // Check if any items should be ignored now that properties have loaded
   if (!ignore_list_.empty()) {

src/modules/sway/window.cpp

@@ -101,7 +101,7 @@ auto Window::update() -> void {
                   fmt::arg("shell", shell_), fmt::arg("marks", marks_)),
       config_["rewrite"]));
   if (tooltipEnabled()) {
-    setTooltipMarkup(window_);
+    setTooltipMarkup(Glib::Markup::escape_text(window_));
   }
 
   // Resolve the app icon on the main thread to avoid racing with GTK draw on the

src/modules/systemd_failed_units.cpp

@@ -281,21 +281,42 @@ auto SystemdFailedUnits::update() -> void {
 
   last_status_ = overall_state_;
 
+  // A malformed user format/tooltip-format (e.g. an unknown {placeholder}) makes fmt throw a
+  // fmt::format_error; catch it so a bad config warns once instead of taking update() down.
+  try {
     setLabelMarkup(fmt::format(
         fmt::runtime(nr_failed_ == 0 ? format_ok_ : format_), fmt::arg("nr_failed", nr_failed_),
-      fmt::arg("nr_failed_system", nr_failed_system_), fmt::arg("nr_failed_user", nr_failed_user_),
+        fmt::arg("nr_failed_system", nr_failed_system_),
-      fmt::arg("system_state", system_state_), fmt::arg("user_state", user_state_),
+        fmt::arg("nr_failed_user", nr_failed_user_), fmt::arg("system_state", system_state_),
-      fmt::arg("overall_state", overall_state_)));
+        fmt::arg("user_state", user_state_), fmt::arg("overall_state", overall_state_)));
+  } catch (const std::exception& e) {
+    static bool labelWarned = false;
+    if (!labelWarned) {
+      spdlog::warn("systemd-failed-units: invalid format, using fallback: {}", e.what());
+      labelWarned = true;
+    }
+    setLabelMarkup(fmt::format("{} failed", nr_failed_));
+  }
   if (tooltipEnabled()) {
     std::string failed_list = BuildTooltipFailedList();
     auto tooltip_template = overall_state_ == "ok" ? tooltip_format_ok_ : tooltip_format_;
     if (!tooltip_template.empty()) {
+      try {
         setTooltipMarkup(fmt::format(
             fmt::runtime(tooltip_template), fmt::arg("nr_failed", nr_failed_),
             fmt::arg("nr_failed_system", nr_failed_system_),
             fmt::arg("nr_failed_user", nr_failed_user_), fmt::arg("system_state", system_state_),
             fmt::arg("user_state", user_state_), fmt::arg("overall_state", overall_state_),
             fmt::arg("failed_units_list", failed_list)));
+      } catch (const std::exception& e) {
+        static bool tooltipWarned = false;
+        if (!tooltipWarned) {
+          spdlog::warn("systemd-failed-units: invalid tooltip-format, skipping tooltip: {}",
+                       e.what());
+          tooltipWarned = true;
+        }
+        setTooltipMarkup("");
+      }
     } else {
       setTooltipMarkup("");
     }

src/modules/wwan.cpp

@@ -43,7 +43,7 @@ waybar::modules::Wwan::Wwan(const std::string& id, const Json::Value& config)
   if (error) {
     spdlog::error("Failed to create ModemManager proxy: " + std::string(error->message));
     g_error_free(error);
-    g_object_unref(connection);
+    g_clear_object(&connection);
     return;
   }
 
@@ -302,7 +302,7 @@ auto waybar::modules::Wwan::update() -> void {
 }
 
 waybar::modules::Wwan::~Wwan() {
-  g_object_unref(current_modem);
+  g_clear_object(&current_modem);
-  g_object_unref(manager);
+  g_clear_object(&manager);
-  g_object_unref(connection);
+  g_clear_object(&connection);
 }