From b345abdd0a8d1e733b60eb44af2b7d7d56a9dcca Mon Sep 17 00:00:00 2001
From: Thayne McCombs <astrothayne@gmail.com>
Date: Fri, 10 Jan 2025 01:32:31 -0700
Subject: [PATCH] fix: Make signal handling safer

std::signal is very restrictive about what can be done inside the signal
handler. Posix adds some additional functions that are safe to call, but
it is still pretty limited.

This changes waybar to use the "self pipe trick" where all the signal
handler does is write a byte into a pipe, and then we have a slot run on
the Gtk main loop that reads from that pipe and actually handles the
signal.
---
 src/main.cpp | 101 ++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 79 insertions(+), 22 deletions(-)

diff --git a/src/main.cpp b/src/main.cpp
index 045b2cd49..12fd64d1f 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -1,7 +1,9 @@
 #include <spdlog/spdlog.h>
 #include <sys/types.h>
 #include <sys/wait.h>
+#include <unistd.h>
 
+#include <cerrno>
 #include <csignal>
 #include <list>
 #include <mutex>
@@ -70,35 +72,90 @@ void startSignalThread() {
   }
 }
 
+static int signal_fds[2];
+
+extern "C" void raw_signal_handler(int signum) {
+  // just write a single byte to be more reliable
+  auto sigbyte = static_cast<unsigned char>(signum);
+  // The only way I know of that this could fail is if we have queued a truly
+  // remarkable number of signals without handling them
+  write(signal_fds[1], &sigbyte, 1);
+}
+
+static void dispatch_signal(unsigned char signum) {
+  auto client = waybar::Client::inst();
+  if (client == nullptr) {
+    // TODO: should we do something for SIGINT?
+    return;
+  }
+  if (signum == SIGUSR1) {
+    for (auto& bar : client->bars) {
+      bar->toggle();
+    }
+  } else if (signum == SIGUSR2) {
+    spdlog::info("Reloading...");
+    reload = true;
+    client->reset();
+  } else if (signum == SIGINT) {
+    spdlog::info("Quitting.");
+    reload = false;
+    client->reset();
+  } else if (signum > SIGRTMIN && signum <= SIGRTMAX) {
+    for (auto& bar : client->bars) {
+      bar->handleSignal(signum);
+    }
+  }
+}
+
+static bool handle_signals(Glib::IOCondition cond) {
+  unsigned char buf[16];
+  const size_t bufsize = sizeof(buf);
+  while (true) {
+    auto n = read(signal_fds[0], buf, bufsize);
+    if (n < 0) {
+      if (errno == EAGAIN) {
+        return true;
+      }
+      throw std::system_error(errno, std::system_category());
+    }
+    for (int i = 0; i < n; i++) {
+      dispatch_signal(buf[i]);
+    }
+    if (static_cast<size_t>(n) < bufsize) {
+      return true;
+    }
+  }
+}
+
 int main(int argc, char* argv[]) {
   try {
     auto* client = waybar::Client::inst();
 
-    std::signal(SIGUSR1, [](int /*signal*/) {
-      for (auto& bar : waybar::Client::inst()->bars) {
-        bar->toggle();
-      }
-    });
+    // It would be nice if we could use g_unix_signal_add, but unfortunately, that
+    // doesn't support RT signals
+    if (pipe(signal_fds)) {
+      throw std::runtime_error("Failed to create pipe");
+    }
+    std::signal(SIGUSR1, &raw_signal_handler);
+    std::signal(SIGUSR2, &raw_signal_handler);
+    std::signal(SIGINT, &raw_signal_handler);
+    for (int sig = SIGRTMIN + 1; sig <= SIGRTMAX; ++sig) {
+      std::signal(sig, &raw_signal_handler);
+    }
 
-    std::signal(SIGUSR2, [](int /*signal*/) {
-      spdlog::info("Reloading...");
-      reload = true;
-      waybar::Client::inst()->reset();
-    });
+    auto signalPipe = Glib::IOChannel::create_from_fd(signal_fds[0]);
+    // Make the read side, non-blocking
+    int pipe_flags = fcntl(signal_fds[0], F_GETFL);
+    if (pipe_flags != -1) {
+      pipe_flags = fcntl(signal_fds[0], F_SETFL, pipe_flags | O_NONBLOCK);
+    }
+    if (pipe_flags == -1) {
+      throw std::runtime_error("Failed to set pipe to nonblocking mode");
+    }
 
-    std::signal(SIGINT, [](int /*signal*/) {
-      spdlog::info("Quitting.");
-      reload = false;
-      waybar::Client::inst()->reset();
-    });
+    Glib::signal_io().connect(sigc::ptr_fun(handle_signals), signal_fds[0],
+                              Glib::IOCondition::IO_IN);
 
-    for (int sig = SIGRTMIN + 1; sig <= SIGRTMAX; ++sig) {
-      std::signal(sig, [](int sig) {
-        for (auto& bar : waybar::Client::inst()->bars) {
-          bar->handleSignal(sig);
-        }
-      });
-    }
     startSignalThread();
 
     auto ret = 0;