[RFC] glx: fix DRI2 memory leak
Kristian Høgsberg
krh at bitplanet.net
Thu Apr 9 10:57:57 PDT 2009
On Wed, Mar 25, 2009 at 10:21:51AM -0700, Jesse Barnes wrote:
> In trying to track down the memory leak in 20704, I found that the DRI2
> drawable destroy routine doesn't seem to get called when new drawables
> are created and old ones destroyed (as in the glViewport case in the
> bug).
Ok, sorry for taking so long on this. Here's the patch I'd like to push
to fix the problem - let me know if it works alright.
thanks,
Kristian
>From 2f2538d20291ead2187087228a391dade1a0434f Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Kristian=20H=C3=B8gsberg?= <krh at 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
More information about the xorg
mailing list