[PATCH] glx: Fix lifetime tracking for pixmaps

Adam Jackson ajax at redhat.com
Mon Mar 28 09:40:32 PDT 2011


GLX pixmaps take a reference on the underlying pixmap; X and GLX pixmap
IDs can be destroyed in either order with no error.  Only windows need
to be tracked under both XIDs.

Fixes piglit/glx-pixmap-life.

Signed-off-by: Adam Jackson <ajax at redhat.com>
---
 glx/glxcmds.c |   11 +++++++----
 glx/glxext.c  |   22 +++++++++++++---------
 2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/glx/glxcmds.c b/glx/glxcmds.c
index 9b4bc9e..9d60bdb 100644
--- a/glx/glxcmds.c
+++ b/glx/glxcmds.c
@@ -1177,10 +1177,11 @@ DoCreateGLXDrawable(ClientPtr client, __GLXscreen *pGlxScreen,
 	return BadAlloc;
     }
 
-    /* Add the glx drawable under the XID of the underlying X drawable
-     * too.  That way we'll get a callback in DrawableGone and can
-     * clean up properly when the drawable is destroyed. */
-    if (drawableId != glxDrawableId &&
+    /*
+     * Windows aren't refcounted, so track both the X and the GLX window
+     * so we get called regardless of destruction order.
+     */
+    if (drawableId != glxDrawableId && type == GLX_DRAWABLE_WINDOW &&
 	!AddResource(pDraw->id, __glXDrawableRes, pGlxDraw)) {
 	pGlxDraw->destroy (pGlxDraw);
 	return BadAlloc;
@@ -1211,6 +1212,8 @@ DoCreateGLXPixmap(ClientPtr client, __GLXscreen *pGlxScreen, __GLXconfig *config
     err = DoCreateGLXDrawable(client, pGlxScreen, config, pDraw, drawableId,
 			      glxDrawableId, GLX_DRAWABLE_PIXMAP);
 
+    ((PixmapPtr)pDraw)->refcnt++;
+
     return err;
 }
 
diff --git a/glx/glxext.c b/glx/glxext.c
index 4bd5d6b..cec3ee5 100644
--- a/glx/glxext.c
+++ b/glx/glxext.c
@@ -124,15 +124,15 @@ static Bool DrawableGone(__GLXdrawable *glxPriv, XID xid)
 {
     __GLXcontext *c, *next;
 
-    /* If this drawable was created using glx 1.3 drawable
-     * constructors, we added it as a glx drawable resource under both
-     * its glx drawable ID and it X drawable ID.  Remove the other
-     * resource now so we don't a callback for freed memory. */
-    if (glxPriv->drawId != glxPriv->pDraw->id) {
-	if (xid == glxPriv->drawId)
-	    FreeResourceByType(glxPriv->pDraw->id, __glXDrawableRes, TRUE);
-	else
-	    FreeResourceByType(glxPriv->drawId, __glXDrawableRes, TRUE);
+    if (glxPriv->type == GLX_DRAWABLE_WINDOW) {
+        /* If this was created by glXCreateWindow, free the matching resource */
+        if (glxPriv->drawId != glxPriv->pDraw->id) {
+            if (xid == glxPriv->drawId)
+                FreeResourceByType(glxPriv->pDraw->id, __glXDrawableRes, TRUE);
+            else
+                FreeResourceByType(glxPriv->drawId, __glXDrawableRes, TRUE);
+        }
+        /* otherwise this window was implicitly created by MakeCurrent */
     }
 
     for (c = glxAllContexts; c; c = next) {
@@ -168,6 +168,10 @@ static Bool DrawableGone(__GLXdrawable *glxPriv, XID xid)
 	    __glXFreeContext(c);
     }
 
+    /* drop our reference to any backing pixmap */
+    if (glxPriv->type == GLX_DRAWABLE_PIXMAP)
+        glxPriv->pDraw->pScreen->DestroyPixmap((PixmapPtr)glxPriv->pDraw);
+
     glxPriv->destroy(glxPriv);
 
     return True;
-- 
1.7.4.1



More information about the xorg-devel mailing list