[x11-texture-pixmap] Fixes a reported lockup due to an undesireable X server grab

A lockup was reported by fargoilas on #clutter and removing the server grab in
clutter_x11_texture_pixmap_sync_window fixed the problem.

We were doing an x server grab to guarantee that if the XGetWindowAttributes
request reported that our redirected window was viewable then we would also
be able to get a valid pixmap name.  (the composite protocol says it's an
error to request a name for a window if it's not viewable) Without the grab
there would be a race condition.

Instead we now handle error conditions gracefully.

diff --git a/clutter/x11/clutter-x11-texture-pixmap.c b/clutter/x11/clutter-x11-texture-pixmap.c
index e2673b7..52c7eea 100644 (file)
--- a/clutter/x11/clutter-x11-texture-pixmap.c
+++ b/clutter/x11/clutter-x11-texture-pixmap.c
@@ -1196,32 +1196,40 @@ clutter_x11_texture_pixmap_sync_window (ClutterX11TexturePixmap *texture)
     {
       XWindowAttributes attr;
       Display *dpy = clutter_x11_get_default_display ();
-      gboolean mapped, notify_x, notify_y, notify_override_redirect;
-
-      /*
-       * Make sure the window is mapped when getting the pixmap -- it's what
-       * compiz does.
+      gboolean mapped = FALSE;
+      gboolean notify_x = FALSE;
+      gboolean notify_y = FALSE;
+      gboolean notify_override_redirect = FALSE;
+      Status status;
+
+      /* NB: It's only valid to name a pixmap if the window is viewable.
+       *
+       * We don't explicitly check this though since there would be a race
+       * between checking and naming unless we use a server grab which is
+       * undesireable.
+       *
+       * Instead we gracefully handle any error with naming the pixmap.
        */
-
       clutter_x11_trap_x_errors ();
-      XGrabServer (dpy);
 
-      XGetWindowAttributes (dpy, priv->window, &attr);
-      mapped = attr.map_state == IsViewable;
-      if (mapped)
-        pixmap = XCompositeNameWindowPixmap (dpy, priv->window);
-      else
-        pixmap = None;
+      pixmap = XCompositeNameWindowPixmap (dpy, priv->window);
 
-      XUngrabServer (dpy);
-      clutter_x11_untrap_x_errors ();
+      status = XGetWindowAttributes (dpy, priv->window, &attr);
+      if (status != 0)
+       {
+         notify_x = attr.x != priv->window_x;
+         notify_y = attr.y != priv->window_y;
+         notify_override_redirect = attr.override_redirect != priv->override_redirect;
+         priv->window_x = attr.x;
+         priv->window_y = attr.y;
+         priv->override_redirect = attr.override_redirect;
+       }
 
-      notify_x = attr.x != priv->window_x;
-      notify_y = attr.y != priv->window_y;
-      notify_override_redirect = attr.override_redirect != priv->override_redirect;
-      priv->window_x = attr.x;
-      priv->window_y = attr.y;
-      priv->override_redirect = attr.override_redirect;
+      /* We rely on XGetWindowAttributes() implicitly syncing with the server
+       * so if there was an error naming the pixmap that will have been
+       * caught by now. */
+      if (clutter_x11_untrap_x_errors ())
+       pixmap = None;
 
       g_object_ref (texture); /* guard against unparent */
       if (pixmap)