From c2e07f2258a157718ee868c7f3d3c30de56cd9f7 Mon Sep 17 00:00:00 2001 From: Po Lu Date: Sun, 26 Jun 2022 10:20:35 +0800 Subject: [PATCH] Handle errors while sending client events asynchronously * src/xterm.c (xm_send_drop_message) (xm_send_top_level_enter_message, xm_send_drag_motion_message) (xm_send_top_level_leave_message, x_dnd_send_enter) (x_dnd_send_position, x_dnd_send_leave, x_dnd_send_drop): Avoid sync to check for errors while sending client events. (x_dnd_begin_drag_and_drop, handle_one_xevent, XTread_socket): Clean up failable requests. (x_request_can_fail): New functions. (x_clean_failable_requests, x_ignore_errors_for_next_request) (x_uncatch_errors): Clean up failable requests. (x_error_handler): If a request is allowed to fail, just return. (x_term_init): Set up new pointer. * src/xterm.h (N_FAILABLE_REQUESTS): New macro. (struct x_display_info): New field `failable_requests' and associated next pointer. --- src/xterm.c | 148 +++++++++++++++++++++++++++++++++++++++++++--------- src/xterm.h | 10 ++++ 2 files changed, 133 insertions(+), 25 deletions(-) diff --git a/src/xterm.c b/src/xterm.c index cd9645af074..8a42b77f511 100644 --- a/src/xterm.c +++ b/src/xterm.c @@ -1116,6 +1116,8 @@ static void x_scroll_bar_end_update (struct x_display_info *, struct scroll_bar #ifdef HAVE_X_I18N static int x_filter_event (struct x_display_info *, XEvent *); #endif +static void x_ignore_errors_for_next_request (struct x_display_info *); +static void x_clean_failable_requests (struct x_display_info *); static struct frame *x_tooltip_window_to_frame (struct x_display_info *, Window, bool *); @@ -2417,9 +2419,8 @@ xm_send_drop_message (struct x_display_info *dpyinfo, Window source, *((uint32_t *) &msg.xclient.data.b[12]) = dmsg->index_atom; *((uint32_t *) &msg.xclient.data.b[16]) = dmsg->source_window; - x_catch_errors (dpyinfo->display); + x_ignore_errors_for_next_request (dpyinfo); XSendEvent (dpyinfo->display, target, False, NoEventMask, &msg); - x_uncatch_errors (); } static void @@ -2444,9 +2445,8 @@ xm_send_top_level_enter_message (struct x_display_info *dpyinfo, Window source, msg.xclient.data.b[18] = 0; msg.xclient.data.b[19] = 0; - x_catch_errors (dpyinfo->display); + x_ignore_errors_for_next_request (dpyinfo); XSendEvent (dpyinfo->display, target, False, NoEventMask, &msg); - x_uncatch_errors (); } static void @@ -2475,9 +2475,8 @@ xm_send_drag_motion_message (struct x_display_info *dpyinfo, Window source, msg.xclient.data.b[18] = 0; msg.xclient.data.b[19] = 0; - x_catch_errors (dpyinfo->display); + x_ignore_errors_for_next_request (dpyinfo); XSendEvent (dpyinfo->display, target, False, NoEventMask, &msg); - x_uncatch_errors (); } static void @@ -2534,9 +2533,8 @@ xm_send_top_level_leave_message (struct x_display_info *dpyinfo, Window source, msg.xclient.data.b[18] = 0; msg.xclient.data.b[19] = 0; - x_catch_errors (dpyinfo->display); + x_ignore_errors_for_next_request (dpyinfo); XSendEvent (dpyinfo->display, target, False, NoEventMask, &msg); - x_uncatch_errors (); } static int @@ -4353,9 +4351,8 @@ x_dnd_send_enter (struct frame *f, Window target, int supported) so we don't have to set it again. */ x_dnd_init_type_lists = true; - x_catch_errors (dpyinfo->display); + x_ignore_errors_for_next_request (dpyinfo); XSendEvent (FRAME_X_DISPLAY (f), target, False, NoEventMask, &msg); - x_uncatch_errors (); } static void @@ -4416,9 +4413,8 @@ x_dnd_send_position (struct frame *f, Window target, int supported, x_dnd_pending_send_position = msg; else { - x_catch_errors (dpyinfo->display); + x_ignore_errors_for_next_request (dpyinfo); XSendEvent (FRAME_X_DISPLAY (f), target, False, NoEventMask, &msg); - x_uncatch_errors (); x_dnd_waiting_for_status_window = target; } @@ -4442,9 +4438,8 @@ x_dnd_send_leave (struct frame *f, Window target) x_dnd_waiting_for_status_window = None; - x_catch_errors (dpyinfo->display); + x_ignore_errors_for_next_request (dpyinfo); XSendEvent (FRAME_X_DISPLAY (f), target, False, NoEventMask, &msg); - x_uncatch_errors (); } static bool @@ -4475,9 +4470,8 @@ x_dnd_send_drop (struct frame *f, Window target, Time timestamp, if (supported >= 1) msg.xclient.data.l[2] = timestamp; - x_catch_errors (dpyinfo->display); + x_ignore_errors_for_next_request (dpyinfo); XSendEvent (FRAME_X_DISPLAY (f), target, False, NoEventMask, &msg); - x_uncatch_errors (); return true; } @@ -11625,6 +11619,9 @@ x_dnd_begin_drag_and_drop (struct frame *f, Time time, Atom xaction, #endif x_dnd_inside_handle_one_xevent = false; + /* Clean up any event handlers that are now out of date. */ + x_clean_failable_requests (FRAME_DISPLAY_INFO (f)); + /* The unblock_input below might try to read input, but XTread_socket does nothing inside a drag-and-drop event loop, so don't let it clear the pending_signals flag. */ @@ -16405,11 +16402,10 @@ handle_one_xevent (struct x_display_info *dpyinfo, { if (x_dnd_pending_send_position.type != 0) { - x_catch_errors (dpyinfo->display); + x_ignore_errors_for_next_request (dpyinfo); XSendEvent (dpyinfo->display, target, False, NoEventMask, &x_dnd_pending_send_position); - x_uncatch_errors (); } x_dnd_pending_send_position.type = 0; @@ -22274,6 +22270,8 @@ XTread_socket (struct terminal *terminal, struct input_event *hold_quit) && dpyinfo->display == x_dnd_finish_display))) return 0; + x_clean_failable_requests (dpyinfo); + block_input (); /* For debugging, this gives a way to fake an I/O error. */ @@ -22896,7 +22894,12 @@ x_error_catcher (Display *display, XErrorEvent *event, always skips an XSync to the server, and should be used only immediately after x_had_errors_p or x_check_errors, or when it is known that no requests have been made since the last x_catch_errors - call for DPY. */ + call for DPY. + + There is no need to use this mechanism for ignoring errors from + single asynchronous requests, such as sending a ClientMessage to a + window that might no longer exist. Use + x_ignore_errors_for_next_request instead. */ void x_catch_errors_with_handler (Display *dpy, x_special_error_handler handler, @@ -22921,6 +22924,76 @@ x_catch_errors (Display *dpy) x_catch_errors_with_handler (dpy, NULL, NULL); } +/* Return if errors for REQUEST should be ignored even if there is no + error handler applied. */ +static unsigned long * +x_request_can_fail (struct x_display_info *dpyinfo, + unsigned long request) +{ + unsigned long *failable_requests; + + for (failable_requests = dpyinfo->failable_requests; + failable_requests < dpyinfo->next_failable_request; + failable_requests++) + { + if (*failable_requests == request) + return failable_requests; + } + + return NULL; +} + +/* Remove outdated request serials from + dpyinfo->failable_requests. */ +static void +x_clean_failable_requests (struct x_display_info *dpyinfo) +{ + unsigned long *first, *last; + + last = dpyinfo->next_failable_request; + + for (first = dpyinfo->failable_requests; first < last; first++) + { + if (*first > LastKnownRequestProcessed (dpyinfo->display)) + break; + } + + memmove (&dpyinfo->failable_requests, first, + sizeof *first * (last - first)); + + dpyinfo->next_failable_request = (dpyinfo->failable_requests + + (last - first)); +} + +static void +x_ignore_errors_for_next_request (struct x_display_info *dpyinfo) +{ + unsigned long *request, *max; + + request = dpyinfo->next_failable_request; + max = dpyinfo->failable_requests + N_FAILABLE_REQUESTS; + + if (request > max) + { + /* There is no point in making this extra sync if all requests + are known to have been fully processed. */ + if ((LastKnownRequestProcessed (x_error_message->dpy) + != NextRequest (x_error_message->dpy) - 1)) + XSync (dpyinfo->display, False); + + x_clean_failable_requests (dpyinfo); + request = dpyinfo->next_failable_request; + } + + if (request >= max) + /* A request should always be made immediately after calling this + function. */ + emacs_abort (); + + *request = NextRequest (dpyinfo->display); + dpyinfo->next_failable_request++; +} + /* Undo the last x_catch_errors call. DPY should be the display that was passed to x_catch_errors. @@ -22949,6 +23022,7 @@ void x_uncatch_errors (void) { struct x_error_message_stack *tmp; + struct x_display_info *dpyinfo; /* In rare situations when running Emacs run in daemon mode, shutting down an emacsclient via delete-frame can cause @@ -22959,9 +23033,11 @@ x_uncatch_errors (void) block_input (); + dpyinfo = x_display_info_for_display (x_error_message->dpy); + /* The display may have been closed before this function is called. Check if it is still open before calling XSync. */ - if (x_display_info_for_display (x_error_message->dpy) != 0 + if (dpyinfo != 0 /* There is no point in making this extra sync if all requests are known to have been fully processed. */ && (LastKnownRequestProcessed (x_error_message->dpy) @@ -22970,7 +23046,10 @@ x_uncatch_errors (void) installed. */ && (NextRequest (x_error_message->dpy) > x_error_message->first_request)) - XSync (x_error_message->dpy, False); + { + XSync (x_error_message->dpy, False); + x_clean_failable_requests (dpyinfo); + } tmp = x_error_message; x_error_message = x_error_message->prev; @@ -23280,9 +23359,8 @@ static int x_error_handler (Display *display, XErrorEvent *event) { struct x_error_message_stack *stack; -#ifdef HAVE_XINPUT2 struct x_display_info *dpyinfo; -#endif + unsigned long *fail, *last; #if defined USE_GTK && defined HAVE_GTK3 if ((event->error_code == BadMatch @@ -23291,12 +23369,30 @@ x_error_handler (Display *display, XErrorEvent *event) return 0; #endif + dpyinfo = x_display_info_for_display (display); + + if (dpyinfo) + { + fail = x_request_can_fail (dpyinfo, event->serial); + + if (fail) + { + /* Now that this request has been handled, remove it from + the list of requests that can fail. */ + last = dpyinfo->next_failable_request; + memmove (&dpyinfo->failable_requests, fail, + sizeof *fail * (last - fail)); + dpyinfo->next_failable_request = (dpyinfo->failable_requests + + (last - fail)); + + return 0; + } + } + /* If we try to ungrab or grab a device that doesn't exist anymore (that happens a lot in xmenu.c), just ignore the error. */ #ifdef HAVE_XINPUT2 - dpyinfo = x_display_info_for_display (display); - /* 51 is X_XIGrabDevice and 52 is X_XIUngrabDevice. 53 is X_XIAllowEvents. We handle errors from that here to avoid @@ -26272,6 +26368,8 @@ x_term_init (Lisp_Object display_name, char *xrm_option, char *resource_name) dpyinfo = xzalloc (sizeof *dpyinfo); terminal = x_create_terminal (dpyinfo); + dpyinfo->next_failable_request = dpyinfo->failable_requests; + { struct x_display_info *share; diff --git a/src/xterm.h b/src/xterm.h index f136b6b97fe..ff81babc337 100644 --- a/src/xterm.h +++ b/src/xterm.h @@ -75,6 +75,9 @@ typedef GtkWidget *xt_or_gtk_widget; #endif #endif /* USE_GTK */ +/* Number of "failable requests" to store. */ +#define N_FAILABLE_REQUESTS 128 + #ifdef USE_CAIRO #include #ifdef CAIRO_HAS_PDF_SURFACE @@ -742,6 +745,13 @@ struct x_display_info RRScreenChangeNotify. */ int screen_mm_width; int screen_mm_height; + + /* Circular buffer of request serials to ignore inside an error + handler in increasing order. */ + unsigned long failable_requests[N_FAILABLE_REQUESTS]; + + /* Pointer to the next request in `failable_requests'. */ + unsigned long *next_failable_request; }; #ifdef HAVE_X_I18N -- 2.39.5