From ca4e1fd4c9e0c2c466a5602c349fe6abb3d01c10 Mon Sep 17 00:00:00 2001 From: "Jasper St. Pierre" Date: Mon, 6 May 2013 15:45:24 -0400 Subject: [PATCH] window: Eliminate a potential race condition with _NET_WM_MOVERESIZE Clients using _NET_WM_MOVERESIZE to start a drag operation may encounter a race condition if the user presses and releases a mouse button very fast, getting "stuck" in a grab state. While this is easily fixed with the user pressing the button or hitting Escape as the EWMH spec suggests, its's still a bit of annoyance for users. After starting a grab operation, check that the button is actually pressed by the client, and if not, cancel the grab operation. This prevents the stuck grab in a race-free way, although it requires an extra round-trip to the server. With client-side decorations becoming more popular, the use of _NET_WM_MOVERESIZE is on the rise, thus this bug is seen more frequently than before. https://bugzilla.gnome.org/show_bug.cgi?id=699777 --- src/core/window.c | 119 ++++++++++++++++++++++++++++++---------------- 1 file changed, 78 insertions(+), 41 deletions(-) diff --git a/src/core/window.c b/src/core/window.c index ae3c69460..60347ef65 100644 --- a/src/core/window.c +++ b/src/core/window.c @@ -6634,6 +6634,41 @@ meta_window_change_workspace_by_index (MetaWindow *window, #define _NET_WM_MOVERESIZE_MOVE_KEYBOARD 10 #define _NET_WM_MOVERESIZE_CANCEL 11 +static int +query_pressed_buttons (MetaWindow *window) +{ + double x, y, query_root_x, query_root_y; + Window root, child; + XIButtonState buttons; + XIModifierState mods; + XIGroupState group; + int button = 0; + + meta_error_trap_push (window->display); + XIQueryPointer (window->display->xdisplay, + META_VIRTUAL_CORE_POINTER_ID, + window->xwindow, + &root, &child, + &query_root_x, &query_root_y, + &x, &y, + &buttons, &mods, &group); + + if (meta_error_trap_pop_with_return (window->display) != Success) + goto out; + + if (XIMaskIsSet (buttons.mask, Button1)) + button |= 1 << 1; + if (XIMaskIsSet (buttons.mask, Button2)) + button |= 1 << 2; + if (XIMaskIsSet (buttons.mask, Button3)) + button |= 1 << 3; + + free (buttons.mask); + + out: + return button; +} + gboolean meta_window_client_message (MetaWindow *window, XEvent *event) @@ -6992,56 +7027,58 @@ meta_window_client_message (MetaWindow *window, (op != META_GRAB_OP_MOVING && op != META_GRAB_OP_KEYBOARD_MOVING)))) { - /* - * the button SHOULD already be included in the message - */ + int button_mask; + + meta_topic (META_DEBUG_WINDOW_OPS, + "Beginning move/resize with button = %d\n", button); + meta_display_begin_grab_op (window->display, + window->screen, + window, + op, + FALSE, + frame_action, + button, 0, + timestamp, + x_root, + y_root); + + button_mask = query_pressed_buttons (window); + if (button == 0) { - double x, y, query_root_x, query_root_y; - Window root, child; - XIButtonState buttons; - XIModifierState mods; - XIGroupState group; - - /* The race conditions in this _NET_WM_MOVERESIZE thing - * are mind-boggling + /* + * the button SHOULD already be included in the message */ - meta_error_trap_push (window->display); - XIQueryPointer (window->display->xdisplay, - META_VIRTUAL_CORE_POINTER_ID, - window->xwindow, - &root, &child, - &query_root_x, &query_root_y, - &x, &y, - &buttons, &mods, &group); - meta_error_trap_pop (window->display); - - if (XIMaskIsSet (buttons.mask, Button1)) + if ((button_mask & (1 << 1)) != 0) button = 1; - else if (XIMaskIsSet (buttons.mask, Button2)) + else if ((button_mask & (1 << 2)) != 0) button = 2; - else if (XIMaskIsSet (buttons.mask, Button3)) + else if ((button_mask & (1 << 3)) != 0) button = 3; + + if (button != 0) + window->display->grab_button = button; else - button = 0; - - free (buttons.mask); + meta_display_end_grab_op (window->display, + timestamp); } - - if (button != 0) + else { - meta_topic (META_DEBUG_WINDOW_OPS, - "Beginning move/resize with button = %d\n", button); - meta_display_begin_grab_op (window->display, - window->screen, - window, - op, - FALSE, - frame_action, - button, 0, - timestamp, - x_root, - y_root); + /* There is a potential race here. If the user presses and + * releases their mouse button very fast, it's possible for + * both the ButtonPress and ButtonRelease to be sent to the + * client before it can get a chance to send _NET_WM_MOVERESIZE + * to us. When that happens, we'll become stuck in a grab + * state, as we haven't received a ButtonRelease to cancel the + * grab. + * + * We can solve this by querying after we take the explicit + * pointer grab -- if the button isn't pressed, we cancel the + * drag immediately. + */ + + if ((button_mask & (1 << button)) == 0) + meta_display_end_grab_op (window->display, timestamp); } }