Skip to content

Commit b345abd

Browse files
committed
fix: Make signal handling safer
std::signal is very restrictive about what can be done inside the signal handler. Posix adds some additional functions that are safe to call, but it is still pretty limited. This changes waybar to use the "self pipe trick" where all the signal handler does is write a byte into a pipe, and then we have a slot run on the Gtk main loop that reads from that pipe and actually handles the signal.
1 parent ac08b75 commit b345abd

File tree

1 file changed

+79
-22
lines changed

1 file changed

+79
-22
lines changed

src/main.cpp

+79-22
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
#include <spdlog/spdlog.h>
22
#include <sys/types.h>
33
#include <sys/wait.h>
4+
#include <unistd.h>
45

6+
#include <cerrno>
57
#include <csignal>
68
#include <list>
79
#include <mutex>
@@ -70,35 +72,90 @@ void startSignalThread() {
7072
}
7173
}
7274

75+
static int signal_fds[2];
76+
77+
extern "C" void raw_signal_handler(int signum) {
78+
// just write a single byte to be more reliable
79+
auto sigbyte = static_cast<unsigned char>(signum);
80+
// The only way I know of that this could fail is if we have queued a truly
81+
// remarkable number of signals without handling them
82+
write(signal_fds[1], &sigbyte, 1);
83+
}
84+
85+
static void dispatch_signal(unsigned char signum) {
86+
auto client = waybar::Client::inst();
87+
if (client == nullptr) {
88+
// TODO: should we do something for SIGINT?
89+
return;
90+
}
91+
if (signum == SIGUSR1) {
92+
for (auto& bar : client->bars) {
93+
bar->toggle();
94+
}
95+
} else if (signum == SIGUSR2) {
96+
spdlog::info("Reloading...");
97+
reload = true;
98+
client->reset();
99+
} else if (signum == SIGINT) {
100+
spdlog::info("Quitting.");
101+
reload = false;
102+
client->reset();
103+
} else if (signum > SIGRTMIN && signum <= SIGRTMAX) {
104+
for (auto& bar : client->bars) {
105+
bar->handleSignal(signum);
106+
}
107+
}
108+
}
109+
110+
static bool handle_signals(Glib::IOCondition cond) {
111+
unsigned char buf[16];
112+
const size_t bufsize = sizeof(buf);
113+
while (true) {
114+
auto n = read(signal_fds[0], buf, bufsize);
115+
if (n < 0) {
116+
if (errno == EAGAIN) {
117+
return true;
118+
}
119+
throw std::system_error(errno, std::system_category());
120+
}
121+
for (int i = 0; i < n; i++) {
122+
dispatch_signal(buf[i]);
123+
}
124+
if (static_cast<size_t>(n) < bufsize) {
125+
return true;
126+
}
127+
}
128+
}
129+
73130
int main(int argc, char* argv[]) {
74131
try {
75132
auto* client = waybar::Client::inst();
76133

77-
std::signal(SIGUSR1, [](int /*signal*/) {
78-
for (auto& bar : waybar::Client::inst()->bars) {
79-
bar->toggle();
80-
}
81-
});
134+
// It would be nice if we could use g_unix_signal_add, but unfortunately, that
135+
// doesn't support RT signals
136+
if (pipe(signal_fds)) {
137+
throw std::runtime_error("Failed to create pipe");
138+
}
139+
std::signal(SIGUSR1, &raw_signal_handler);
140+
std::signal(SIGUSR2, &raw_signal_handler);
141+
std::signal(SIGINT, &raw_signal_handler);
142+
for (int sig = SIGRTMIN + 1; sig <= SIGRTMAX; ++sig) {
143+
std::signal(sig, &raw_signal_handler);
144+
}
82145

83-
std::signal(SIGUSR2, [](int /*signal*/) {
84-
spdlog::info("Reloading...");
85-
reload = true;
86-
waybar::Client::inst()->reset();
87-
});
146+
auto signalPipe = Glib::IOChannel::create_from_fd(signal_fds[0]);
147+
// Make the read side, non-blocking
148+
int pipe_flags = fcntl(signal_fds[0], F_GETFL);
149+
if (pipe_flags != -1) {
150+
pipe_flags = fcntl(signal_fds[0], F_SETFL, pipe_flags | O_NONBLOCK);
151+
}
152+
if (pipe_flags == -1) {
153+
throw std::runtime_error("Failed to set pipe to nonblocking mode");
154+
}
88155

89-
std::signal(SIGINT, [](int /*signal*/) {
90-
spdlog::info("Quitting.");
91-
reload = false;
92-
waybar::Client::inst()->reset();
93-
});
156+
Glib::signal_io().connect(sigc::ptr_fun(handle_signals), signal_fds[0],
157+
Glib::IOCondition::IO_IN);
94158

95-
for (int sig = SIGRTMIN + 1; sig <= SIGRTMAX; ++sig) {
96-
std::signal(sig, [](int sig) {
97-
for (auto& bar : waybar::Client::inst()->bars) {
98-
bar->handleSignal(sig);
99-
}
100-
});
101-
}
102159
startSignalThread();
103160

104161
auto ret = 0;

0 commit comments

Comments
 (0)