From 1d2ac51ea34cae9e94e6d5f831db13f0ac3613d8 Mon Sep 17 00:00:00 2001 From: Robert Bragg Date: Fri, 17 Apr 2009 19:05:21 +0100 Subject: [PATCH] [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. --- clutter/x11/clutter-x11-texture-pixmap.c | 50 ++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/clutter/x11/clutter-x11-texture-pixmap.c b/clutter/x11/clutter-x11-texture-pixmap.c index e2673b7..52c7eea 100644 --- 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) -- 2.7.4