From e0107c4dd7baec5845d16fa151b18f33159c3729 Mon Sep 17 00:00:00 2001 From: Scott Anderson Date: Sat, 21 Apr 2018 13:44:32 +1200 Subject: [PATCH 1/4] Always send POLLHUP and POLLERR with event loop --- swaybar/event_loop.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/swaybar/event_loop.c b/swaybar/event_loop.c index 748372edf..1e0d426b8 100644 --- a/swaybar/event_loop.c +++ b/swaybar/event_loop.c @@ -118,7 +118,10 @@ void event_loop_poll() { struct pollfd pfd = event_loop.fds.items[i]; struct event_item *item = (struct event_item *)event_loop.items->items[i]; - if (pfd.revents & pfd.events) { + // Always send these events + unsigned events = pfd.events | POLLHUP | POLLERR; + + if (pfd.revents & events) { item->cb(pfd.fd, pfd.revents, item->data); } } From 9a3fb33e33b2503809d9d3a1b0d10c21bc112a80 Mon Sep 17 00:00:00 2001 From: Scott Anderson Date: Sat, 21 Apr 2018 14:38:34 +1200 Subject: [PATCH 2/4] Change remove_event logic We defer the removal of entries until after the poll loop has finished. Otherwise we may end up adjusting the poll array while we're still reading from it, causing us to skip events. --- swaybar/event_loop.c | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/swaybar/event_loop.c b/swaybar/event_loop.c index 1e0d426b8..bc4053beb 100644 --- a/swaybar/event_loop.c +++ b/swaybar/event_loop.c @@ -72,24 +72,18 @@ void add_event(int fd, short mask, } bool remove_event(int fd) { - int index = -1; + /* + * Instead of removing events immediately, we mark them for deletion + * and clean them up later. This is so we can call remove_event inside + * an event callback safely. + */ for (int i = 0; i < event_loop.fds.length; ++i) { if (event_loop.fds.items[i].fd == fd) { - index = i; + event_loop.fds.items[i].fd = -1; + return true; } } - if (index != -1) { - free(event_loop.items->items[index]); - - --event_loop.fds.length; - memmove(&event_loop.fds.items[index], &event_loop.fds.items[index + 1], - sizeof(struct pollfd) * event_loop.fds.length - index); - - list_del(event_loop.items, index); - return true; - } else { - return false; - } + return false; } static int timer_item_timer_cmp(const void *_timer_item, const void *_timer) { @@ -126,6 +120,21 @@ void event_loop_poll() { } } + // Cleanup removed events + int end = 0; + int length = event_loop.fds.length; + for (int i = 0; i < length; ++i) { + if (event_loop.fds.items[i].fd == -1) { + free(event_loop.items->items[i]); + list_del(event_loop.items, i); + --event_loop.fds.length; + } else if (end != i) { + event_loop.fds.items[end++] = event_loop.fds.items[i]; + } else { + end = i + 1; + } + } + // check timers // not tested, but seems to work for (int i = 0; i < event_loop.timers->length; ++i) { From 2ebb6073b7eb052984008110e3df4469d2d9c596 Mon Sep 17 00:00:00 2001 From: Scott Anderson Date: Sat, 21 Apr 2018 14:39:46 +1200 Subject: [PATCH 3/4] Remove status command event on error This prevents very high CPU load when the status command dies, and poll continuously awoken with POLLHUP. --- swaybar/bar.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/swaybar/bar.c b/swaybar/bar.c index d51c4ec71..10a840cc0 100644 --- a/swaybar/bar.c +++ b/swaybar/bar.c @@ -418,7 +418,11 @@ static void ipc_in(int fd, short mask, void *_bar) { static void status_in(int fd, short mask, void *_bar) { struct swaybar *bar = (struct swaybar *)_bar; - if (status_handle_readable(bar->status)) { + if (mask & (POLLHUP | POLLERR)) { + status_error(bar->status, "[error reading from status command]"); + render_all_frames(bar); + remove_event(fd); + } else if (status_handle_readable(bar->status)) { render_all_frames(bar); } } From c63554885e4aae4e1c8f97ffbd53160f3eb99510 Mon Sep 17 00:00:00 2001 From: Scott Anderson Date: Sat, 21 Apr 2018 14:45:34 +1200 Subject: [PATCH 4/4] Remove void * casts They're pointless. --- swaybar/bar.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/swaybar/bar.c b/swaybar/bar.c index 10a840cc0..669cb11ac 100644 --- a/swaybar/bar.c +++ b/swaybar/bar.c @@ -401,23 +401,23 @@ void bar_setup(struct swaybar *bar, render_all_frames(bar); } -static void display_in(int fd, short mask, void *_bar) { - struct swaybar *bar = (struct swaybar *)_bar; +static void display_in(int fd, short mask, void *data) { + struct swaybar *bar = data; if (wl_display_dispatch(bar->display) == -1) { bar_teardown(bar); exit(0); } } -static void ipc_in(int fd, short mask, void *_bar) { - struct swaybar *bar = (struct swaybar *)_bar; +static void ipc_in(int fd, short mask, void *data) { + struct swaybar *bar = data; if (handle_ipc_readable(bar)) { render_all_frames(bar); } } -static void status_in(int fd, short mask, void *_bar) { - struct swaybar *bar = (struct swaybar *)_bar; +static void status_in(int fd, short mask, void *data) { + struct swaybar *bar = data; if (mask & (POLLHUP | POLLERR)) { status_error(bar->status, "[error reading from status command]"); render_all_frames(bar);