diff --git a/include/group.hpp b/include/group.hpp index 0e981d0d..1e2a5d55 100644 --- a/include/group.hpp +++ b/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; diff --git a/include/modules/hyprland/workspaces.hpp b/include/modules/hyprland/workspaces.hpp index 4be7fd44..1237ef24 100644 --- a/include/modules/hyprland/workspaces.hpp +++ b/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 diff --git a/include/modules/mango/backend.hpp b/include/modules/mango/backend.hpp index a1b0392e..7e4313f3 100644 --- a/include/modules/mango/backend.hpp +++ b/include/modules/mango/backend.hpp @@ -1,6 +1,7 @@ // include/modules/mango/backend.hpp #pragma once +#include #include #include #include @@ -52,6 +53,7 @@ class IPC { static Json::Value sendCommand(const std::string& cmd); + std::atomic running_ = true; int sockfd_ = -1; std::thread ipc_thread_; mutable std::mutex data_mutex_; diff --git a/include/modules/sni/host.hpp b/include/modules/sni/host.hpp index 49771fac..4dad1f94 100644 --- a/include/modules/sni/host.hpp +++ b/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&, const std::function&)>&, - const std::function&)>&, const std::function&); + const std::function&)>&, const std::function&, + const std::function&); ~Host(); void checkIgnoreList(const std::vector& ignore_list, @@ -55,6 +56,10 @@ class Host { const std::vector ignore_list_; const std::function&)> on_add_; const std::function&)> 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 on_reorder_; ItemOrderMap orders_; const std::function on_update_; diff --git a/include/modules/sni/tray.hpp b/include/modules/sni/tray.hpp index a2fd08eb..c3ac4e34 100644 --- a/include/modules/sni/tray.hpp +++ b/include/modules/sni/tray.hpp @@ -1,6 +1,10 @@ #pragma once #include +#include + +#include +#include #include "AModule.hpp" #include "bar.hpp" @@ -19,6 +23,9 @@ class Tray : public AModule { private: void onAdd(std::unique_ptr& item); void onRemove(std::unique_ptr& 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); std::vector parseIgnoreList(const Json::Value& config); void queueUpdate(); @@ -29,6 +36,9 @@ class Tray : public AModule { std::vector ignore_list_; SNI::Host host_; std::vector 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_connections_; }; } // namespace waybar::modules::SNI diff --git a/src/AModule.cpp b/src/AModule.cpp index 83bafbcf..65ffb433 100644 --- a/src/AModule.cpp +++ b/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 = - fmt::format(fmt::runtime(format), fmt::arg("x", (int)round(100. * e->x / width)), - fmt::arg("y", (int)round(100. * e->y / height))); + // Substitute {x}/{y} with the click position. The configured command is + // 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(); diff --git a/src/bar.cpp b/src/bar.cpp index cf284e0d..1eb5ce41 100644 --- a/src/bar.cpp +++ b/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] { - window.queue_resize(); - window.queue_draw(); - forceLayerCommit(); - return false; - }, *this)); + Glib::signal_idle().connect(sigc::track_obj( + [this] { + window.queue_resize(); + window.queue_draw(); + forceLayerCommit(); + return false; + }, + *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,20 +752,18 @@ void waybar::Bar::onOutputGeometryChanged() { } void waybar::Bar::toggleSuspend(bool suspend) { - auto process_modules = [suspend](Gtk::Box& module_box) { - for (auto* widget : module_box.get_children()) { - auto* module = dynamic_cast(widget); - if (module && module->shouldSuspend()) { - if (suspend) { - module->suspend(); - } else { - module->resume(); - } + // Iterate the actual module objects. Modules are packed into the Gtk::Box via + // AModule::operator Gtk::Widget&(), which returns the member event_box_, so the + // 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(); + } else { + module->resume(); } } - }; - - process_modules(left_); - process_modules(center_); - process_modules(right_); + } } diff --git a/src/config.cpp b/src/config.cpp index ad136339..800b6b02 100644 --- a/src/config.cpp +++ b/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(" "); - std::string dimension = str.substr(0, i); - str = str.substr(i + 1); - i = str.find(" "); - std::string comparator = str.substr(0, i); - int value = std::stoi(str.substr(i)); + auto first_space = str.find(' '); + if (first_space == std::string::npos) { + spdlog::warn( + "Ignoring malformed 'output-dimensions' entry (expected ' " + "'): '{}'", + 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 ' " + "'): '{}'", + 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") { diff --git a/src/group.cpp b/src/group.cpp index 9f11661f..5fa04884 100644 --- a/src/group.cpp +++ b/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); diff --git a/src/modules/clock.cpp b/src/modules/clock.cpp index 38dec4b2..bd0c828d 100644 --- a/src/modules/clock.cpp +++ b/src/modules/clock.cpp @@ -220,20 +220,35 @@ 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); - if (tzInTooltip_ || cldInTooltip_ || ordInTooltip_) { - // std::vformat doesn't support named arguments. - m_tlpText_ = - std::regex_replace(m_tlpFmt_, std::regex("\\{" + kTZPlaceholder + "\\}"), tzText_); - m_tlpText_ = std::regex_replace( - m_tlpText_, std::regex("\\{" + kCldPlaceholder + "\\}"), - fmt_lib::vformat(m_locale_, cldText_, fmt_lib::make_format_args(shiftedNow))); - m_tlpText_ = - std::regex_replace(m_tlpText_, std::regex("\\{" + kOrdPlaceholder + "\\}"), ordText_); - } else { - m_tlpText_ = m_tlpFmt_; - } + try { + if (tzInTooltip_ || cldInTooltip_ || ordInTooltip_) { + // std::vformat doesn't support named arguments. + m_tlpText_ = + std::regex_replace(m_tlpFmt_, std::regex("\\{" + kTZPlaceholder + "\\}"), tzText_); + m_tlpText_ = std::regex_replace( + m_tlpText_, std::regex("\\{" + kCldPlaceholder + "\\}"), + fmt_lib::vformat(m_locale_, cldText_, fmt_lib::make_format_args(shiftedNow))); + m_tlpText_ = + std::regex_replace(m_tlpText_, std::regex("\\{" + kOrdPlaceholder + "\\}"), ordText_); + } else { + m_tlpText_ = m_tlpFmt_; + } - m_tlpText_ = fmt_lib::vformat(m_locale_, m_tlpText_, fmt_lib::make_format_args(now)); + 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 diff --git a/src/modules/custom_graph.cpp b/src/modules/custom_graph.cpp index 8b0c70af..06e45d06 100644 --- a/src/modules/custom_graph.cpp +++ b/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()) { - 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/hyprland/backend.cpp b/src/modules/hyprland/backend.cpp index f38f8216..891bed1a 100644 --- a/src/modules/hyprland/backend.cpp +++ b/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. - // In Lua-based Hyprland (>= 0.54) the error contains "hl.dispatch". - // In older versions it returns "ok" or a different error. - auto reply = getSocket1Reply("dispatch workspace __waybar_probe__"); - bool luaProto = reply.find("hl.dispatch") != std::string::npos; + // Detect the Lua-based dispatch protocol (Hyprland >= 0.54) via the read-only + // "version" query. This MUST have no side effects: an earlier probe issued a real + // "dispatch workspace __waybar_probe__", which on Hyprland < 0.54 actually switched + // the user to a junk workspace named __waybar_probe__ on the first click/scroll. + 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)"); diff --git a/src/modules/hyprland/workspaces.cpp b/src/modules/hyprland/workspaces.cpp index 4cd0d2ae..202b892b 100644 --- a/src/modules/hyprland/workspaces.cpp +++ b/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 lg(m_mutex); @@ -332,23 +337,11 @@ void Workspaces::onEvent(const std::string& ev) { } } - if (m_debounceTimer.connected()) { - m_debounceTimer.disconnect(); - m_updatePending = false; - } - - m_updatePending = true; - m_debounceTimer = Glib::signal_timeout().connect( - [this]() { - if (!m_updatePending) return false; - std::lock_guard lock(m_mutex); - if (m_updatePending) { - dp.emit(); - m_updatePending = false; - } - return false; - }, - 7); + // Notify the main thread. dp (Glib::Dispatcher) is the only thread-safe way to + // hand off to the GTK main loop; GLib timer state must never be touched from the + // IPC listener thread. The debounce timer is owned entirely by the main-thread + // update() path (see Workspaces::update). + dp.emit(); } void Workspaces::onWorkspaceActivated(std::string const& payload) { @@ -1041,8 +1034,20 @@ void Workspaces::setUrgentWorkspace(std::string const& windowaddress) { } auto Workspaces::update() -> void { - doUpdate(); - AModule::update(); + // 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() { diff --git a/src/modules/keyboard_state.cpp b/src/modules/keyboard_state.cpp index 87a843c4..fa80ef64 100644 --- a/src/modules/keyboard_state.cpp +++ b/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 lock(devices_mutex_); diff --git a/src/modules/mango/backend.cpp b/src/modules/mango/backend.cpp index 5083a7fc..63aecb7b 100644 --- a/src/modules/mango/backend.cpp +++ b/src/modules/mango/backend.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -100,71 +101,121 @@ 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; - pfd.fd = sockfd_; - pfd.events = POLLIN; - - const std::vector 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; - } - } - char buf[4096]; std::string buffer; - while (true) { - int ret = poll(&pfd, 1, 1000); - if (ret == 0) continue; - if (ret < 0) { - if (errno == EINTR) continue; - spdlog::error("IPC poll error: {}", strerror(errno)); - break; - } + bool have_initial_fd = true; - if (pfd.revents & (POLLERR | POLLHUP | POLLNVAL)) { - spdlog::info("Mango IPC socket closed or invalid"); - break; + // 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; - if (pfd.revents & POLLIN) { - ssize_t n = read(sockfd_, buf, sizeof(buf)); - if (n == 0) { - spdlog::info("Mango IPC connection closed"); + bool subscribed = true; + const std::vector 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); + subscribed = false; break; } - if (n < 0) { + } + if (!subscribed) { + if (sockfd_ != -1) { + close(sockfd_); + sockfd_ = -1; + } + std::this_thread::sleep_for(std::chrono::seconds(2)); + continue; + } + + struct pollfd pfd; + pfd.fd = sockfd_; + 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 read error: {}", strerror(errno)); + spdlog::error("IPC poll error: {}", strerror(errno)); + connected = false; break; } - buffer.append(buf, n); - size_t pos; - while ((pos = buffer.find('\n')) != std::string::npos) { - std::string line = buffer.substr(0, pos); - buffer.erase(0, pos + 1); - if (line.empty()) continue; - try { - parseIPC(line); - } catch (const std::exception& e) { - spdlog::warn("Failed to parse IPC line: {} - {}", line, e.what()); + if (pfd.revents & (POLLERR | POLLHUP | POLLNVAL)) { + spdlog::info("Mango IPC socket closed or invalid"); + connected = false; + break; + } + + if (pfd.revents & POLLIN) { + 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); + + size_t pos; + while ((pos = buffer.find('\n')) != std::string::npos) { + std::string line = buffer.substr(0, pos); + buffer.erase(0, pos + 1); + if (line.empty()) continue; + try { + parseIPC(line); + } catch (const std::exception& e) { + spdlog::warn("Failed to parse IPC line: {} - {}", line, e.what()); + } } } } + + // 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"); }); } diff --git a/src/modules/mango/language.cpp b/src/modules/mango/language.cpp index 9415002a..549d22ed 100644 --- a/src/modules/mango/language.cpp +++ b/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; } diff --git a/src/modules/mpris/mpris.cpp b/src/modules/mpris/mpris.cpp index 387e0597..d70f5902 100644 --- a/src/modules/mpris/mpris.cpp +++ b/src/modules/mpris/mpris.cpp @@ -491,7 +491,13 @@ auto Mpris::getPlayerInfo() -> std::optional { 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; diff --git a/src/modules/network.cpp b/src/modules/network.cpp index 7b4dc15f..72a367fc 100644 --- a/src/modules/network.cpp +++ b/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//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(speed); } waybar::modules::Network::Network(const std::string& id, const Json::Value& config) diff --git a/src/modules/niri/backend.cpp b/src/modules/niri/backend.cpp index 460754cc..19eeada0 100644 --- a/src/modules/niri/backend.cpp +++ b/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); diff --git a/src/modules/sni/host.cpp b/src/modules/sni/host.cpp index 272e05aa..c55c8f2e 100644 --- a/src/modules/sni/host.cpp +++ b/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& ignore_list, const std::function&)>& on_add, const std::function&)>& on_remove, - const std::function& on_update) + const std::function& on_reorder, const std::function& 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_); - std::ranges::sort(items_, [](std::unique_ptr& item1, std::unique_ptr& item2) { - return item1->order_ < item2->order_; - }); - std::ranges::for_each(items_, on_add_); + // Re-apply the configured ordering to the tray. This is invoked while an + // item's Id/order is first resolved (from Item::setCustomIcon), which happens + // *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 + // 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 diff --git a/src/modules/sni/tray.cpp b/src/modules/sni/tray.cpp index f3e413a8..285c103f 100644 --- a/src/modules/sni/tray.cpp +++ b/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) { 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) { } items_.push_back(item.get()); - item->event_box.signal_show().connect([this] { dp.emit(); }); - item->event_box.signal_hide().connect([this] { dp.emit(); }); + auto show_conn = item->event_box.signal_show().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) { } void Tray::onRemove(std::unique_ptr& 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(items_.size() - 1 - i) : static_cast(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()) { diff --git a/src/modules/sway/window.cpp b/src/modules/sway/window.cpp index f005bd14..4faa7d77 100644 --- a/src/modules/sway/window.cpp +++ b/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 diff --git a/src/modules/systemd_failed_units.cpp b/src/modules/systemd_failed_units.cpp index faa1ca7e..3804cbe2 100644 --- a/src/modules/systemd_failed_units.cpp +++ b/src/modules/systemd_failed_units.cpp @@ -281,21 +281,42 @@ auto SystemdFailedUnits::update() -> void { last_status_ = overall_state_; - 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("system_state", system_state_), fmt::arg("user_state", user_state_), - fmt::arg("overall_state", 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("system_state", system_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()) { - 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))); + 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(""); } diff --git a/src/modules/wwan.cpp b/src/modules/wwan.cpp index 86e68624..c741405d 100644 --- a/src/modules/wwan.cpp +++ b/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_object_unref(manager); - g_object_unref(connection); + g_clear_object(¤t_modem); + g_clear_object(&manager); + g_clear_object(&connection); }