glx: Fix drawable private leak on destroy

When a drawable goes away, we don't destroy the GLX drawable in full,
since it may be current for a context.  This means that when the drawable
is destroyed in full later, the backend doesn't get a chance to
destroy resources associated with the drawable (the DRI2Drawable).

With this patch, we destroy the GLX drawable in full when it goes away
and then track down all contexts that reference it and NULL their
pointers.

10 files changed, 67 insertions, 115 deletions

diff --git a/glx/Makefile.am b/glx/Makefile.am
index 2537db865..a23ae0a47 100644
--- a/glx/Makefile.am
+++ b/glx/Makefile.am
@@ -80,7 +80,6 @@ libglx_la_SOURCES = \
         glxscreens.c \
         glxscreens.h \
         glxserver.h \
-        glxutil.c \
         glxutil.h \
         render2.c \
         render2swap.c \
diff --git a/glx/glxcmds.c b/glx/glxcmds.c
index e21f0f098..86e8dd8a5 100644
--- a/glx/glxcmds.c
+++ b/glx/glxcmds.c
@@ -193,18 +193,9 @@ validGlxDrawable(ClientPtr client, XID id, int type, int access_mode,
 void
 __glXContextDestroy(__GLXcontext *context)
 {
-    if (!context->isDirect) {
-	if (context->drawPriv)
-	    __glXUnrefDrawable(context->drawPriv);
-	if (context->readPriv)
-	    __glXUnrefDrawable(context->readPriv);
-	context->drawPriv = NULL;
-	context->readPriv = NULL;
-    }
     __glXFlushContextCache();
 }
 
-
 static void __glXdirectContextDestroy(__GLXcontext *context)
 {
     __glXContextDestroy(context);
@@ -320,6 +311,8 @@ DoCreateContext(__GLXclientState *cl, GLXContextID gcId,
     glxc->isDirect = isDirect;
     glxc->renderMode = GL_RENDER;
 
+    __glXAddToContextList(glxc);
+
     return Success;
 }
 
@@ -639,10 +632,6 @@ DoMakeCurrent(__GLXclientState *cl,
 	}
 	__glXFlushContextCache();
 	if (!prevglxc->isDirect) {
-	    if (prevglxc->drawPriv)
-		__glXUnrefDrawable(prevglxc->drawPriv);
-	    if (prevglxc->readPriv)
-		__glXUnrefDrawable(prevglxc->readPriv);
 	    prevglxc->drawPriv = NULL;
 	    prevglxc->readPriv = NULL;
 	}
@@ -662,8 +651,6 @@ DoMakeCurrent(__GLXclientState *cl,
 	}
 
 	glxc->isCurrent = GL_TRUE;
-	__glXRefDrawable(glxc->drawPriv);
-	__glXRefDrawable(glxc->readPriv);
     }
 
     if (prevglxc) {
@@ -1090,6 +1077,33 @@ int __glXDisp_GetFBConfigsSGIX(__GLXclientState *cl, GLbyte *pc)
     return DoGetFBConfigs(cl, req->screen);
 }
 
+GLboolean
+__glXDrawableInit(__GLXdrawable *drawable,
+		  __GLXscreen *screen, DrawablePtr pDraw, int type,
+		  XID drawId, __GLXconfig *config)
+{
+    drawable->pDraw = pDraw;
+    drawable->type = type;
+    drawable->drawId = drawId;
+    drawable->config = config;
+    drawable->eventMask = 0;
+
+    return GL_TRUE;
+}
+
+void
+__glXDrawableRelease(__GLXdrawable *drawable)
+{
+    ScreenPtr pScreen = drawable->pDraw->pScreen;
+
+    switch (drawable->type) {
+    case GLX_DRAWABLE_PIXMAP:
+    case GLX_DRAWABLE_PBUFFER:
+	(*pScreen->DestroyPixmap)((PixmapPtr) drawable->pDraw);
+	break;
+    }
+}
+
 static int 
 DoCreateGLXDrawable(ClientPtr client, __GLXscreen *pGlxScreen, __GLXconfig *config,
 		    DrawablePtr pDraw, XID glxDrawableId, int type)
diff --git a/glx/glxdrawable.h b/glx/glxdrawable.h
index b64ff35c9..0215b3b20 100644
--- a/glx/glxdrawable.h
+++ b/glx/glxdrawable.h
@@ -67,11 +67,6 @@ struct __GLXdrawable {
     */
     __GLXconfig *config;
 
-    /*
-    ** reference count
-    */
-    int refCount;
-
     GLenum target;
     GLenum format;
 
diff --git a/glx/glxdri.c b/glx/glxdri.c
index cc6d93976..eeac7fcb2 100644
--- a/glx/glxdri.c
+++ b/glx/glxdri.c
@@ -238,6 +238,8 @@ __glXDRIdrawableDestroy(__GLXdrawable *drawable)
 	__glXleaveServer(GL_FALSE);
     }
 
+    __glXDrawableRelease(drawable);
+
     xfree(private);
 }
 
