Merge pull request #5158 from Alexays/fix/crash-freeze

fix: batch of crash/freeze fixes (clock DST, interval:0, thread-safety, reconnects)

include/modules/niri/backend.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <atomic>
 #include <list>
 #include <mutex>
 #include <string>
@@ -18,6 +19,7 @@ class EventHandler {
 class IPC {
  public:
   IPC();
+  ~IPC();
 
   void registerForIPC(const std::string& ev, EventHandler* ev_handler);
   void unregisterForIPC(EventHandler* handler);
@@ -45,6 +47,8 @@ class IPC {
   util::JsonParser parser_;
   std::mutex callbackMutex_;
   std::list<std::pair<std::string, EventHandler*>> callbacks_;
+
+  std::atomic<bool> running_{true};
 };
 
 inline std::unique_ptr<IPC> gIPC;

include/modules/sway/language.hpp

@@ -54,6 +54,9 @@ class Language : public ALabel, public sigc::trackable {
   const static std::string XKB_ACTIVE_LAYOUT_NAME_KEY;
 
   Layout layout_;
+  // CSS class currently applied to label_. Tracked so update() (main thread) can swap classes
+  // instead of set_current_layout() mutating the widget from the IPC worker thread (#3702).
+  std::string applied_class_;
   std::string tooltip_format_ = "";
   std::map<std::string, Layout> layouts_map_;
   bool hide_single_;

include/util/audio_backend.hpp

@@ -29,10 +29,17 @@ class AudioBackend {
   static void volumeModifyCb(pa_context*, int, void*);
   static void sourceVolumeModifyCb(pa_context*, int, void*);
   void connectContext();
+  // Non-throwing reconnect used from the PulseAudio callback thread. Throwing
+  // across the libpulse C callback boundary calls std::terminate, so this
+  // swallows any failure and reports it via the return value instead.
+  bool reconnectContext() noexcept;
 
   pa_threaded_mainloop* mainloop_;
   pa_mainloop_api* mainloop_api_;
   pa_context* context_;
+  // Guards against the FAILED -> connect -> FAILED recursion / busy loop when a
+  // reconnect attempt fails synchronously inside pa_context_connect().
+  bool reconnecting_{false};
   pa_cvolume pa_volume_;
   pa_cvolume pa_source_volume_;
 

src/ALabel.cpp

@@ -25,8 +25,15 @@ ALabel::ALabel(const Json::Value& config, const std::string& name, const std::st
                     ? std::chrono::milliseconds::max()
                     : std::chrono::milliseconds(
                           (config_["interval"].isNumeric()
-                               ? std::max(1L,  // Minimum 1ms due to millisecond precision
+                               ? (config_["interval"].asDouble() > 0
-                                          static_cast<long>(config_["interval"].asDouble() * 1000))
+                                      // Minimum 1ms due to millisecond precision
+                                      ? std::max(1L, static_cast<long>(
+                                                         config_["interval"].asDouble() * 1000))
+                                      // Only modules with no periodic default use 0 as an
+                                      // event-driven sentinel. Periodic modules fall back to their
+                                      // default interval so interval:0 cannot busy-loop or hit
+                                      // modulo-by-zero clock code.
+                                      : (interval == 0 ? 0L : 1000L * static_cast<long>(interval)))
                                : 1000 * (long)interval))),
       default_format_(format_) {
   label_.set_name(name);

src/client.cpp

@@ -211,7 +211,7 @@ const std::string waybar::Client::getStyle(const std::string& style,
 
   if (style.empty()) {
     std::vector<std::string> search_files;
-    switch (appearance.value_or(portal->getAppearance())) {
+    switch (appearance.value_or(portal ? portal->getAppearance() : waybar::Appearance::UNKNOWN)) {
       case waybar::Appearance::LIGHT:
         search_files.emplace_back("style-light.css");
         gtk_settings->property_gtk_application_prefer_dark_theme() = false;
@@ -344,16 +344,26 @@ int waybar::Client::main(int argc, char* argv[]) {
   wl_display = gdk_wayland_display_get_wl_display(gdk_display->gobj());
   config.load(config_opt);
   if (!portal) {
-    portal = std::make_unique<waybar::Portal>();
+    try {
+      portal = std::make_unique<waybar::Portal>();
+    } catch (const Glib::Error& e) {
+      spdlog::warn(
+          "Failed to connect to the desktop portal, light/dark theme detection disabled: {}",
+          std::string(e.what()));
+    } catch (...) {
+      spdlog::warn("Failed to connect to the desktop portal, light/dark theme detection disabled");
+    }
   }
   m_cssFile = getStyle(style_opt);
   setupCss(m_cssFile);
   m_cssReloadHelper = std::make_unique<CssReloadHelper>(m_cssFile, [&](const std::string& css_file) { setupCss(css_file); });
-  portal->signal_appearance_changed().connect([&](waybar::Appearance appearance) {
+  if (portal) {
-    auto css_file = getStyle(style_opt, appearance);
+    portal->signal_appearance_changed().connect([&](waybar::Appearance appearance) {
-    m_cssReloadHelper->changeCssFile(css_file);
+      auto css_file = getStyle(style_opt, appearance);
-    setupCss(css_file);
+      m_cssReloadHelper->changeCssFile(css_file);
-  });
+      setupCss(css_file);
+    });
+  }
 
   auto m_config = config.getConfig();
   if (m_config.isObject() && m_config["reload_style_on_change"].asBool()) {
@@ -378,5 +388,7 @@ int waybar::Client::main(int argc, char* argv[]) {
 void waybar::Client::reset() {
   gtk_app->quit();
   // delete signal handler for css changes
-  portal->signal_appearance_changed().clear();
+  if (portal) {
+    portal->signal_appearance_changed().clear();
+  }
 }

src/modules/clock.cpp

@@ -190,8 +190,13 @@ auto waybar::modules::Clock::update() -> void {
   if (tooltipEnabled()) {
     const year_month_day today{floor<days>(now.get_local_time())};
     const auto shiftedDay{today + cldCurrShift_};
+    // choose::earliest disambiguates the DST fall-back hour (ambiguous local
+    // time) and skips forward over the spring-forward gap (nonexistent local
+    // time); without it this constructor throws and aborts Waybar every minute
+    // during a DST transition. Fixes #2615 (and its many duplicates).
     const zoned_time shiftedNow{
-        tz, local_days(shiftedDay) + (now.get_local_time() - floor<days>(now.get_local_time()))};
+        tz, local_days(shiftedDay) + (now.get_local_time() - floor<days>(now.get_local_time())),
+        choose::earliest};
 
     if (tzInTooltip_) tzText_ = getTZtext(now.get_sys_time());
     if (cldInTooltip_) cldText_ = get_calendar(today, shiftedDay, tz);
@@ -441,9 +446,10 @@ auto waybar::modules::Clock::get_calendar(const year_month_day& today, const yea
                           fmt_lib::make_format_args(
                               (line == 2)
                                   ? static_cast<const zoned_seconds&&>(
-                                        zoned_seconds{tz, local_days{ymTmp / 1}})
+                                        zoned_seconds{tz, local_days{ymTmp / 1}, choose::earliest})
                                   : static_cast<const zoned_seconds&&>(zoned_seconds{
-                                        tz, local_days{cldGetWeekForLine(ymTmp, firstdow, line)}})))
+                                        tz, local_days{cldGetWeekForLine(ymTmp, firstdow, line)},
+                                        choose::earliest})))
                    << ' ';
               } else {
                 os << pads;
@@ -482,11 +488,12 @@ auto waybar::modules::Clock::get_calendar(const year_month_day& today, const yea
                    << fmt_lib::vformat(
                           m_locale_, fmtMap_[4],
                           fmt_lib::make_format_args(
-                              (line == 2) ? static_cast<const zoned_seconds&&>(
+                              (line == 2)
-                                                zoned_seconds{tz, local_days{ymTmp / 1}})
+                                  ? static_cast<const zoned_seconds&&>(
-                                          : static_cast<const zoned_seconds&&>(
+                                        zoned_seconds{tz, local_days{ymTmp / 1}, choose::earliest})
-                                                zoned_seconds{tz, local_days{cldGetWeekForLine(
+                                  : static_cast<const zoned_seconds&&>(zoned_seconds{
-                                                                      ymTmp, firstdow, line)}})));
+                                        tz, local_days{cldGetWeekForLine(ymTmp, firstdow, line)},
+                                        choose::earliest})));
               else
                 os << pads;
             }

src/modules/custom.cpp

@@ -105,7 +105,7 @@ void waybar::modules::Custom::continuousWorker() {
         dp.emit();
         spdlog::error("{} stopped unexpectedly, is it endless?", name_);
       }
-      if (config_["restart-interval"].isNumeric()) {
+      if (config_["restart-interval"].isNumeric() && config_["restart-interval"].asDouble() > 0) {
         pid_ = -1;
         thread_.sleep_for(std::chrono::milliseconds(
             std::max(1L,  // Minimum 1ms due to millisecond precision
@@ -115,6 +115,8 @@ void waybar::modules::Custom::continuousWorker() {
           throw std::runtime_error("Unable to open " + cmd);
         }
       } else {
+        // A non-positive restart-interval must not busy-respawn the script
+        // (that starves the GTK main loop); treat it as "do not restart".
         thread_.stop();
         return;
       }
@@ -316,22 +318,32 @@ void waybar::modules::Custom::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/custom_graph.cpp

@@ -139,7 +139,7 @@ void waybar::modules::CustomGraph::waitingWorker() {
 }
 
 void waybar::modules::CustomGraph::refresh(int sig) {
-  if (sig == SIGRTMIN + config_["signal"].asInt()) {
+  if (config_["signal"].isInt() && sig == SIGRTMIN + config_["signal"].asInt()) {
     thread_.wake_up();
   }
 }

src/modules/hyprland/language.cpp

@@ -38,10 +38,18 @@ auto Language::update() -> void {
   std::string layoutName = std::string{};
   if (config_.isMember("format-" + layout_.short_description + "-" + layout_.variant)) {
     const auto propName = "format-" + layout_.short_description + "-" + layout_.variant;
-    layoutName = fmt::format(fmt::runtime(format_), config_[propName].asString());
+    layoutName =
+        trim(fmt::format(fmt::runtime(format_), config_[propName].asString(),
+                         fmt::arg("long", layout_.full_name), fmt::arg("short", layout_.short_name),
+                         fmt::arg("shortDescription", layout_.short_description),
+                         fmt::arg("variant", layout_.variant)));
   } else if (config_.isMember("format-" + layout_.short_description)) {
     const auto propName = "format-" + layout_.short_description;
-    layoutName = fmt::format(fmt::runtime(format_), config_[propName].asString());
+    layoutName =
+        trim(fmt::format(fmt::runtime(format_), config_[propName].asString(),
+                         fmt::arg("long", layout_.full_name), fmt::arg("short", layout_.short_name),
+                         fmt::arg("shortDescription", layout_.short_description),
+                         fmt::arg("variant", layout_.variant)));
   } else {
     layoutName = trim(fmt::format(fmt::runtime(format_), fmt::arg("long", layout_.full_name),
                                   fmt::arg("short", layout_.short_name),

src/modules/keyboard_state.cpp

@@ -268,10 +268,16 @@ waybar::modules::KeyboardState::KeyboardState(const std::string& id, const Bar&
           std::lock_guard<std::mutex> lock(devices_mutex_);
           auto it = libinput_devices_.find(dev_path);
           if (it != libinput_devices_.end()) {
-            spdlog::info("Keyboard {} has been removed.", dev_path);
+            struct libinput_device* device = it->second;
-            libinput_path_remove_device(it->second);
+            // Erase from the map first so that a second IN_DELETE event for the
-            libinput_device_unref(it->second);
+            // same path becomes a no-op. This keeps removal idempotent and
+            // ensures libinput_path_remove_device()/libinput_device_unref() are
+            // called exactly once per device, avoiding a libinput list_remove
+            // assertion abort on double removal.
             libinput_devices_.erase(it);
+            spdlog::info("Keyboard {} has been removed.", dev_path);
+            libinput_path_remove_device(device);
+            libinput_device_unref(device);
           }
         }
         i += sizeof(struct inotify_event) + event->len;

src/modules/niri/backend.cpp

@@ -23,6 +23,8 @@ namespace waybar::modules::niri {
 
 IPC::IPC() { startIPC(); }
 
+IPC::~IPC() { running_ = false; }
+
 int IPC::connectToSocket() {
   const char* socket_path = getenv("NIRI_SOCKET");
 
@@ -55,40 +57,60 @@ int IPC::connectToSocket() {
 void IPC::startIPC() {
   // will start IPC and relay events to parseIPC
 
-  int socketfd = connectToSocket();
+  std::thread([this]() {
-
-  std::thread([this, socketfd]() {
     spdlog::info("Niri IPC starting");
 
-    auto unix_istream = Gio::UnixInputStream::create(socketfd, true);
+    // Reconnect loop: if the event stream drops we back off briefly and
-    auto unix_ostream = Gio::UnixOutputStream::create(socketfd, false);
+    // re-establish the socket instead of leaving the module frozen forever.
-    auto istream = Gio::DataInputStream::create(unix_istream);
+    while (running_) {
-    auto ostream = Gio::DataOutputStream::create(unix_ostream);
+      int socketfd;
-
-    if (!ostream->put_string("\"EventStream\"\n") || !ostream->flush()) {
-      spdlog::error("Niri IPC: failed to start event stream");
-      return;
-    }
-
-    std::string line;
-    if (!istream->read_line(line) || line != R"({"Ok":"Handled"})") {
-      spdlog::error("Niri IPC: failed to start event stream");
-      return;
-    }
-
-    while (istream->read_line(line)) {
-      spdlog::debug("Niri IPC: received {}", line);
-
       try {
-        parseIPC(line);
+        socketfd = connectToSocket();
       } catch (std::exception& e) {
-        spdlog::warn("Failed to parse IPC message: {}, reason: {}", line, e.what());
+        spdlog::error("Niri IPC: failed to connect: {}", e.what());
-      } catch (...) {
+        std::this_thread::sleep_for(std::chrono::seconds(2));
-        throw;
+        continue;
       }
 
-      std::this_thread::sleep_for(std::chrono::milliseconds(1));
+      auto unix_istream = Gio::UnixInputStream::create(socketfd, true);
+      auto unix_ostream = Gio::UnixOutputStream::create(socketfd, false);
+      auto istream = Gio::DataInputStream::create(unix_istream);
+      auto ostream = Gio::DataOutputStream::create(unix_ostream);
+
+      if (!ostream->put_string("\"EventStream\"\n") || !ostream->flush()) {
+        spdlog::error("Niri IPC: failed to start event stream");
+        std::this_thread::sleep_for(std::chrono::seconds(2));
+        continue;
+      }
+
+      std::string line;
+      if (!istream->read_line(line) || line != R"({"Ok":"Handled"})") {
+        spdlog::error("Niri IPC: failed to start event stream");
+        std::this_thread::sleep_for(std::chrono::seconds(2));
+        continue;
+      }
+
+      // Drain events as fast as they arrive; throttling here back-pressures the
+      // socket, fills niri's send buffer and makes niri drop the stream.
+      while (running_ && istream->read_line(line)) {
+        spdlog::debug("Niri IPC: received {}", line);
+
+        try {
+          parseIPC(line);
+        } catch (std::exception& e) {
+          spdlog::warn("Failed to parse IPC message: {}, reason: {}", line, e.what());
+        } catch (...) {
+          throw;
+        }
+      }
+
+      if (!running_) break;
+
+      spdlog::warn("Niri IPC: event stream closed, reconnecting");
+      std::this_thread::sleep_for(std::chrono::seconds(2));
     }
+
+    spdlog::info("Niri IPC stopping");
   }).detach();
 }
 
@@ -196,12 +218,12 @@ void IPC::parseIPC(const std::string& line) {
       for (auto& win : windows_) {
         win["is_focused"] = focused && win["id"].asUInt64() == id;
       }
-    } else if (const auto &payload = ev["WindowLayoutsChanged"]) {
+    } else if (const auto& payload = ev["WindowLayoutsChanged"]) {
-      const auto &values = payload["changes"];
+      const auto& values = payload["changes"];
-      for (const auto &changed : values) {
+      for (const auto& changed : values) {
         const auto id = changed[0].asUInt64();
-        const auto &change = changed[1];
+        const auto& change = changed[1];
-        for (auto &win : windows_) {
+        for (auto& win : windows_) {
           if (win["id"].asUInt64() == id) {
             win["layout"] = change;
             break;

src/modules/sway/language.cpp

@@ -116,6 +116,17 @@ void Language::onEvent(const struct Ipc::ipc_response& res) {
 
 auto Language::update() -> void {
   std::lock_guard<std::mutex> lock(mutex_);
+  // Apply the CSS class here, on the GTK main thread. set_current_layout() runs on the IPC worker
+  // thread, so mutating label_'s style context there would crash (#3702).
+  if (layout_.short_name != applied_class_) {
+    if (!applied_class_.empty()) {
+      label_.get_style_context()->remove_class(applied_class_);
+    }
+    if (!layout_.short_name.empty()) {
+      label_.get_style_context()->add_class(layout_.short_name);
+    }
+    applied_class_ = layout_.short_name;
+  }
   if (hide_single_ && layouts_map_.size() <= 1) {
     event_box_.hide();
     return;
@@ -145,6 +156,10 @@ auto Language::update() -> void {
 }
 
 auto Language::set_current_layout(const std::string& current_layout) -> void {
+  // Runs on the IPC worker thread (via onEvent) as well as the main thread (via onCmd), so it must
+  // not touch GTK widgets - off-main-thread widget mutation caused SIGSEGV (#3702). Only record the
+  // target layout here; update() applies the matching CSS class on the main thread.
+  //
   // Guard against unknown / empty layout names: transient virtual keyboards (e.g. wtype) and
   // hot-plugged devices whose layouts haven't made it into the map yet would otherwise blank out
   // layout_ via map::operator[]'s default-construct-on-miss.
@@ -152,9 +167,7 @@ auto Language::set_current_layout(const std::string& current_layout) -> void {
   if (it == layouts_map_.end()) {
     return;
   }
-  label_.get_style_context()->remove_class(layout_.short_name);
   layout_ = it->second;
-  label_.get_style_context()->add_class(layout_.short_name);
 }
 
 auto Language::init_layouts_map(const std::vector<std::string>& used_layouts) -> void {

src/modules/sway/window.cpp

@@ -43,7 +43,9 @@ void Window::onCmd(const struct Ipc::ipc_response& res) {
     auto output = payload["output"].isString() ? payload["output"].asString() : "";
     std::tie(app_nb_, floating_count_, windowId_, window_, app_id_, app_class_, shell_, layout_,
              marks_) = getFocusedNode(payload["nodes"], output);
-    updateAppIconName(app_id_, app_class_);
+    // Do not resolve the app icon here: onCmd runs on the sway IPC worker thread and
+    // updateAppIconName() touches the global Gtk::IconTheme cache, which is not thread-safe.
+    // The icon is resolved in update() on the main thread instead (triggered by dp.emit()).
     dp.emit();
   } catch (const std::exception& e) {
     spdlog::error("Window: {}", e.what());
@@ -102,6 +104,9 @@ auto Window::update() -> void {
     setTooltipMarkup(window_);
   }
 
+  // Resolve the app icon on the main thread to avoid racing with GTK draw on the
+  // global Gtk::IconTheme cache (see onCmd).
+  updateAppIconName(app_id_, app_class_);
   updateAppIcon();
 
   // Call parent update

src/util/audio_backend.cpp

@@ -74,6 +74,22 @@ void AudioBackend::connectContext() {
   }
 }
 
+// Reconnect the context without ever throwing. This is safe to call from within
+// a PulseAudio state callback (which runs in pure C libpulse frames), where an
+// escaping C++ exception cannot be unwound and would abort the process.
+bool AudioBackend::reconnectContext() noexcept {
+  try {
+    connectContext();
+    return true;
+  } catch (const std::exception& e) {
+    spdlog::error("PulseAudio reconnect failed: {}", e.what());
+    return false;
+  } catch (...) {
+    spdlog::error("PulseAudio reconnect failed: unknown error");
+    return false;
+  }
+}
+
 void AudioBackend::contextStateCb(pa_context* c, void* data) {
   auto* backend = static_cast<AudioBackend*>(data);
   switch (pa_context_get_state(c)) {
@@ -104,12 +120,29 @@ void AudioBackend::contextStateCb(pa_context* c, void* data) {
         // When pulseaudio server restarts, the connection is "failed". Try to reconnect.
         // pa_threaded_mainloop_lock is already acquired in callback threads.
         // So there is no need to lock it again.
+        //
+        // Guard against re-entrancy: pa_context_connect() can fire this callback
+        // synchronously with PA_CONTEXT_FAILED again, which would otherwise
+        // recurse (FAILED -> connect -> FAILED -> ...) and busy-loop.
+        if (backend->reconnecting_) {
+          break;
+        }
         if (backend->context_ != nullptr) {
           pa_context_disconnect(backend->context_);
           pa_context_unref(backend->context_);
           backend->context_ = nullptr;
         }
-        backend->connectContext();
+        backend->reconnecting_ = true;
+        // Never throw across the libpulse C callback boundary: a failed reconnect
+        // is logged and left for a later PA event to retry instead of aborting.
+        if (!backend->reconnectContext()) {
+          spdlog::warn("PulseAudio context reconnect failed; will retry on next event");
+          if (backend->context_ != nullptr) {
+            pa_context_unref(backend->context_);
+            backend->context_ = nullptr;
+          }
+        }
+        backend->reconnecting_ = false;
       }
       break;
     case PA_CONTEXT_CONNECTING: