refactor: improve handling of reload signal (SIGUSR2)

Repeated calls to `Client::main()` result in unnecessary work and
duplication of several event handlers. A common consequence of that is
the bug with multiple bars being created (#3344).

This change ensures that the `main` is called only once and makes reload
logic much simpler.

include/client.hpp

@@ -15,11 +15,16 @@ struct zwp_idle_inhibit_manager_v1;
 
 namespace waybar {
 
-class Client {
+class Client : public sigc::trackable {
  public:
   static Client *inst();
   int main(int argc, char *argv[]);
-  void reset();
+
+  /* signal handlers */
+  void handleSignal(int signum);
+  void reload(int signum = 0);
+  void toggle(int signum = 0);
+  void quit(int signum = 0);
 
   Glib::RefPtr<Gtk::Application> gtk_app;
   Glib::RefPtr<Gdk::Display> gdk_display;
@@ -54,8 +59,11 @@ class Client {
   Glib::RefPtr<Gtk::CssProvider> css_provider_;
   std::unique_ptr<Portal> portal;
   std::list<struct waybar_output> outputs_;
+  sigc::connection output_added_, output_removed_;
   std::unique_ptr<CssReloadHelper> m_cssReloadHelper;
   std::string m_cssFile;
+  std::string config_opt_;
+  std::string style_opt_;
 };
 
 }  // namespace waybar

src/client.cpp

@@ -1,12 +1,12 @@
 #include "client.hpp"
 
 #include <gtk-layer-shell.h>
+#include <gtkmm/icontheme.h>
 #include <spdlog/spdlog.h>
 
 #include <iostream>
 #include <utility>
 
-#include "gtkmm/icontheme.h"
 #include "idle-inhibit-unstable-v1-client-protocol.h"
 #include "util/clara.hpp"
 #include "util/format.hpp"
@@ -205,26 +205,16 @@ void waybar::Client::bindInterfaces() {
   if (xdg_output_manager == nullptr) {
     throw std::runtime_error("Failed to acquire required resources.");
   }
-  // add existing outputs and subscribe to updates
-  for (auto i = 0; i < gdk_display->get_n_monitors(); ++i) {
-    auto monitor = gdk_display->get_monitor(i);
-    handleMonitorAdded(monitor);
-  }
-  gdk_display->signal_monitor_added().connect(sigc::mem_fun(*this, &Client::handleMonitorAdded));
-  gdk_display->signal_monitor_removed().connect(
-      sigc::mem_fun(*this, &Client::handleMonitorRemoved));
 }
 
 int waybar::Client::main(int argc, char *argv[]) {
   bool show_help = false;
   bool show_version = false;
-  std::string config_opt;
-  std::string style_opt;
   std::string log_level;
   auto cli = clara::detail::Help(show_help) |
              clara::detail::Opt(show_version)["-v"]["--version"]("Show version") |
-             clara::detail::Opt(config_opt, "config")["-c"]["--config"]("Config path") |
-             clara::detail::Opt(style_opt, "style")["-s"]["--style"]("Style path") |
+             clara::detail::Opt(config_opt_, "config")["-c"]["--config"]("Config path") |
+             clara::detail::Opt(style_opt_, "style")["-s"]["--style"]("Style path") |
              clara::detail::Opt(
                  log_level,
                  "trace|debug|info|warning|error|critical|off")["-l"]["--log-level"]("Log level") |
@@ -260,40 +250,72 @@ int waybar::Client::main(int argc, char *argv[]) {
     throw std::runtime_error("Bar need to run under Wayland");
   }
   wl_display = gdk_wayland_display_get_wl_display(gdk_display->gobj());
-  config.load(config_opt);
-  if (!portal) {
-    portal = std::make_unique<waybar::Portal>();
-  }
-  m_cssFile = getStyle(style_opt);
-  setupCss(m_cssFile);
-  m_cssReloadHelper = std::make_unique<CssReloadHelper>(m_cssFile, [&]() { setupCss(m_cssFile); });
+
+  bindInterfaces();
+
+  portal = std::make_unique<waybar::Portal>();
   portal->signal_appearance_changed().connect([&](waybar::Appearance appearance) {
-    auto css_file = getStyle(style_opt, appearance);
+    auto css_file = getStyle(style_opt_, appearance);
     setupCss(css_file);
   });
 
-  auto m_config = config.getConfig();
-  if (m_config.isObject() && m_config["reload_style_on_change"].asBool()) {
+  reload();
+
+  gtk_app->hold();
+  gtk_app->run();
+  m_cssReloadHelper.reset();  // stop watching css file
+  bars.clear();
+  return 0;
+}
+
+void waybar::Client::handleSignal(int signum) {
+  spdlog::debug("RT signal {} received", signum);
+  for (auto &bar : bars) {
+    bar->handleSignal(signum);
+  }
+}
+
+void waybar::Client::reload(G_GNUC_UNUSED int signum) {
+  config.load(config_opt_);
+
+  m_cssFile = getStyle(style_opt_);
+  setupCss(m_cssFile);
+
+  m_cssReloadHelper = std::make_unique<CssReloadHelper>(m_cssFile, [&]() { setupCss(m_cssFile); });
+
+  auto confObj = config.getConfig();
+  if (confObj.isObject() && confObj["reload_style_on_change"].asBool()) {
     m_cssReloadHelper->monitorChanges();
-  } else if (m_config.isArray()) {
-    for (const auto &conf : m_config) {
+  } else if (confObj.isArray()) {
+    for (const auto &conf : confObj) {
       if (conf["reload_style_on_change"].asBool()) {
         m_cssReloadHelper->monitorChanges();
         break;
       }
     }
   }
 
-  bindInterfaces();
-  gtk_app->hold();
-  gtk_app->run();
-  m_cssReloadHelper.reset();  // stop watching css file
+  output_added_.disconnect();
+  output_removed_.disconnect();
   bars.clear();
-  return 0;
+  outputs_.clear();
+
+  // add existing outputs and subscribe to updates
+  for (auto i = 0; i < gdk_display->get_n_monitors(); ++i) {
+    auto monitor = gdk_display->get_monitor(i);
+    handleMonitorAdded(monitor);
+  }
+
+  output_added_ = gdk_display->signal_monitor_added().connect(
+      sigc::mem_fun(*this, &Client::handleMonitorAdded));
+  output_removed_ = gdk_display->signal_monitor_removed().connect(
+      sigc::mem_fun(*this, &Client::handleMonitorRemoved));
 }
 
-void waybar::Client::reset() {
-  gtk_app->quit();
-  // delete signal handler for css changes
-  portal->signal_appearance_changed().clear();
+void waybar::Client::toggle(G_GNUC_UNUSED int signum) {
+  for (auto &bar : bars) {
+    bar->toggle();
+  }
 }
+
+void waybar::Client::quit(G_GNUC_UNUSED int signum) { gtk_app->quit(); }

src/main.cpp

@@ -3,46 +3,21 @@
 #include "client.hpp"
 #include "util/unix_signal.hpp"
 
-volatile bool reload;
 waybar::UnixSignalHandler signal_handler;
 
 int main(int argc, char* argv[]) {
   try {
     auto* client = waybar::Client::inst();
 
-    signal_handler.signal(SIGUSR1).connect([](int /*signal*/) {
-      for (auto& bar : waybar::Client::inst()->bars) {
-        bar->toggle();
-      }
-    });
-
-    signal_handler.signal(SIGUSR2).connect([](int /*signal*/) {
-      spdlog::info("Reloading...");
-      reload = true;
-      waybar::Client::inst()->reset();
-    });
-
-    signal_handler.signal(SIGINT).connect([](int /*signal*/) {
-      spdlog::info("Quitting.");
-      reload = false;
-      waybar::Client::inst()->reset();
-    });
-
+    signal_handler.signal(SIGUSR1).connect(sigc::mem_fun(*client, &waybar::Client::toggle));
+    signal_handler.signal(SIGUSR2).connect(sigc::mem_fun(*client, &waybar::Client::reload));
+    signal_handler.signal(SIGINT).connect(sigc::mem_fun(*client, &waybar::Client::quit));
     for (int sig = SIGRTMIN + 1; sig <= SIGRTMAX; ++sig) {
-      signal_handler.signal(sig).connect([](int sig) {
-        for (auto& bar : waybar::Client::inst()->bars) {
-          bar->handleSignal(sig);
-        }
-      });
+      signal_handler.signal(sig).connect(sigc::mem_fun(*client, &waybar::Client::handleSignal));
     }
     signal_handler.start();
 
-    auto ret = 0;
-    do {
-      reload = false;
-      ret = client->main(argc, argv);
-    } while (reload);
-
+    auto ret = client->main(argc, argv);
     delete client;
     return ret;
   } catch (const std::exception& e) {