cogl-x11: Trap glXDestroyPixmap()

There are ordering issues in the pixmap destruction with current and
past X11 server, Mesa and dri2. Under some circumstances, an X pixmap
might be destroyed with the GLX pixmap still referencing it, and thus
the X server will decide to destroy the GLX pixmap as well; then, when
Cogl tries to destroy the GLX pixmap, it gets BadDrawable errors.

Clutter 1.2 used to trap + sync all calls to glXDestroyPixmap(), but
then we assumed that the ordering issue had been solved. So, we're back
to square 1.

I left a Big Fat Comment™ right above the glXDestroyPixmap() call
referencing the bug and the reasoning behind the trap, so that we don't
go and remove it in the future without checking that the issue has been
in fact solved.

http://bugzilla.clutter-project.org/show_bug.cgi?id=2324

diff --git a/clutter/cogl/cogl/winsys/cogl-texture-pixmap-x11.c b/clutter/cogl/cogl/winsys/cogl-texture-pixmap-x11.c
index 56097ea..be0c302 100644 (file)
--- a/clutter/cogl/cogl/winsys/cogl-texture-pixmap-x11.c
+++ b/clutter/cogl/cogl/winsys/cogl-texture-pixmap-x11.c
@@ -888,12 +888,34 @@ _cogl_texture_pixmap_x11_free_glx_pixmap (CoglTexturePixmapX11 *tex_pixmap)
 {
   if (tex_pixmap->glx_pixmap)
     {
+      CoglXlibTrapState trap_state;
+
       _COGL_GET_CONTEXT (ctx, NO_RETVAL);
 
       if (tex_pixmap->pixmap_bound)
         glXReleaseTexImage (_cogl_xlib_get_display (), tex_pixmap->glx_pixmap,
                             GLX_FRONT_LEFT_EXT);
+
+      /* FIXME - we need to trap errors and synchronize here because
+       * of ordering issues between the XPixmap destruction and the
+       * GLXPixmap destruction.
+       *
+       * If the X pixmap is destroyed, the GLX pixmap is destroyed as
+       * well immediately, and thus, when Cogl calls glXDestroyPixmap()
+       * it'll cause a BadDrawable error.
+       *
+       * this is technically a bug in the X server, which should not
+       * destroy either pixmaps until the call to glXDestroyPixmap(); so
+       * at some point we should revisit this code and remove the
+       * trap+sync after verifying that the destruction is indeed safe.
+       *
+       * for reference, see:
+       *   http://bugzilla.clutter-project.org/show_bug.cgi?id=2324
+       */
+      _cogl_xlib_trap_errors (&trap_state);
       glXDestroyPixmap (_cogl_xlib_get_display (), tex_pixmap->glx_pixmap);
+      XSync (_cogl_xlib_get_display (), False);
+      _cogl_xlib_untrap_errors (&trap_state);
 
       tex_pixmap->glx_pixmap = None;
       tex_pixmap->pixmap_bound = FALSE;