diff --git a/glx/glxdri2.c b/glx/glxdri2.c
index ea5b5ef72..9dac8a6a3 100644
--- a/glx/glxdri2.c
+++ b/glx/glxdri2.c
@@ -109,6 +109,8 @@ __glXDRIdrawableDestroy(__GLXdrawable *drawable)
     if (drawable->pDraw != NULL)
 	DRI2DestroyDrawable(drawable->pDraw);
 
+    __glXDrawableRelease(drawable);
+
     xfree(private);
 }
 
diff --git a/glx/glxdriswrast.c b/glx/glxdriswrast.c
index de89d381e..328aa7e56 100644
--- a/glx/glxdriswrast.c
+++ b/glx/glxdriswrast.c
@@ -102,6 +102,8 @@ __glXDRIdrawableDestroy(__GLXdrawable *drawable)
     FreeScratchGC(private->gc);
     FreeScratchGC(private->swapgc);
 
+    __glXDrawableRelease(drawable);
+
     xfree(private);
 }
 
diff --git a/glx/glxext.c b/glx/glxext.c
index 025e6198f..93391e9fd 100644
--- a/glx/glxext.c
+++ b/glx/glxext.c
@@ -50,6 +50,7 @@
 ** from the server's perspective.
 */
 __GLXcontext *__glXLastContext;
