Merge pull request #578 from alebastr/ipc-use-after-free

fix(sway): resolve destruction dependency between Ipc and SleeperThread

include/modules/sway/ipc/client.hpp

@@ -8,6 +8,7 @@
 #include <memory>
 #include <mutex>
 #include "ipc.hpp"
+#include "util/sleeper_thread.hpp"
 
 namespace waybar::modules::sway {
 
@@ -28,6 +29,7 @@ class Ipc {
   void sendCmd(uint32_t type, const std::string &payload = "");
   void subscribe(const std::string &payload);
   void handleEvent();
+  void setWorker(std::function<void()> &&func);
 
  protected:
   static inline const std::string ipc_magic_ = "i3-ipc";
@@ -41,6 +43,7 @@ class Ipc {
   int                 fd_;
   int                 fd_event_;
   std::mutex          mutex_;
+  util::SleeperThread thread_;
 };
 
 }  // namespace waybar::modules::sway

include/modules/sway/mode.hpp

@@ -6,7 +6,6 @@
 #include "client.hpp"
 #include "modules/sway/ipc/client.hpp"
 #include "util/json.hpp"
-#include "util/sleeper_thread.hpp"
 
 namespace waybar::modules::sway {
 
@@ -18,13 +17,10 @@ class Mode : public ALabel, public sigc::trackable {
 
  private:
   void onEvent(const struct Ipc::ipc_response&);
-  void worker();
 
   std::string      mode_;
   util::JsonParser parser_;
   std::mutex       mutex_;
-
-  util::SleeperThread thread_;
   Ipc              ipc_;
 };
 

include/modules/sway/window.hpp

@@ -7,7 +7,6 @@
 #include "client.hpp"
 #include "modules/sway/ipc/client.hpp"
 #include "util/json.hpp"
-#include "util/sleeper_thread.hpp"
 
 namespace waybar::modules::sway {
 
@@ -20,7 +19,6 @@ class Window : public ALabel, public sigc::trackable {
  private:
   void                                                   onEvent(const struct Ipc::ipc_response&);
   void                                                   onCmd(const struct Ipc::ipc_response&);
-  void                                                   worker();
   std::tuple<std::size_t, int, std::string, std::string> getFocusedNode(const Json::Value& nodes,
                                                                         std::string&       output);
   void                                                   getTree();
@@ -33,8 +31,6 @@ class Window : public ALabel, public sigc::trackable {
   std::size_t      app_nb_;
   util::JsonParser parser_;
   std::mutex       mutex_;
-
-  util::SleeperThread thread_;
   Ipc              ipc_;
 };
 

include/modules/sway/workspaces.hpp

@@ -8,7 +8,6 @@
 #include "client.hpp"
 #include "modules/sway/ipc/client.hpp"
 #include "util/json.hpp"
-#include "util/sleeper_thread.hpp"
 
 namespace waybar::modules::sway {
 
@@ -21,7 +20,6 @@ class Workspaces : public AModule, public sigc::trackable {
  private:
   void              onCmd(const struct Ipc::ipc_response&);
   void              onEvent(const struct Ipc::ipc_response&);
-  void              worker();
   bool              filterButtons();
   Gtk::Button&      addButton(const Json::Value&);
   void              onButtonReady(const Json::Value&, Gtk::Button&);
@@ -38,8 +36,6 @@ class Workspaces : public AModule, public sigc::trackable {
   util::JsonParser                             parser_;
   std::unordered_map<std::string, Gtk::Button> buttons_;
   std::mutex                                   mutex_;
-
-  util::SleeperThread thread_;
   Ipc                                          ipc_;
 };
 

src/modules/sway/ipc/client.cpp

@@ -10,19 +10,23 @@ Ipc::Ipc() {
 }
 
 Ipc::~Ipc() {
+  thread_.stop();
+
+  if (fd_ > 0) {
     // To fail the IPC header
     write(fd_, "close-sway-ipc", 14);
-  write(fd_event_, "close-sway-ipc", 14);
-  if (fd_ > 0) {
     close(fd_);
     fd_ = -1;
   }
   if (fd_event_ > 0) {
+    write(fd_event_, "close-sway-ipc", 14);
     close(fd_event_);
     fd_event_ = -1;
   }
 }
 
+void Ipc::setWorker(std::function<void()>&& func) { thread_ = func; }
+
 const std::string Ipc::getSocketPath() const {
   const char* env = getenv("SWAYSOCK");
   if (env != nullptr) {

src/modules/sway/mode.cpp

@@ -8,7 +8,13 @@ Mode::Mode(const std::string& id, const Json::Value& config)
   ipc_.subscribe(R"(["mode"])");
   ipc_.signal_event.connect(sigc::mem_fun(*this, &Mode::onEvent));
   // Launch worker
-  worker();
+  ipc_.setWorker([this] {
+    try {
+      ipc_.handleEvent();
+    } catch (const std::exception& e) {
+      spdlog::error("Mode: {}", e.what());
+    }
+  });
   dp.emit();
 }
 
@@ -31,16 +37,6 @@ void Mode::onEvent(const struct Ipc::ipc_response& res) {
   }
 }
 
-void Mode::worker() {
-  thread_ = [this] {
-    try {
-      ipc_.handleEvent();
-    } catch (const std::exception& e) {
-      spdlog::error("Mode: {}", e.what());
-    }
-  };
-}
-
 auto Mode::update() -> void {
   if (mode_.empty()) {
     event_box_.hide();

src/modules/sway/window.cpp

@@ -11,7 +11,13 @@ Window::Window(const std::string& id, const Bar& bar, const Json::Value& config)
   // Get Initial focused window
   getTree();
   // Launch worker
-  worker();
+  ipc_.setWorker([this] {
+    try {
+      ipc_.handleEvent();
+    } catch (const std::exception& e) {
+      spdlog::error("Window: {}", e.what());
+    }
+  });
 }
 
 void Window::onEvent(const struct Ipc::ipc_response& res) { getTree(); }
@@ -28,16 +34,6 @@ void Window::onCmd(const struct Ipc::ipc_response& res) {
   }
 }
 
-void Window::worker() {
-  thread_ = [this] {
-    try {
-      ipc_.handleEvent();
-    } catch (const std::exception& e) {
-      spdlog::error("Window: {}", e.what());
-    }
-  };
-}
-
 auto Window::update() -> void {
   if (!old_app_id_.empty()) {
     bar_.window.get_style_context()->remove_class(old_app_id_);

src/modules/sway/workspaces.cpp

@@ -22,7 +22,13 @@ Workspaces::Workspaces(const std::string &id, const Bar &bar, const Json::Value
     window.signal_scroll_event().connect(sigc::mem_fun(*this, &Workspaces::handleScroll));
   }
   // Launch worker
-  worker();
+  ipc_.setWorker([this] {
+    try {
+      ipc_.handleEvent();
+    } catch (const std::exception &e) {
+      spdlog::error("Workspaces: {}", e.what());
+    }
+  });
 }
 
 void Workspaces::onEvent(const struct Ipc::ipc_response &res) {
@@ -102,16 +108,6 @@ void Workspaces::onCmd(const struct Ipc::ipc_response &res) {
   }
 }
 
-void Workspaces::worker() {
-  thread_ = [this] {
-    try {
-      ipc_.handleEvent();
-    } catch (const std::exception &e) {
-      spdlog::error("Workspaces: {}", e.what());
-    }
-  };
-}
-
 bool Workspaces::filterButtons() {
   bool needReorder = false;
   for (auto it = buttons_.begin(); it != buttons_.end();) {