[mutter/gnome-3-8] window: Eliminate a potential race condition with _NET_WM_MOVERESIZE



commit ca4e1fd4c9e0c2c466a5602c349fe6abb3d01c10
Author: Jasper St. Pierre <jstpierre mecheye net>
Date:   Mon May 6 15:45:24 2013 -0400

    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 files changed, 78 insertions(+), 41 deletions(-)
---
diff --git a/src/core/window.c b/src/core/window.c
index ae3c694..60347ef 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;
-              else
-                button = 0;
 
-              free (buttons.mask);
+              if (button != 0)
+                window->display->grab_button = button;
+              else
+                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);
             }
         }
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]