From 50b3e9d23dbdcd8809aaa8a95f62c2df33868d25 Mon Sep 17 00:00:00 2001 From: Po Lu Date: Sat, 2 Jul 2022 16:41:45 +0800 Subject: [PATCH] Completely get rid of races during Motif drag window creation * src/xterm.c (x_special_window_exists_p): New function. (xm_get_drag_window_1): Rework workflow and display grabbing. --- src/xterm.c | 179 ++++++++++++++++++++++++++++------------------------ 1 file changed, 96 insertions(+), 83 deletions(-) diff --git a/src/xterm.c b/src/xterm.c index 245ffedb800..2629997f2ac 100644 --- a/src/xterm.c +++ b/src/xterm.c @@ -1838,10 +1838,47 @@ xm_drag_window_io_error_handler (Display *dpy) siglongjmp (x_dnd_disconnect_handler, 1); } +/* Determine whether or not WINDOW exists on DPYINFO by selecting for + input from it. */ +static bool +x_special_window_exists_p (struct x_display_info *dpyinfo, + Window window) +{ + bool rc; + + x_catch_errors (dpyinfo->display); + XSelectInput (dpyinfo->display, window, + StructureNotifyMask); + rc = !x_had_errors_p (dpyinfo->display); + x_uncatch_errors_after_check (); + + return rc; +} + +/* Drag window creation strategy (very tricky, but race-free): + + First look for _MOTIF_DRAG_WINDOW. If it is already present, + return it immediately to avoid the overhead of new display + connections. + + Otherwise, create a new connection to the display. In that + connection, create a window, which will be the new drag window. Set + the client disconnect mode of the new connection to + RetainPermanent, and close it. + + Grab the current display. Look up _MOTIF_DRAG_WINDOW, the current + drag window. If it exists (which means _MOTIF_DRAG_WINDOW was + created between the first step and now), kill the client that + created the new drag window to free the client slot on the X + server. Otherwise, set _MOTIF_DRAG_WINDOW to the new drag window. + + Ungrab the display and return whichever window is currently in + _MOTIF_DRAG_WINDOW. */ + static Window xm_get_drag_window_1 (struct x_display_info *dpyinfo) { - Atom actual_type, _MOTIF_DRAG_WINDOW; + Atom actual_type; int rc, actual_format; unsigned long nitems, bytes_remaining; unsigned char *tmp_data = NULL; @@ -1851,9 +1888,9 @@ xm_get_drag_window_1 (struct x_display_info *dpyinfo) Emacs_XErrorHandler old_handler; Emacs_XIOErrorHandler old_io_handler; - /* These are volatile because GCC mistakenly warns about them being + /* This is volatile because GCC mistakenly warns about them being clobbered by longjmp. */ - volatile bool error, created; + volatile bool error; drag_window = None; rc = XGetWindowProperty (dpyinfo->display, dpyinfo->root_window, @@ -1862,26 +1899,20 @@ xm_get_drag_window_1 (struct x_display_info *dpyinfo) &actual_format, &nitems, &bytes_remaining, &tmp_data) == Success; - if (rc) + if (rc && actual_type == XA_WINDOW + && actual_format == 32 && nitems == 1 + && tmp_data) { - if (actual_type == XA_WINDOW - && actual_format == 32 && nitems == 1) - { - drag_window = *(Window *) tmp_data; - x_catch_errors (dpyinfo->display); - XSelectInput (dpyinfo->display, drag_window, - StructureNotifyMask); - rc = !x_had_errors_p (dpyinfo->display); - x_uncatch_errors_after_check (); + drag_window = *(Window *) tmp_data; + rc = x_special_window_exists_p (dpyinfo, drag_window); - if (!rc) - drag_window = None; - } - - if (tmp_data) - XFree (tmp_data); + if (!rc) + drag_window = None; } + if (tmp_data) + XFree (tmp_data); + if (drag_window == None) { block_input (); @@ -1910,74 +1941,22 @@ xm_get_drag_window_1 (struct x_display_info *dpyinfo) error = false; xm_drag_window_error = &error; - XGrabServer (temp_display); XSetCloseDownMode (temp_display, RetainPermanent); - old_handler = XSetErrorHandler (xm_drag_window_error_handler); - _MOTIF_DRAG_WINDOW = XInternAtom (temp_display, - "_MOTIF_DRAG_WINDOW", False); - - if (error) - goto give_up; - - /* Some other program might've created a drag window between now - and when we first looked. Use that if it exists. */ - - tmp_data = NULL; - rc = XGetWindowProperty (temp_display, DefaultRootWindow (temp_display), - _MOTIF_DRAG_WINDOW, 0, 1, False, XA_WINDOW, - &actual_type, &actual_format, &nitems, - &bytes_remaining, &tmp_data) == Success; - - if (rc && actual_type == XA_WINDOW - && actual_format == 32 && nitems == 1 - && tmp_data) - drag_window = *(Window *) tmp_data; - - if (tmp_data) - XFree (tmp_data); - - error = false; - - if (drag_window == None) - { - created = true; - - attrs.override_redirect = True; - drag_window = XCreateWindow (temp_display, DefaultRootWindow (temp_display), - -1, -1, 1, 1, 0, CopyFromParent, InputOnly, - CopyFromParent, CWOverrideRedirect, &attrs); - XChangeProperty (temp_display, DefaultRootWindow (temp_display), - _MOTIF_DRAG_WINDOW, XA_WINDOW, 32, PropModeReplace, - (unsigned char *) &drag_window, 1); - } - else - created = false; + attrs.override_redirect = True; + drag_window = XCreateWindow (temp_display, DefaultRootWindow (temp_display), + -1, -1, 1, 1, 0, CopyFromParent, InputOnly, + CopyFromParent, CWOverrideRedirect, &attrs); /* Handle all errors now. */ XSync (temp_display, False); - give_up: - /* Some part of the drag window creation process failed, so - punt. */ + punt. Release all resources too. */ if (error) { XSetCloseDownMode (temp_display, DestroyAll); - - /* If the drag window was actually created, delete it now. - Probably, a BadAlloc happened during the XChangeProperty - request. */ - if (created) - { - if (drag_window != None) - XDestroyWindow (temp_display, drag_window); - - XDeleteProperty (temp_display, DefaultRootWindow (temp_display), - _MOTIF_DRAG_WINDOW); - } - drag_window = None; } @@ -1995,15 +1974,49 @@ xm_get_drag_window_1 (struct x_display_info *dpyinfo) /* Make sure the drag window created is actually valid for the current display, and the XOpenDisplay above didn't accidentally connect to some other display. */ - x_catch_errors (dpyinfo->display); - XSelectInput (dpyinfo->display, drag_window, StructureNotifyMask); - rc = !x_had_errors_p (dpyinfo->display); - x_uncatch_errors_after_check (); + if (!x_special_window_exists_p (dpyinfo, drag_window)) + drag_window = None; unblock_input (); - /* We connected to the wrong display, so just give up. */ - if (!rc) - drag_window = None; + if (drag_window != None) + { + XGrabServer (dpyinfo->display); + + x_catch_errors (dpyinfo->display); + tmp_data = NULL; + + rc = XGetWindowProperty (dpyinfo->display, dpyinfo->root_window, + dpyinfo->Xatom_MOTIF_DRAG_WINDOW, + 0, 1, False, XA_WINDOW, &actual_type, + &actual_format, &nitems, &bytes_remaining, + &tmp_data) == Success; + + if (rc && actual_type == XA_WINDOW + && actual_format == 32 && nitems == 1 + && tmp_data + && x_special_window_exists_p (dpyinfo, + *(Window *) tmp_data)) + { + /* Kill the client now to avoid leaking a client slot, + which is a limited resource. */ + XKillClient (dpyinfo->display, drag_window); + drag_window = *(Window *) tmp_data; + } + else + XChangeProperty (dpyinfo->display, dpyinfo->root_window, + dpyinfo->Xatom_MOTIF_DRAG_WINDOW, + XA_WINDOW, 32, PropModeReplace, + (unsigned char *) &drag_window, 1); + + if (tmp_data) + XFree (tmp_data); + + if (x_had_errors_p (dpyinfo->display)) + drag_window = None; + x_uncatch_errors (); + + XUngrabServer (dpyinfo->display); + } } return drag_window; -- 2.39.5