Handle swaybar status line errors

The event loop API was redesigned to avoid race conditions as well.
Fixes #1583

include/swaybar/event_loop.h

@@ -13,11 +13,11 @@ void add_timer(timer_t timer,
 		void(*cb)(timer_t timer, void *data),
 		void *data);
 
-// Returns false if nothing exists, true otherwise
+// Remove the given event from the event loop
-bool remove_event(int fd);
+void remove_event(int fd);
 
-// Returns false if nothing exists, true otherwise
+// Remove the given timer from the event loop
-bool remove_timer(timer_t timer);
+void remove_timer(timer_t timer);
 
 // Blocks and returns after sending callbacks
 void event_loop_poll();

include/swaybar/status_line.h

@@ -48,6 +48,12 @@ struct status_line *init_status_line();
  */
 bool handle_status_line(struct bar *bar);
 
+/**
+ * This should be called if statusline input cannot be accessed.
+ * It will set an error statusline instead of using the status command
+ */
+void handle_status_hup(struct status_line *status);
+
 /**
  * Handle mouse clicks.
  */

swaybar/bar.c

@@ -274,6 +274,14 @@ static void respond_ipc(int fd, short mask, void *_bar) {
 
 static void respond_command(int fd, short mask, void *_bar) {
 	struct bar *bar = (struct bar *)_bar;
+	if (mask & POLLHUP) {
+		// Something's wrong with the command
+		handle_status_hup(bar->status);
+		dirty = true;
+		// We will stop watching the status line so swaybar won't
+		// flood the CPU with its HUPs
+		remove_event(fd);
+	}
 	dirty = handle_status_line(bar);
 }
 
@@ -286,7 +294,7 @@ static void respond_output(int fd, short mask, void *_output) {
 
 void bar_run(struct bar *bar) {
 	add_event(bar->ipc_event_socketfd, POLLIN, respond_ipc, bar);
-	add_event(bar->status_read_fd, POLLIN, respond_command, bar);
+	add_event(bar->status_read_fd, POLLIN | POLLHUP, respond_command, bar);
 
 	int i;
 	for (i = 0; i < bar->outputs->length; ++i) {

swaybar/event_loop.c

@@ -20,6 +20,19 @@ struct timer_item {
 	void *data;
 };
 
+enum state_item_flags {
+	ITEM_IS_FD,
+	ITEM_IS_TIMER,
+};
+
+struct state_item {
+	enum state_item_flags flags;
+	union {
+		int fd;
+		timer_t timer;
+	} inner;
+};
+
 static struct {
 	// The order of each must be kept consistent
 	struct {   /* pollfd array */
@@ -31,6 +44,9 @@ static struct {
 
 	// Timer list
 	list_t *timers;
+
+	// List of state changes at the end of each iteration
+	list_t *state;
 } event_loop;
 
 void add_timer(timer_t timer,
@@ -72,7 +88,7 @@ void add_event(int fd, short mask,
 	return;
 }
 
-bool remove_event(int fd) {
+static void _remove_event(int fd) {
 	int index = -1;
 	for (int i = 0; i < event_loop.fds.length; ++i) {
 		if (event_loop.fds.items[i].fd == fd) {
@@ -87,12 +103,16 @@ bool remove_event(int fd) {
 				sizeof(struct pollfd) * event_loop.fds.length - index);
 
 		list_del(event_loop.items, index);
-		return true;
-	} else {
-		return false;
 	}
 }
 
+void remove_event(int fd) {
+	struct state_item *item = malloc(sizeof(struct state_item));
+	item->flags = ITEM_IS_FD;
+	item->inner.fd = fd;
+	list_add(event_loop.state, item);
+}
+
 static int timer_item_timer_cmp(const void *_timer_item, const void *_timer) {
 	const struct timer_item *timer_item = _timer_item;
 	const timer_t *timer = _timer;
@@ -102,14 +122,19 @@ static int timer_item_timer_cmp(const void *_timer_item, const void *_timer) {
 		return -1;
 	}
 }
-bool remove_timer(timer_t timer) {
+static void _remove_timer(timer_t timer) {
 	int index = list_seq_find(event_loop.timers, timer_item_timer_cmp, &timer);
 	if (index != -1) {
 		free(event_loop.timers->items[index]);
 		list_del(event_loop.timers, index);
-		return true;
 	}
-	return false;
+}
+
+void remove_timer(timer_t timer) {
+	struct state_item *item = malloc(sizeof(struct state_item));
+	item->flags = ITEM_IS_TIMER;
+	item->inner.timer = timer;
+	list_add(event_loop.state, item);
 }
 
 void event_loop_poll() {
@@ -133,6 +158,19 @@ void event_loop_poll() {
 			item->cb(item->timer, item->data);
 		}
 	}
+
+	// Remove all requested items from the event loop. We can't do this
+	// during normal operation, as it will cause race conditions.
+	for (int i = 0; i < event_loop.state->length; ++i) {
+		struct state_item *item = event_loop.state->items[i];
+		if (item->flags == ITEM_IS_FD) {
+			_remove_event(item->inner.fd);
+		} else {
+			_remove_timer(item->inner.timer);
+		}
+		free(item);
+	}
+	event_loop.state->length = 0; // reset state list
 }
 
 void init_event_loop() {
@@ -141,4 +179,5 @@ void init_event_loop() {
 	event_loop.fds.items = malloc(event_loop.fds.capacity * sizeof(struct pollfd));
 	event_loop.items = create_list();
 	event_loop.timers = create_list();
+	event_loop.state = create_list();
 }

swaybar/status_line.c

@@ -511,6 +511,35 @@ bool handle_status_line(struct bar *bar) {
 	return dirty;
 }
 
+void handle_status_hup(struct status_line *line) {
+	// This is somewhat hacky, but free all previous status line state and
+	// then create a status block that displays an error string. This is so
+	// we can have pretty error colors.
+	sway_log(L_ERROR, "Replacing statusline with error string, as the status command has failed");
+	if (line->block_line) {
+		list_foreach(line->block_line, free_status_block);
+		list_free(line->block_line);
+	}
+	line->block_line = create_list();
+	struct status_block *new = calloc(1, sizeof(struct status_block));
+	new->full_text = strdup("ERROR: swaybar cannot access the statusline");
+	new->color = 0xff0000ff;
+	new->min_width = 0;
+	new->align = strdup("left");
+	new->markup = false;
+	new->separator = true;
+	new->separator_block_width = 9;
+	new->background = 0x0;
+	new->border = 0x0;
+	new->border_top = 1;
+	new->border_bottom = 1;
+	new->border_left = 1;
+	new->border_right = 1;
+	list_add(line->block_line, new);
+
+	line->protocol = I3BAR;
+}
+
 struct status_line *init_status_line() {
 	struct status_line *line = malloc(sizeof(struct status_line));
 	line->block_line = create_list();