From ae6ca36fa74802f487c3b45c8ff32c571ec36833 Mon Sep 17 00:00:00 2001 From: Aleksei Bavshin Date: Tue, 27 Aug 2019 19:43:03 -0700 Subject: [PATCH] fix(sway): resolve destruction dependency between Ipc and SleeperThread Ipc destructor closes socket and thus wakes up SleeperThread which was waiting for socket data in Ipc::handleEvent. Ipc::handleEvent then proceeds with sending signal to already destroyed object, causing heap-use-after-free Address Sanitizer error. --- include/modules/sway/ipc/client.hpp | 9 ++++++--- include/modules/sway/mode.hpp | 6 +----- include/modules/sway/window.hpp | 6 +----- include/modules/sway/workspaces.hpp | 6 +----- src/modules/sway/ipc/client.cpp | 10 +++++++--- src/modules/sway/mode.cpp | 18 +++++++----------- src/modules/sway/window.cpp | 18 +++++++----------- src/modules/sway/workspaces.cpp | 18 +++++++----------- 8 files changed, 37 insertions(+), 54 deletions(-) diff --git a/include/modules/sway/ipc/client.hpp b/include/modules/sway/ipc/client.hpp index 629556f7..7df53629 100644 --- a/include/modules/sway/ipc/client.hpp +++ b/include/modules/sway/ipc/client.hpp @@ -8,6 +8,7 @@ #include #include #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 &&func); protected: static inline const std::string ipc_magic_ = "i3-ipc"; @@ -38,9 +40,10 @@ 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_event_; - std::mutex mutex_; + int fd_; + int fd_event_; + std::mutex mutex_; + util::SleeperThread thread_; }; } // namespace waybar::modules::sway diff --git a/include/modules/sway/mode.hpp b/include/modules/sway/mode.hpp index f0cf74c1..a1a88b02 100644 --- a/include/modules/sway/mode.hpp +++ b/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,14 +17,11 @@ 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_; + Ipc ipc_; }; } // namespace waybar::modules::sway diff --git a/include/modules/sway/window.hpp b/include/modules/sway/window.hpp index 5bb129d7..40aaa1a0 100644 --- a/include/modules/sway/window.hpp +++ b/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 getFocusedNode(const Json::Value& nodes, std::string& output); void getTree(); @@ -33,9 +31,7 @@ class Window : public ALabel, public sigc::trackable { std::size_t app_nb_; util::JsonParser parser_; std::mutex mutex_; - - util::SleeperThread thread_; - Ipc ipc_; + Ipc ipc_; }; } // namespace waybar::modules::sway diff --git a/include/modules/sway/workspaces.hpp b/include/modules/sway/workspaces.hpp index 498acc95..cd806125 100644 --- a/include/modules/sway/workspaces.hpp +++ b/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,9 +36,7 @@ class Workspaces : public AModule, public sigc::trackable { util::JsonParser parser_; std::unordered_map buttons_; std::mutex mutex_; - - util::SleeperThread thread_; - Ipc ipc_; + Ipc ipc_; }; } // namespace waybar::modules::sway diff --git a/src/modules/sway/ipc/client.cpp b/src/modules/sway/ipc/client.cpp index eae6c76e..58aed60c 100644 --- a/src/modules/sway/ipc/client.cpp +++ b/src/modules/sway/ipc/client.cpp @@ -10,19 +10,23 @@ Ipc::Ipc() { } Ipc::~Ipc() { - // To fail the IPC header - write(fd_, "close-sway-ipc", 14); - write(fd_event_, "close-sway-ipc", 14); + thread_.stop(); + if (fd_ > 0) { + // To fail the IPC header + write(fd_, "close-sway-ipc", 14); 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&& func) { thread_ = func; } + const std::string Ipc::getSocketPath() const { const char* env = getenv("SWAYSOCK"); if (env != nullptr) { diff --git a/src/modules/sway/mode.cpp b/src/modules/sway/mode.cpp index cd02c0ca..632709d0 100644 --- a/src/modules/sway/mode.cpp +++ b/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(); diff --git a/src/modules/sway/window.cpp b/src/modules/sway/window.cpp index 2e4ec468..c139180a 100644 --- a/src/modules/sway/window.cpp +++ b/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_); diff --git a/src/modules/sway/workspaces.cpp b/src/modules/sway/workspaces.cpp index b65e47db..fe87f5ea 100644 --- a/src/modules/sway/workspaces.cpp +++ b/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();) {