From aca3fdcef7bfcb1c3ce65cf87413fa6ab280d183 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 14 Dec 2022 14:23:55 +0000 Subject: [PATCH] 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 --- src/common.h | 44 ++++++++++++++++++------------ src/event.c | 2 +- src/picom.c | 76 ++++++++++++++++++++++++++++++++-------------------- src/picom.h | 2 +- src/x.c | 10 ++++++- src/x.h | 2 ++ 6 files changed, 87 insertions(+), 49 deletions(-) diff --git a/src/common.h b/src/common.h index c7731876..863e14ee 100644 --- a/src/common.h +++ b/src/common.h @@ -36,9 +36,9 @@ #include #include #include -#include #include #include +#include #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 next 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 XRectangle-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; } /** diff --git a/src/event.c b/src/event.c index c4a62e8f..89f9624a 100644 --- a/src/event.c +++ b/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); diff --git a/src/picom.c b/src/picom.c index f922978a..ba97f4cc 100644 --- a/src/picom.c +++ b/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); diff --git a/src/picom.h b/src/picom.h index 7ee289bd..b5a1e8a7 100644 --- a/src/picom.h +++ b/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); diff --git a/src/x.c b/src/x.c index e71f1ee5..e5983452 100644 --- a/src/x.c +++ b/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); } /* diff --git a/src/x.h b/src/x.h index a1928862..78efea39 100644 --- a/src/x.h +++ b/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