diff --git a/include/modules/niri/backend.hpp b/include/modules/niri/backend.hpp index 07be039a..acbbf52e 100644 --- a/include/modules/niri/backend.hpp +++ b/include/modules/niri/backend.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -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> callbacks_; + + std::atomic running_{true}; }; inline std::unique_ptr gIPC; diff --git a/include/modules/sway/language.hpp b/include/modules/sway/language.hpp index 470b9879..6ababa4f 100644 --- a/include/modules/sway/language.hpp +++ b/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 layouts_map_; bool hide_single_; diff --git a/include/util/audio_backend.hpp b/include/util/audio_backend.hpp index ec732e1e..2086e868 100644 --- a/include/util/audio_backend.hpp +++ b/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_; diff --git a/src/ALabel.cpp b/src/ALabel.cpp index f7a98329..84ac2052 100644 --- a/src/ALabel.cpp +++ b/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 - static_cast(config_["interval"].asDouble() * 1000)) + ? (config_["interval"].asDouble() > 0 + // Minimum 1ms due to millisecond precision + ? std::max(1L, static_cast( + 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(interval))) : 1000 * (long)interval))), default_format_(format_) { label_.set_name(name); diff --git a/src/client.cpp b/src/client.cpp index 57e0fe5c..511e35af 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -211,7 +211,7 @@ const std::string waybar::Client::getStyle(const std::string& style, if (style.empty()) { std::vector 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(); + try { + portal = std::make_unique(); + } 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(m_cssFile, [&](const std::string& css_file) { setupCss(css_file); }); - portal->signal_appearance_changed().connect([&](waybar::Appearance appearance) { - auto css_file = getStyle(style_opt, appearance); - m_cssReloadHelper->changeCssFile(css_file); - setupCss(css_file); - }); + if (portal) { + portal->signal_appearance_changed().connect([&](waybar::Appearance appearance) { + auto css_file = getStyle(style_opt, appearance); + 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(); + } } diff --git a/src/modules/clock.cpp b/src/modules/clock.cpp index 016d1d75..095ac403 100644 --- a/src/modules/clock.cpp +++ b/src/modules/clock.cpp @@ -190,8 +190,13 @@ auto waybar::modules::Clock::update() -> void { if (tooltipEnabled()) { const year_month_day today{floor(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(now.get_local_time()))}; + tz, local_days(shiftedDay) + (now.get_local_time() - floor(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( - zoned_seconds{tz, local_days{ymTmp / 1}}) + zoned_seconds{tz, local_days{ymTmp / 1}, choose::earliest}) : static_cast(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( - zoned_seconds{tz, local_days{ymTmp / 1}}) - : static_cast( - zoned_seconds{tz, local_days{cldGetWeekForLine( - ymTmp, firstdow, line)}}))); + (line == 2) + ? static_cast( + zoned_seconds{tz, local_days{ymTmp / 1}, choose::earliest}) + : static_cast(zoned_seconds{ + tz, local_days{cldGetWeekForLine(ymTmp, firstdow, line)}, + choose::earliest}))); else os << pads; } diff --git a/src/modules/custom.cpp b/src/modules/custom.cpp index 01781036..83f36797 100644 --- a/src/modules/custom.cpp +++ b/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()) { - text_ = Glib::Markup::escape_text(parsed["text"].asString()); + const bool escape = config_["escape"].isBool() && config_["escape"].asBool(); + 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()) { - alt_ = Glib::Markup::escape_text(parsed["alt"].asString()); + if (escape) { + 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()) { - tooltip_ = Glib::Markup::escape_text(parsed["tooltip"].asString()); + if (escape) { + 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()); diff --git a/src/modules/custom_graph.cpp b/src/modules/custom_graph.cpp index 0f236304..8b0c70af 100644 --- a/src/modules/custom_graph.cpp +++ b/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(); } } diff --git a/src/modules/hyprland/language.cpp b/src/modules/hyprland/language.cpp index 9c082d3a..85adaee0 100644 --- a/src/modules/hyprland/language.cpp +++ b/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), diff --git a/src/modules/keyboard_state.cpp b/src/modules/keyboard_state.cpp index 7e43c74c..458cf0af 100644 --- a/src/modules/keyboard_state.cpp +++ b/src/modules/keyboard_state.cpp @@ -268,10 +268,16 @@ waybar::modules::KeyboardState::KeyboardState(const std::string& id, const Bar& std::lock_guard lock(devices_mutex_); auto it = libinput_devices_.find(dev_path); if (it != libinput_devices_.end()) { - spdlog::info("Keyboard {} has been removed.", dev_path); - libinput_path_remove_device(it->second); - libinput_device_unref(it->second); + struct libinput_device* device = it->second; + // Erase from the map first so that a second IN_DELETE event for the + // 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; diff --git a/src/modules/niri/backend.cpp b/src/modules/niri/backend.cpp index 0900e092..245bf2de 100644 --- a/src/modules/niri/backend.cpp +++ b/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, socketfd]() { + std::thread([this]() { spdlog::info("Niri IPC starting"); - 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"); - 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); - + // Reconnect loop: if the event stream drops we back off briefly and + // re-establish the socket instead of leaving the module frozen forever. + while (running_) { + int socketfd; try { - parseIPC(line); + socketfd = connectToSocket(); } catch (std::exception& e) { - spdlog::warn("Failed to parse IPC message: {}, reason: {}", line, e.what()); - } catch (...) { - throw; + spdlog::error("Niri IPC: failed to connect: {}", e.what()); + std::this_thread::sleep_for(std::chrono::seconds(2)); + 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"]) { - const auto &values = payload["changes"]; - for (const auto &changed : values) { + } else if (const auto& payload = ev["WindowLayoutsChanged"]) { + const auto& values = payload["changes"]; + for (const auto& changed : values) { const auto id = changed[0].asUInt64(); - const auto &change = changed[1]; - for (auto &win : windows_) { + const auto& change = changed[1]; + for (auto& win : windows_) { if (win["id"].asUInt64() == id) { win["layout"] = change; break; diff --git a/src/modules/sway/language.cpp b/src/modules/sway/language.cpp index 7113d9ef..c284e53c 100644 --- a/src/modules/sway/language.cpp +++ b/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 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& used_layouts) -> void { diff --git a/src/modules/sway/window.cpp b/src/modules/sway/window.cpp index 2b62da14..f005bd14 100644 --- a/src/modules/sway/window.cpp +++ b/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 diff --git a/src/util/audio_backend.cpp b/src/util/audio_backend.cpp index 482d7886..25ccde37 100644 --- a/src/util/audio_backend.cpp +++ b/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(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: