Better way to notify controller

include/components/controller.hpp

@@ -64,7 +64,7 @@ class controller : public signal_receiver<SIGN_PRIORITY_CONTROLLER, signals::eve
 
  protected:
   void read_events(bool confwatch);
-  void process_inputdata();
+  void process_inputdata(string&& cmd);
   bool process_update(bool force);
 
   bool on(const signals::eventqueue::notify_change& evt) override;
@@ -78,6 +78,16 @@ class controller : public signal_receiver<SIGN_PRIORITY_CONTROLLER, signals::eve
   bool on(const signals::ui::update_background& evt) override;
 
  private:
+  struct notifications_t {
+    bool quit;
+    bool reload;
+    bool update;
+    bool force_update;
+    string inputdata;
+
+    notifications_t() : quit(false), reload(false), update(false), force_update(false), inputdata(string{}) {}
+  };
+
   size_t setup_modules(alignment align);
 
   bool forward_action(const actions_util::action& cmd);
@@ -100,6 +110,20 @@ class controller : public signal_receiver<SIGN_PRIORITY_CONTROLLER, signals::eve
    */
   std::unique_ptr<uv_async_t> m_notifier{nullptr};
 
+  /**
+   * Notification data for the controller.
+   *
+   * Triggers, potentially from other threads, update this structure and notify the controller through m_notifier.
+   */
+  notifications_t m_notifications{};
+
+  /**
+   * \brief Protected m_notifications.
+   *
+   * All accesses to m_notifications must hold this mutex.
+   */
+  std::mutex m_notification_mutex{};
+
   /**
    * \brief Destination path of generated snapshot
    */
@@ -119,11 +143,6 @@ class controller : public signal_receiver<SIGN_PRIORITY_CONTROLLER, signals::eve
    * \brief Loaded modules grouped by block
    */
   modulemap_t m_blocks;
-
-  /**
-   * \brief Input data
-   */
-  string m_inputdata;
 };
 
 POLYBAR_NS_END

src/components/controller.cpp

@@ -29,11 +29,6 @@
 POLYBAR_NS
 
 sig_atomic_t g_reload{0};
-sig_atomic_t g_terminate{0};
-
-// TODO pass this information in a better way
-sig_atomic_t g_update{0};
-sig_atomic_t g_force_update{0};
 
 /**
  * Build controller instance
@@ -138,36 +133,38 @@ bool controller::run(bool writeback, string snapshot_dst, bool confwatch) {
  * Enqueue input data
  */
 void controller::trigger_action(string&& input_data) {
-  if (!m_inputdata.empty()) {
+  std::unique_lock<std::mutex> guard(m_notification_mutex);
-    m_log.trace("controller: Swallowing input event (pending data)");
+
-  } else {
+  if (m_notifications.inputdata.empty()) {
-    m_inputdata = forward<string>(input_data);
+    m_notifications.inputdata = std::forward<string>(input_data);
+    // TODO create function for this
     UV(uv_async_send, m_notifier.get());
+  } else {
+    m_log.trace("controller: Swallowing input event (pending data)");
   }
 }
 
 void controller::trigger_quit(bool reload) {
-  g_terminate = 1;
+  std::unique_lock<std::mutex> guard(m_notification_mutex);
-  g_reload = reload;
+  m_notifications.quit = true;
+  m_notifications.reload = m_notifications.reload || reload;
   // TODO create function for this
   UV(uv_async_send, m_notifier.get());
 }
 
 void controller::trigger_update(bool force) {
-  if (force) {
+  std::unique_lock<std::mutex> guard(m_notification_mutex);
-    g_force_update = 1;
+  m_notifications.update = true;
-  } else {
+  m_notifications.force_update = m_notifications.force_update || force;
-    g_update = 1;
-  }
 
   // TODO this isn't really safe
   if (m_notifier) {
+    // TODO create function for this
     UV(uv_async_send, m_notifier.get());
   }
 }
 
 void controller::stop(bool reload) {
-  g_terminate = 1;
   g_reload = reload;
   eloop->stop();
 }
@@ -220,23 +217,27 @@ void controller::confwatch_handler(const char* filename, int, int) {
 }
 
 void controller::notifier_handler() {
-  if (g_terminate) {
+  notifications_t data{};
+
+  {
+    std::unique_lock<std::mutex> guard(m_notification_mutex);
+    std::swap(m_notifications, data);
+  }
+
+  if (data.quit) {
+    // TODO store this in the instance
+    g_reload = data.reload;
     eloop->stop();
     return;
   }
 
-  if (!m_inputdata.empty()) {
+  if (!data.inputdata.empty()) {
-    process_inputdata();
+    process_inputdata(std::move(data.inputdata));
   }
 
-  if (g_force_update) {
+  if (data.update) {
-    process_update(true);
+    process_update(data.force_update);
-  } else if (g_update) {
-    process_update(false);
   }
-
-  g_update = 0;
-  g_force_update = 0;
 }
 
 static void ipc_alloc_cb(uv_handle_t*, size_t, uv_buf_t* buf) {
@@ -484,14 +485,7 @@ bool controller::forward_action(const actions_util::action& action_triple) {
 /**
  * Process stored input data
  */
-void controller::process_inputdata() {
+void controller::process_inputdata(string&& cmd) {
-  if (m_inputdata.empty()) {
-    return;
-  }
-
-  const string cmd = std::move(m_inputdata);
-  m_inputdata = string{};
-
   m_log.trace("controller: Processing inputdata: %s", cmd);
 
   // Every command that starts with '#' is considered an action string.