fix(Workspaces): fix concurrence and move json parser to ipc client

include/modules/sway/ipc/client.hpp

@@ -6,8 +6,10 @@
 #include <unistd.h>
 #include <cstring>
 #include <iostream>
+#include <memory>
 #include <mutex>
 #include "ipc.hpp"
+#include "util/json.hpp"
 
 namespace waybar::modules::sway {
 
@@ -19,7 +21,7 @@ class Ipc {
   struct ipc_response {
     uint32_t    size;
     uint32_t    type;
-    std::string payload;
+    Json::Value payload;
   };
 
   sigc::signal<void, const struct ipc_response> signal_event;
@@ -38,10 +40,11 @@ class Ipc {
   struct ipc_response send(int fd, uint32_t type, const std::string &payload = "");
   struct ipc_response recv(int fd);
 
-  int        fd_;
+  int              fd_;
-  int        fd_event_;
+  int              fd_event_;
-  std::mutex mutex_;
+  std::mutex       mutex_;
-  std::mutex mutex_event_;
+  std::mutex       mutex_event_;
+  util::JsonParser parser_;
 };
 
 }  // namespace waybar::modules::sway

include/modules/sway/mode.hpp

@@ -5,7 +5,6 @@
 #include "bar.hpp"
 #include "client.hpp"
 #include "modules/sway/ipc/client.hpp"
-#include "util/json.hpp"
 #include "util/sleeper_thread.hpp"
 
 namespace waybar::modules::sway {
@@ -22,7 +21,6 @@ class Mode : public ALabel {
 
   const Bar&                  bar_;
   waybar::util::SleeperThread thread_;
-  util::JsonParser            parser_;
   Ipc                         ipc_;
   std::string                 mode_;
 };

include/modules/sway/window.hpp

@@ -6,7 +6,6 @@
 #include "bar.hpp"
 #include "client.hpp"
 #include "modules/sway/ipc/client.hpp"
-#include "util/json.hpp"
 #include "util/sleeper_thread.hpp"
 
 namespace waybar::modules::sway {
@@ -26,7 +25,6 @@ class Window : public ALabel {
 
   const Bar&                  bar_;
   waybar::util::SleeperThread thread_;
-  util::JsonParser            parser_;
   Ipc                         ipc_;
   std::string                 window_;
   int                         windowId_;

include/modules/sway/workspaces.hpp

@@ -7,7 +7,6 @@
 #include "bar.hpp"
 #include "client.hpp"
 #include "modules/sway/ipc/client.hpp"
-#include "util/json.hpp"
 #include "util/sleeper_thread.hpp"
 
 namespace waybar::modules::sway {
@@ -36,8 +35,8 @@ class Workspaces : public IModule {
   const Json::Value&                           config_;
   std::vector<Json::Value>                     workspaces_;
   waybar::util::SleeperThread                  thread_;
+  std::mutex                                   mutex_;
   Gtk::Box                                     box_;
-  util::JsonParser                             parser_;
   Ipc                                          ipc_;
   bool                                         scrolling_;
   std::unordered_map<std::string, Gtk::Button> buttons_;

include/util/json.hpp

@@ -7,13 +7,14 @@ namespace waybar::util {
 struct JsonParser {
   JsonParser() : reader_(builder_.newCharReader()) {}
 
-  const Json::Value parse(const std::string& data) const {
+  const Json::Value parse(const std::string& data, std::size_t size = 0) const {
-    Json::Value root;
+    Json::Value root(Json::objectValue);
-    std::string err;
     if (data.empty()) {
       return root;
     }
-    bool res = reader_->parse(data.c_str(), data.c_str() + data.size(), &root, &err);
+    std::string err;
+    auto data_size = size > 0 ? size : data.size();
+    bool res = reader_->parse(data.c_str(), data.c_str() + data_size, &root, &err);
     if (!res) throw std::runtime_error(err);
     return root;
   }

src/modules/mpd.cpp

@@ -259,13 +259,12 @@ void waybar::modules::MPD::tryConnect() {
 
   try {
     checkErrors(connection_.get());
+    std::cerr << module_name_ << ": Connected to MPD" << std::endl;
   } catch (std::runtime_error& e) {
     std::cerr << module_name_ << ": Failed to connect to MPD: " << e.what() << std::endl;
     connection_.reset();
     alternate_connection_.reset();
   }
-
-  std::cerr << module_name_ << ": Connected to MPD" << std::endl;
 }
 
 void waybar::modules::MPD::checkErrors(mpd_connection* conn) {

src/modules/sway/ipc/client.cpp

@@ -97,11 +97,15 @@ struct Ipc::ipc_response Ipc::recv(int fd) {
   while (total < data32[0]) {
     auto res = ::recv(fd, payload.data() + total, data32[0] - total, 0);
     if (res < 0) {
+      if (errno == EINTR || errno == EAGAIN) {
+        continue;
+      }
       throw std::runtime_error("Unable to receive IPC payload");
     }
     total += res;
   }
-  return {data32[0], data32[1], &payload.front()};
+  auto parsed = parser_.parse(&payload.front(), data32[0]);
+  return {data32[0], data32[1], parsed};
 }
 
 struct Ipc::ipc_response Ipc::send(int fd, uint32_t type, const std::string& payload) {
@@ -130,7 +134,7 @@ void Ipc::sendCmd(uint32_t type, const std::string& payload) {
 void Ipc::subscribe(const std::string& payload) {
   std::lock_guard<std::mutex> lock(mutex_event_);
   auto res = Ipc::send(fd_event_, IPC_SUBSCRIBE, payload);
-  if (res.payload != "{\"success\": true}") {
+  if (!res.payload["success"].asBool()) {
     throw std::runtime_error("Unable to subscribe ipc event");
   }
 }

src/modules/sway/mode.cpp

@@ -16,9 +16,8 @@ Mode::Mode(const std::string& id, const Bar& bar, const Json::Value& config)
 }
 
 void Mode::onEvent(const struct Ipc::ipc_response res) {
-  auto parsed = parser_.parse(res.payload);
+  if (res.payload["change"] != "default") {
-  if (parsed["change"] != "default") {
+    mode_ = res.payload["change"].asString();
-    mode_ = parsed["change"].asString();
   } else {
     mode_.clear();
   }

src/modules/sway/window.cpp

@@ -21,18 +21,17 @@ Window::Window(const std::string& id, const Bar& bar, const Json::Value& config)
 }
 
 void Window::onEvent(const struct Ipc::ipc_response res) {
-  auto parsed = parser_.parse(res.payload);
+  auto data = res.payload;
   // Check for waybar prevents flicker when hovering window module
-  if ((parsed["change"] == "focus" || parsed["change"] == "title") &&
+  if ((data["change"] == "focus" || data["change"] == "title") &&
-      parsed["container"]["focused"].asBool() &&
+      data["container"]["focused"].asBool() && data["container"]["name"].asString() != "waybar") {
-      parsed["container"]["name"].asString() != "waybar") {
+    window_ = Glib::Markup::escape_text(data["container"]["name"].asString());
-    window_ = Glib::Markup::escape_text(parsed["container"]["name"].asString());
+    windowId_ = data["container"]["id"].asInt();
-    windowId_ = parsed["container"]["id"].asInt();
     dp.emit();
-  } else if ((parsed["change"] == "close" && parsed["container"]["focused"].asBool() &&
+  } else if ((data["change"] == "close" && data["container"]["focused"].asBool() &&
-              windowId_ == parsed["container"]["id"].asInt()) ||
+              windowId_ == data["container"]["id"].asInt()) ||
-             (parsed["change"] == "focus" && parsed["current"]["focus"].isArray() &&
+             (data["change"] == "focus" && data["current"]["focus"].isArray() &&
-              parsed["current"]["focus"].empty())) {
+              data["current"]["focus"].empty())) {
     window_.clear();
     windowId_ = -1;
     dp.emit();
@@ -40,8 +39,7 @@ void Window::onEvent(const struct Ipc::ipc_response res) {
 }
 
 void Window::onCmd(const struct Ipc::ipc_response res) {
-  auto parsed = parser_.parse(res.payload);
+  auto [id, name] = getFocusedNode(res.payload["nodes"]);
-  auto [id, name] = getFocusedNode(parsed["nodes"]);
   windowId_ = id;
   window_ = name;
   dp.emit();

src/modules/sway/workspaces.cpp

@@ -23,17 +23,19 @@ void Workspaces::onEvent(const struct Ipc::ipc_response res) { ipc_.sendCmd(IPC_
 
 void Workspaces::onCmd(const struct Ipc::ipc_response res) {
   if (res.type == IPC_GET_WORKSPACES) {
-    auto workspaces = parser_.parse(res.payload);
+    if (res.payload.isArray()) {
-    workspaces_.clear();
+      std::lock_guard<std::mutex> lock(mutex_);
-    std::copy_if(workspaces.begin(),
+      workspaces_.clear();
-                 workspaces.end(),
+      std::copy_if(res.payload.begin(),
-                 std::back_inserter(workspaces_),
+                   res.payload.end(),
-                 [&](const auto &workspace) {
+                   std::back_inserter(workspaces_),
-                   return !config_["all-outputs"].asBool()
+                   [&](const auto &workspace) {
-                              ? workspace["output"].asString() == bar_.output->name
+                     return !config_["all-outputs"].asBool()
-                              : true;
+                                ? workspace["output"].asString() == bar_.output->name
-                 });
+                                : true;
-    dp.emit();
+                   });
+      dp.emit();
+    }
   } else {
     if (scrolling_) {
       scrolling_ = false;
@@ -69,7 +71,8 @@ bool Workspaces::filterButtons() {
 }
 
 auto Workspaces::update() -> void {
-  bool needReorder = filterButtons();
+  std::lock_guard<std::mutex> lock(mutex_);
+  bool                        needReorder = filterButtons();
   for (auto it = workspaces_.begin(); it != workspaces_.end(); ++it) {
     auto bit = buttons_.find((*it)["name"].asString());
     if (bit == buttons_.end()) {
@@ -149,39 +152,42 @@ bool Workspaces::handleScroll(GdkEventScroll *e) {
   if (scrolling_) {
     return false;
   }
-  scrolling_ = true;
   std::string name;
-  auto        it = std::find_if(workspaces_.begin(), workspaces_.end(), [](const auto &workspace) {
+  scrolling_ = true;
-    return workspace["focused"].asBool();
+  {
-  });
+    std::lock_guard<std::mutex> lock(mutex_);
-  if (it == workspaces_.end()) {
+    auto it = std::find_if(workspaces_.begin(), workspaces_.end(), [](const auto &workspace) {
-    scrolling_ = false;
+      return workspace["focused"].asBool();
-    return false;
+    });
-  }
+    if (it == workspaces_.end()) {
-  switch (e->direction) {
+      scrolling_ = false;
-    case GDK_SCROLL_DOWN:
+      return false;
-    case GDK_SCROLL_RIGHT:
+    }
-      name = getCycleWorkspace(it, false);
+    switch (e->direction) {
-      break;
+      case GDK_SCROLL_DOWN:
-    case GDK_SCROLL_UP:
+      case GDK_SCROLL_RIGHT:
-    case GDK_SCROLL_LEFT:
-      name = getCycleWorkspace(it, true);
-      break;
-    case GDK_SCROLL_SMOOTH:
-      gdouble delta_x, delta_y;
-      gdk_event_get_scroll_deltas(reinterpret_cast<const GdkEvent *>(e), &delta_x, &delta_y);
-      if (delta_y < 0) {
-        name = getCycleWorkspace(it, true);
-      } else if (delta_y > 0) {
         name = getCycleWorkspace(it, false);
-      }
+        break;
-      break;
+      case GDK_SCROLL_UP:
-    default:
+      case GDK_SCROLL_LEFT:
-      break;
+        name = getCycleWorkspace(it, true);
-  }
+        break;
-  if (name.empty() || name == (*it)["name"].asString()) {
+      case GDK_SCROLL_SMOOTH:
-    scrolling_ = false;
+        gdouble delta_x, delta_y;
-    return false;
+        gdk_event_get_scroll_deltas(reinterpret_cast<const GdkEvent *>(e), &delta_x, &delta_y);
+        if (delta_y < 0) {
+          name = getCycleWorkspace(it, true);
+        } else if (delta_y > 0) {
+          name = getCycleWorkspace(it, false);
+        }
+        break;
+      default:
+        break;
+    }
+    if (name.empty() || name == (*it)["name"].asString()) {
+      scrolling_ = false;
+      return false;
+    }
   }
   ipc_.sendCmd(IPC_COMMAND, fmt::format("workspace \"{}\"", name));
   return true;