Attachment 24684 Details for Bug 20704 – full fix

[patch] full fix

0001-glx-Fix-drawable-private-leak-on-destroy.patch (text/plain), 10.72 KB, created by Kristian Høgsberg on 2009-04-09 10:38:14 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kristian Høgsberg

Created: 2009-04-09 10:38:14 UTC

Size: 10.72 KB

patch

obsolete

>From 2f2538d20291ead2187087228a391dade1a0434f Mon Sep 17 00:00:00 2001
>From: =?utf-8?q?Kristian=20H=C3=B8gsberg?= <krh@redhat.com>
>Date: Thu, 9 Apr 2009 13:16:37 -0400
>Subject: [PATCH] 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.
>---
> glx/Makefile.am    |    1 -
> glx/glxcmds.c      |   44 ++++++++++++++++++++----------
> glx/glxdrawable.h  |    5 ---
> glx/glxdri.c       |    2 +
> glx/glxdri2.c      |    2 +
> glx/glxdriswrast.c |    2 +
> glx/glxext.c       |   42 +++++++++++++++++++++--------
> glx/glxext.h       |    1 +
> glx/glxutil.c      |   74 ----------------------------------------------------
> glx/glxutil.h      |    9 +-----
> 10 files changed, 67 insertions(+), 115 deletions(-)
> delete mode 100644 glx/glxutil.c
>
>diff --git a/glx/Makefile.am b/glx/Makefile.am
>index 2537db8..a23ae0a 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 e21f0f0..86e8dd8 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 b64ff35..0215b3b 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 cc6d939..eeac7fc 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 c896536..44c2337 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 de89d38..328aa7e 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 025e619..93391e9 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 72092f3..7008c47 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 52a10e4..0000000
>--- 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 baa4905..d1a715b 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);
>-- 
>1.6.2.2
>

Actions: View | Splinter Review

Attachments on bug 20704: 23950 | 23951 | 24216 | 24296 | 24328 | 24684 | 26326 | 26327 | 26328

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.