core: expand X error handling

We used to have a list of X errors we should ignore in case they do
occur. This commit expands that functionality to also allow us aborting
on certain errors.

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>

src/common.h

@@ -36,9 +36,9 @@
 #include <X11/Xlib.h>
 #include <ev.h>
 #include <pixman.h>
-#include <xcb/xproto.h>
 #include <xcb/render.h>
 #include <xcb/sync.h>
+#include <xcb/xproto.h>
 
 #include "uthash_extra.h"
 #ifdef CONFIG_OPENGL
@@ -55,11 +55,11 @@
 #include "backend/driver.h"
 #include "compiler.h"
 #include "config.h"
+#include "list.h"
 #include "region.h"
+#include "render.h"
 #include "types.h"
 #include "utils.h"
-#include "list.h"
-#include "render.h"
 #include "win_defs.h"
 #include "x.h"
 
@@ -83,10 +83,17 @@ struct glx_session;
 struct atom;
 struct conv;
 
-typedef struct _ignore {
-	struct _ignore *next;
+enum pending_reply_action {
+	PENDING_REPLY_ACTION_IGNORE,
+	PENDING_REPLY_ACTION_ABORT,
+	PENDING_REPLY_ACTION_DEBUG_ABORT,
+};
+
+typedef struct pending_reply {
+	struct pending_reply *next;
 	unsigned long sequence;
-} ignore_t;
+	enum pending_reply_action action;
+} pending_reply_t;
 
 #ifdef CONFIG_OPENGL
 #ifdef DEBUG_GLX_DEBUG_CONTEXT