+__GLXcontext *__glXContextList;
 
 /*
 ** X resources.
@@ -111,31 +112,46 @@ static int ContextGone(__GLXcontext* cx, XID id)
     return True;
 }
 
+static __GLXcontext *glxPendingDestroyContexts;
+static __GLXcontext *glxAllContexts;
+static int glxServerLeaveCount;
+static int glxBlockClients;
+
 /*
 ** Destroy routine that gets called when a drawable is freed.  A drawable
 ** contains the ancillary buffers needed for rendering.
 */
 static Bool DrawableGone(__GLXdrawable *glxPriv, XID xid)
 {
-    ScreenPtr pScreen = glxPriv->pDraw->pScreen;
+    __GLXcontext *c;
 
-    switch (glxPriv->type) {
-	case GLX_DRAWABLE_PIXMAP:
-	case GLX_DRAWABLE_PBUFFER:
-	    (*pScreen->DestroyPixmap)((PixmapPtr) glxPriv->pDraw);
-	    break;
+    for (c = glxAllContexts; c; c = c->next) {
+	if (c->drawPriv == glxPriv)
+	    c->drawPriv = NULL;
+	if (c->readPriv == glxPriv)
+	    c->readPriv = NULL;
     }
 
-    glxPriv->pDraw = NULL;
-    glxPriv->drawId = 0;
-    __glXUnrefDrawable(glxPriv);
+    glxPriv->destroy(glxPriv);
 
     return True;
 }
 
-static __GLXcontext *glxPendingDestroyContexts;
-static int glxServerLeaveCount;
-static int glxBlockClients;
+void __glXAddToContextList(__GLXcontext *cx)
+{
+    cx->next = glxAllContexts;
+    glxAllContexts = cx;
+}
+
+void __glXRemoveFromContextList(__GLXcontext *cx)
+{
+    __GLXcontext *c, **prev;
+
+    prev = &glxAllContexts;
+    for (c = glxAllContexts; c; c = c->next)
+	if (c == cx)
+	    *prev = c->next;
+}
 
 /*
 ** Free a context.
@@ -150,6 +166,8 @@ GLboolean __glXFreeContext(__GLXcontext *cx)
 	__glXFlushContextCache();
     }
 
+    __glXRemoveFromContextList(cx);
+
     /* We can get here through both regular dispatching from
      * __glXDispatch() or as a callback from the resource manager.  In
      * the latter case we need to lift the DRI lock manually. */
diff --git a/glx/glxext.h b/glx/glxext.h
index 72092f34a..7008c4763 100644
--- a/glx/glxext.h
+++ b/glx/glxext.h
@@ -38,6 +38,7 @@
 extern GLboolean __glXFreeContext(__GLXcontext *glxc);
 extern void __glXFlushContextCache(void);
 
+extern void __glXAddToContextList(__GLXcontext *cx);
 extern void __glXErrorCallBack(GLenum code);
 extern void __glXClearErrorOccured(void);
 extern GLboolean __glXErrorOccured(void);
diff --git a/glx/glxutil.c b/glx/glxutil.c
deleted file mode 100644
index 52a10e4e5..000000000
--- a/glx/glxutil.c
+++ /dev/null
@@ -1,74 +0,0 @@
-/*
- * SGI FREE SOFTWARE LICENSE B (Version 2.0, Sept. 18, 2008)
- * Copyright (C) 1991-2000 Silicon Graphics, Inc. All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice including the dates of first publication and
- * either this permission notice or a reference to
- * http://oss.sgi.com/projects/FreeB/
- * shall be included in all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
- * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * SILICON GRAPHICS, INC. BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
- * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF
- * OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
- * SOFTWARE.
- *
- * Except as contained in this notice, the name of Silicon Graphics, Inc.
- * shall not be used in advertising or otherwise to promote the sale, use or
- * other dealings in this Software without prior written authorization from
- * Silicon Graphics, Inc.
- */
-
-#define FONT_PCF
-#ifdef HAVE_DIX_CONFIG_H
-#include <dix-config.h>
-#endif
-
-#include <string.h>
-
-#include "glxserver.h"
-#include "glxutil.h"
-
-/*****************************************************************************/
-/* Drawable private stuff */
-
-void
-__glXRefDrawable(__GLXdrawable *glxPriv)
-{
-    glxPriv->refCount++;
-}
-
-void
-__glXUnrefDrawable(__GLXdrawable *glxPriv)
-{
-    glxPriv->refCount--;
-    if (glxPriv->refCount == 0) {
-	/* remove the drawable from the drawable list */
-	FreeResourceByType(glxPriv->drawId, __glXDrawableRes, FALSE);
-	glxPriv->destroy(glxPriv);
-    }
-}
-
-GLboolean
-__glXDrawableInit(__GLXdrawable *drawable,
-		  __GLXscreen *screen, DrawablePtr pDraw, int type,
-		  XID drawId, __GLXconfig *config)
-{
-    drawable->pDraw = pDraw;
-    drawable->type = type;
-    drawable->drawId = drawId;
-    drawable->refCount = 1;
-    drawable->config = config;
-    drawable->eventMask = 0;
-
-    return GL_TRUE;
-}
diff --git a/glx/glxutil.h b/glx/glxutil.h
index baa490500..d1a715b4b 100644
--- a/glx/glxutil.h
+++ b/glx/glxutil.h
@@ -35,18 +35,11 @@
  * Silicon Graphics, Inc.
  */
 
-/* relate contexts with drawables */
-extern void __glXAssociateContext(__GLXcontext *glxc);
-extern void __glXDeassociateContext(__GLXcontext *glxc);
-
-/* drawable management */
-extern void __glXRefDrawable(__GLXdrawable *glxPriv);
-extern void __glXUnrefDrawable(__GLXdrawable *glxPriv);
-
 extern GLboolean __glXDrawableInit(__GLXdrawable *drawable,
 				   __GLXscreen *screen,
 				   DrawablePtr pDraw, int type, XID drawID,
 				   __GLXconfig *config);
+extern void __glXDrawableRelease(__GLXdrawable *drawable);
 
 /* context helper routines */
 extern __GLXcontext *__glXLookupContextByTag(__GLXclientState*, GLXContextTag);