fix(hyprland-ipc): harden fd lifecycle and listener loop
Hyprland IPC had fd lifecycle risks on failure/shutdown paths and used a spin-sleep listener model. I initialized fd state defensively, tightened connect/close/shutdown handling, moved to blocking read with newline framing, and added RAII-style fd cleanup in socket1 reply paths. Signed-off-by: Austin Horstman <khaneliman12@gmail.com>
This commit is contained in:
@@ -1,5 +1,6 @@
|
|||||||
#pragma once
|
#pragma once
|
||||||
|
|
||||||
|
#include <atomic>
|
||||||
#include <filesystem>
|
#include <filesystem>
|
||||||
#include <list>
|
#include <list>
|
||||||
#include <mutex>
|
#include <mutex>
|
||||||
@@ -43,10 +44,11 @@ class IPC {
|
|||||||
|
|
||||||
std::thread ipcThread_;
|
std::thread ipcThread_;
|
||||||
std::mutex callbackMutex_;
|
std::mutex callbackMutex_;
|
||||||
|
std::mutex socketMutex_;
|
||||||
util::JsonParser parser_;
|
util::JsonParser parser_;
|
||||||
std::list<std::pair<std::string, EventHandler*>> callbacks_;
|
std::list<std::pair<std::string, EventHandler*>> callbacks_;
|
||||||
int socketfd_; // the hyprland socket file descriptor
|
int socketfd_ = -1; // the hyprland socket file descriptor
|
||||||
pid_t socketOwnerPid_;
|
pid_t socketOwnerPid_ = -1;
|
||||||
bool running_ = true; // the ipcThread will stop running when this is false
|
std::atomic<bool> running_ = true; // the ipcThread will stop running when this is false
|
||||||
};
|
};
|
||||||
}; // namespace waybar::modules::hyprland
|
}; // namespace waybar::modules::hyprland
|
||||||
|
|||||||
@@ -9,11 +9,35 @@
|
|||||||
#include <sys/un.h>
|
#include <sys/un.h>
|
||||||
#include <unistd.h>
|
#include <unistd.h>
|
||||||
|
|
||||||
|
#include <array>
|
||||||
|
#include <cerrno>
|
||||||
|
#include <cstring>
|
||||||
#include <filesystem>
|
#include <filesystem>
|
||||||
#include <string>
|
#include <string>
|
||||||
|
|
||||||
namespace waybar::modules::hyprland {
|
namespace waybar::modules::hyprland {
|
||||||
|
|
||||||
|
namespace {
|
||||||
|
class ScopedFd {
|
||||||
|
public:
|
||||||
|
explicit ScopedFd(int fd) : fd_(fd) {}
|
||||||
|
~ScopedFd() {
|
||||||
|
if (fd_ != -1) {
|
||||||
|
close(fd_);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// ScopedFd is non-copyable
|
||||||
|
ScopedFd(const ScopedFd&) = delete;
|
||||||
|
ScopedFd& operator=(const ScopedFd&) = delete;
|
||||||
|
|
||||||
|
int get() const { return fd_; }
|
||||||
|
|
||||||
|
private:
|
||||||
|
int fd_;
|
||||||
|
};
|
||||||
|
} // namespace
|
||||||
|
|
||||||
std::filesystem::path IPC::socketFolder_;
|
std::filesystem::path IPC::socketFolder_;
|
||||||
|
|
||||||
std::filesystem::path IPC::getSocketFolder(const char* instanceSig) {
|
std::filesystem::path IPC::getSocketFolder(const char* instanceSig) {
|
||||||
@@ -45,8 +69,8 @@ std::filesystem::path IPC::getSocketFolder(const char* instanceSig) {
|
|||||||
|
|
||||||
IPC::IPC() {
|
IPC::IPC() {
|
||||||
// will start IPC and relay events to parseIPC
|
// will start IPC and relay events to parseIPC
|
||||||
ipcThread_ = std::thread([this]() { socketListener(); });
|
|
||||||
socketOwnerPid_ = getpid();
|
socketOwnerPid_ = getpid();
|
||||||
|
ipcThread_ = std::thread([this]() { socketListener(); });
|
||||||
}
|
}
|
||||||
|
|
||||||
IPC::~IPC() {
|
IPC::~IPC() {
|
||||||
@@ -54,19 +78,20 @@ IPC::~IPC() {
|
|||||||
// failed exec()) exits.
|
// failed exec()) exits.
|
||||||
if (getpid() != socketOwnerPid_) return;
|
if (getpid() != socketOwnerPid_) return;
|
||||||
|
|
||||||
running_ = false;
|
running_.store(false, std::memory_order_relaxed);
|
||||||
spdlog::info("Hyprland IPC stopping...");
|
spdlog::info("Hyprland IPC stopping...");
|
||||||
if (socketfd_ != -1) {
|
{
|
||||||
spdlog::trace("Shutting down socket");
|
std::lock_guard<std::mutex> lock(socketMutex_);
|
||||||
if (shutdown(socketfd_, SHUT_RDWR) == -1) {
|
if (socketfd_ != -1) {
|
||||||
spdlog::error("Hyprland IPC: Couldn't shutdown socket");
|
spdlog::trace("Shutting down socket");
|
||||||
}
|
if (shutdown(socketfd_, SHUT_RDWR) == -1 && errno != ENOTCONN) {
|
||||||
spdlog::trace("Closing socket");
|
spdlog::error("Hyprland IPC: Couldn't shutdown socket");
|
||||||
if (close(socketfd_) == -1) {
|
}
|
||||||
spdlog::error("Hyprland IPC: Couldn't close socket");
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
ipcThread_.join();
|
if (ipcThread_.joinable()) {
|
||||||
|
ipcThread_.join();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
IPC& IPC::inst() {
|
IPC& IPC::inst() {
|
||||||
@@ -86,9 +111,9 @@ void IPC::socketListener() {
|
|||||||
spdlog::info("Hyprland IPC starting");
|
spdlog::info("Hyprland IPC starting");
|
||||||
|
|
||||||
struct sockaddr_un addr;
|
struct sockaddr_un addr;
|
||||||
socketfd_ = socket(AF_UNIX, SOCK_STREAM, 0);
|
const int socketfd = socket(AF_UNIX, SOCK_STREAM, 0);
|
||||||
|
|
||||||
if (socketfd_ == -1) {
|
if (socketfd == -1) {
|
||||||
spdlog::error("Hyprland IPC: socketfd failed");
|
spdlog::error("Hyprland IPC: socketfd failed");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -102,38 +127,67 @@ void IPC::socketListener() {
|
|||||||
|
|
||||||
int l = sizeof(struct sockaddr_un);
|
int l = sizeof(struct sockaddr_un);
|
||||||
|
|
||||||
if (connect(socketfd_, (struct sockaddr*)&addr, l) == -1) {
|
if (connect(socketfd, (struct sockaddr*)&addr, l) == -1) {
|
||||||
spdlog::error("Hyprland IPC: Unable to connect?");
|
spdlog::error("Hyprland IPC: Unable to connect? {}", std::strerror(errno));
|
||||||
|
close(socketfd);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
auto* file = fdopen(socketfd_, "r");
|
|
||||||
if (file == nullptr) {
|
{
|
||||||
spdlog::error("Hyprland IPC: Couldn't open file descriptor");
|
std::lock_guard<std::mutex> lock(socketMutex_);
|
||||||
return;
|
socketfd_ = socketfd;
|
||||||
}
|
}
|
||||||
while (running_) {
|
|
||||||
|
std::string pending;
|
||||||
|
while (running_.load(std::memory_order_relaxed)) {
|
||||||
std::array<char, 1024> buffer; // Hyprland socket2 events are max 1024 bytes
|
std::array<char, 1024> buffer; // Hyprland socket2 events are max 1024 bytes
|
||||||
|
const ssize_t bytes_read = read(socketfd, buffer.data(), buffer.size());
|
||||||
|
|
||||||
auto* receivedCharPtr = fgets(buffer.data(), buffer.size(), file);
|
if (bytes_read == 0) {
|
||||||
|
if (running_.load(std::memory_order_relaxed)) {
|
||||||
if (receivedCharPtr == nullptr) {
|
spdlog::warn("Hyprland IPC: Socket closed by peer");
|
||||||
std::this_thread::sleep_for(std::chrono::milliseconds(1));
|
}
|
||||||
continue;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
std::string messageReceived(buffer.data());
|
if (bytes_read < 0) {
|
||||||
messageReceived = messageReceived.substr(0, messageReceived.find_first_of('\n'));
|
if (errno == EINTR) {
|
||||||
spdlog::debug("hyprland IPC received {}", messageReceived);
|
continue;
|
||||||
|
}
|
||||||
try {
|
if (!running_.load(std::memory_order_relaxed)) {
|
||||||
parseIPC(messageReceived);
|
break;
|
||||||
} catch (std::exception& e) {
|
}
|
||||||
spdlog::warn("Failed to parse IPC message: {}, reason: {}", messageReceived, e.what());
|
spdlog::error("Hyprland IPC: read failed: {}", std::strerror(errno));
|
||||||
} catch (...) {
|
break;
|
||||||
throw;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
std::this_thread::sleep_for(std::chrono::milliseconds(1));
|
pending.append(buffer.data(), static_cast<std::size_t>(bytes_read));
|
||||||
|
for (auto newline_pos = pending.find('\n'); newline_pos != std::string::npos;
|
||||||
|
newline_pos = pending.find('\n')) {
|
||||||
|
std::string messageReceived = pending.substr(0, newline_pos);
|
||||||
|
pending.erase(0, newline_pos + 1);
|
||||||
|
if (messageReceived.empty()) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
spdlog::debug("hyprland IPC received {}", messageReceived);
|
||||||
|
|
||||||
|
try {
|
||||||
|
parseIPC(messageReceived);
|
||||||
|
} catch (std::exception& e) {
|
||||||
|
spdlog::warn("Failed to parse IPC message: {}, reason: {}", messageReceived, e.what());
|
||||||
|
} catch (...) {
|
||||||
|
throw;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
{
|
||||||
|
std::lock_guard<std::mutex> lock(socketMutex_);
|
||||||
|
if (socketfd_ != -1) {
|
||||||
|
if (close(socketfd_) == -1) {
|
||||||
|
spdlog::error("Hyprland IPC: Couldn't close socket");
|
||||||
|
}
|
||||||
|
socketfd_ = -1;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
spdlog::debug("Hyprland IPC stopped");
|
spdlog::debug("Hyprland IPC stopped");
|
||||||
}
|
}
|
||||||
@@ -178,9 +232,9 @@ void IPC::unregisterForIPC(EventHandler* ev_handler) {
|
|||||||
std::string IPC::getSocket1Reply(const std::string& rq) {
|
std::string IPC::getSocket1Reply(const std::string& rq) {
|
||||||
// basically hyprctl
|
// basically hyprctl
|
||||||
|
|
||||||
const auto serverSocket = socket(AF_UNIX, SOCK_STREAM, 0);
|
ScopedFd serverSocket(socket(AF_UNIX, SOCK_STREAM, 0));
|
||||||
|
|
||||||
if (serverSocket < 0) {
|
if (serverSocket.get() < 0) {
|
||||||
throw std::runtime_error("Hyprland IPC: Couldn't open a socket (1)");
|
throw std::runtime_error("Hyprland IPC: Couldn't open a socket (1)");
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -203,12 +257,13 @@ std::string IPC::getSocket1Reply(const std::string& rq) {
|
|||||||
throw std::runtime_error("Hyprland IPC: Couldn't copy socket path (6)");
|
throw std::runtime_error("Hyprland IPC: Couldn't copy socket path (6)");
|
||||||
}
|
}
|
||||||
|
|
||||||
if (connect(serverSocket, reinterpret_cast<sockaddr*>(&serverAddress), sizeof(serverAddress)) <
|
if (connect(serverSocket.get(), reinterpret_cast<sockaddr*>(&serverAddress),
|
||||||
|
sizeof(serverAddress)) <
|
||||||
0) {
|
0) {
|
||||||
throw std::runtime_error("Hyprland IPC: Couldn't connect to " + socketPath + ". (3)");
|
throw std::runtime_error("Hyprland IPC: Couldn't connect to " + socketPath + ". (3)");
|
||||||
}
|
}
|
||||||
|
|
||||||
auto sizeWritten = write(serverSocket, rq.c_str(), rq.length());
|
auto sizeWritten = write(serverSocket.get(), rq.c_str(), rq.length());
|
||||||
|
|
||||||
if (sizeWritten < 0) {
|
if (sizeWritten < 0) {
|
||||||
spdlog::error("Hyprland IPC: Couldn't write (4)");
|
spdlog::error("Hyprland IPC: Couldn't write (4)");
|
||||||
@@ -219,17 +274,15 @@ std::string IPC::getSocket1Reply(const std::string& rq) {
|
|||||||
std::string response;
|
std::string response;
|
||||||
|
|
||||||
do {
|
do {
|
||||||
sizeWritten = read(serverSocket, buffer.data(), 8192);
|
sizeWritten = read(serverSocket.get(), buffer.data(), 8192);
|
||||||
|
|
||||||
if (sizeWritten < 0) {
|
if (sizeWritten < 0) {
|
||||||
spdlog::error("Hyprland IPC: Couldn't read (5)");
|
spdlog::error("Hyprland IPC: Couldn't read (5)");
|
||||||
close(serverSocket);
|
|
||||||
return "";
|
return "";
|
||||||
}
|
}
|
||||||
response.append(buffer.data(), sizeWritten);
|
response.append(buffer.data(), sizeWritten);
|
||||||
} while (sizeWritten > 0);
|
} while (sizeWritten > 0);
|
||||||
|
|
||||||
close(serverSocket);
|
|
||||||
return response;
|
return response;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user