@@ -256,18 +263,18 @@ typedef struct session {
 	/// Time of last fading. In milliseconds.
 	long long fade_time;
 	/// Head pointer of the error ignore linked list.
-	ignore_t *ignore_head;
+	pending_reply_t *pending_reply_head;
 	/// Pointer to the <code>next</code> member of tail element of the error
 	/// ignore linked list.
-	ignore_t **ignore_tail;
+	pending_reply_t **pending_reply_tail;
 	// Cached blur convolution kernels.
 	struct x_convolution_kernel **blur_kerns_cache;
 	/// If we should quit
-	bool quit:1;
+	bool quit : 1;
 	// TODO(yshui) use separate flags for dfferent kinds of updates so we don't
 	// waste our time.
 	/// Whether there are pending updates, like window creation, etc.
-	bool pending_updates:1;
+	bool pending_updates : 1;
 
 	// === Expose event related ===
 	/// Pointer to an array of <code>XRectangle</code>-s of exposed region.
@@ -468,18 +475,21 @@ static inline bool bkend_use_glx(session_t *ps) {
 	return BKEND_GLX == ps->o.backend || BKEND_XR_GLX_HYBRID == ps->o.backend;
 }
 
-static void set_ignore(session_t *ps, unsigned long sequence) {
-	if (ps->o.show_all_xerrors)
+static void set_ignore(session_t *ps, uint32_t sequence) {
+	if (ps->o.show_all_xerrors) {
 		return;
+	}
 
-	auto i = cmalloc(ignore_t);
-	if (!i)
-		return;
+	auto i = cmalloc(pending_reply_t);
+	if (!i) {
+		abort();
+	}
 
 	i->sequence = sequence;
 	i->next = 0;
-	*ps->ignore_tail = i;
-	ps->ignore_tail = &i->next;
+	i->action = PENDING_REPLY_ACTION_IGNORE;
+	*ps->pending_reply_tail = i;
+	ps->pending_reply_tail = &i->next;
 }
 
 /**

src/event.c

@@ -665,7 +665,7 @@ ev_selection_clear(session_t *ps, xcb_selection_clear_event_t attr_unused *ev) {
 
 void ev_handle(session_t *ps, xcb_generic_event_t *ev) {
 	if ((ev->response_type & 0x7f) != KeymapNotify) {
-		discard_ignore(ps, ev->full_sequence);
+		discard_pending(ps, ev->full_sequence);
 	}
 
 	xcb_window_t wid = ev_window(ps, ev);

src/picom.c

@@ -282,14 +282,14 @@ static bool run_fade(session_t *ps, struct managed_win **_w, long long steps) {
 
 // === Error handling ===
 
-void discard_ignore(session_t *ps, unsigned long sequence) {
-	while (ps->ignore_head) {
-		if (sequence > ps->ignore_head->sequence) {
-			ignore_t *next = ps->ignore_head->next;
-			free(ps->ignore_head);
-			ps->ignore_head = next;
-			if (!ps->ignore_head) {
-				ps->ignore_tail = &ps->ignore_head;
+void discard_pending(session_t *ps, uint32_t sequence) {
+	while (ps->pending_reply_head) {
+		if (sequence > ps->pending_reply_head->sequence) {
+			auto next = ps->pending_reply_head->next;
+			free(ps->pending_reply_head);
+			ps->pending_reply_head = next;
+			if (!ps->pending_reply_head) {
+				ps->pending_reply_tail = &ps->pending_reply_head;
 			}
 		} else {
 			break;
@@ -297,13 +297,28 @@ void discard_ignore(session_t *ps, unsigned long sequence) {
 	}
 }
 
-static int should_ignore(session_t *ps, uint32_t sequence) {
+static void handle_error(session_t *ps, xcb_generic_error_t *ev) {
 	if (ps == NULL) {
 		// Do not ignore errors until the session has been initialized
-		return false;
+		return;
 	}
-	discard_ignore(ps, sequence);
-	return ps->ignore_head && ps->ignore_head->sequence == sequence;
+	discard_pending(ps, ev->full_sequence);
+	if (ps->pending_reply_head && ps->pending_reply_head->sequence == ev->full_sequence) {
+		if (ps->pending_reply_head->action != PENDING_REPLY_ACTION_IGNORE) {
+			x_log_error(LOG_LEVEL_ERROR, ev->full_sequence, ev->major_code,
+			            ev->minor_code, ev->error_code);
+		}
+		switch (ps->pending_reply_head->action) {
+		case PENDING_REPLY_ACTION_ABORT:
+			log_fatal("An unrecoverable X error occurred, aborting...");
+			abort();
+		case PENDING_REPLY_ACTION_DEBUG_ABORT: assert(false); break;
+		case PENDING_REPLY_ACTION_IGNORE: break;
+		}
+		return;
+	}
+	x_log_error(LOG_LEVEL_WARN, ev->full_sequence, ev->major_code, ev->minor_code,
+	            ev->error_code);
 }
 
 // === Windows ===
@@ -964,9 +979,13 @@ void root_damaged(session_t *ps) {
  * Xlib error handler function.
  */
 static int xerror(Display attr_unused *dpy, XErrorEvent *ev) {
-	if (!should_ignore(ps_g, (uint32_t)ev->serial)) {
-		x_print_error(ev->serial, ev->request_code, ev->minor_code, ev->error_code);
-	}
+	// Fake a xcb error, fill in just enough information
+	xcb_generic_error_t xcb_err;
+	xcb_err.full_sequence = (uint32_t)ev->serial;
+	xcb_err.major_code = ev->request_code;
+	xcb_err.minor_code = ev->minor_code;
+	xcb_err.error_code = ev->error_code;
+	handle_error(ps_g, &xcb_err);
 	return 0;
 }
 
@@ -974,10 +993,7 @@ static int xerror(Display attr_unused *dpy, XErrorEvent *ev) {
  * XCB error handler function.
  */
 void ev_xcb_error(session_t *ps, xcb_generic_error_t *err) {
-	if (!should_ignore(ps, err->full_sequence)) {
-		x_print_error(err->full_sequence, err->major_code, err->minor_code,
-		              err->error_code);
-	}
+	handle_error(ps, err);
 }
 
 /**
@@ -1678,8 +1694,8 @@ static session_t *session_init(int argc, char **argv, Display *dpy,
 	    .redirected = false,
 	    .alpha_picts = NULL,
 	    .fade_time = 0L,
-	    .ignore_head = NULL,
-	    .ignore_tail = NULL,
+	    .pending_reply_head = NULL,
+	    .pending_reply_tail = NULL,
 	    .quit = false,
 
 	    .expose_rects = NULL,
@@ -1741,7 +1757,7 @@ static session_t *session_init(int argc, char **argv, Display *dpy,
 	ps->loop = EV_DEFAULT;
 	pixman_region32_init(&ps->screen_reg);
 
-	ps->ignore_tail = &ps->ignore_head;
+	ps->pending_reply_tail = &ps->pending_reply_head;
 
 	ps->o.show_all_xerrors = all_xerrors;
 
@@ -2330,26 +2346,28 @@ static void session_destroy(session_t *ps) {
 
 	// Free ignore linked list
 	{
-		ignore_t *next = NULL;
-		for (ignore_t *ign = ps->ignore_head; ign; ign = next) {
+		pending_reply_t *next = NULL;
+		for (auto ign = ps->pending_reply_head; ign; ign = next) {
 			next = ign->next;
 
 			free(ign);
 		}
 
 		// Reset head and tail
-		ps->ignore_head = NULL;
-		ps->ignore_tail = &ps->ignore_head;
+		ps->pending_reply_head = NULL;
+		ps->pending_reply_tail = &ps->pending_reply_head;
 	}
 
 	// Free tgt_{buffer,picture} and root_picture
-	if (ps->tgt_buffer.pict == ps->tgt_picture)
+	if (ps->tgt_buffer.pict == ps->tgt_picture) {
 		ps->tgt_buffer.pict = XCB_NONE;
+	}
 
-	if (ps->tgt_picture == ps->root_picture)
+	if (ps->tgt_picture == ps->root_picture) {
 		ps->tgt_picture = XCB_NONE;
-	else
+	} else {
 		free_picture(ps->c, &ps->tgt_picture);
+	}
 
 	free_picture(ps->c, &ps->root_picture);
 	free_paint(ps, &ps->tgt_buffer);

src/picom.h

@@ -46,7 +46,7 @@ void cxinerama_upd_scrs(session_t *ps);
 
 void queue_redraw(session_t *ps);
 
-void discard_ignore(session_t *ps, unsigned long sequence);
+void discard_pending(session_t *ps, uint32_t sequence);
 
 void set_root_flags(session_t *ps, uint64_t flags);
 

src/x.c

@@ -562,8 +562,16 @@ _x_strerror(unsigned long serial, uint8_t major, uint16_t minor, uint8_t error_c
 /**
  * Log a X11 error
  */
+void x_log_error(enum log_level level, unsigned long serial, uint8_t major,
+                 uint16_t minor, uint8_t error_code) {
+	if (unlikely(level >= log_get_level_tls())) {
+		log_printf(tls_logger, level, __func__, "%s",
+		           _x_strerror(serial, major, minor, error_code));
+	}
+}
+
 void x_print_error(unsigned long serial, uint8_t major, uint16_t minor, uint8_t error_code) {
-	log_debug("%s", _x_strerror(serial, major, minor, error_code));
+	x_log_error(LOG_LEVEL_DEBUG, serial, major, minor, error_code);
 }
 
 /*

src/x.h

@@ -231,6 +231,8 @@ void x_clear_picture_clip_region(xcb_connection_t *, xcb_render_picture_t pict);
  * Log a X11 error
  */
 void x_print_error(unsigned long serial, uint8_t major, uint16_t minor, uint8_t error_code);
+void x_log_error(enum log_level level, unsigned long serial, uint8_t major,
+                 uint16_t minor, uint8_t error_code);
 
 /*
  * Convert a xcb_generic_error_t to a string that describes the error