[PATCH 2/3] glx: Fix memory leak in context garbage collection
Adam Jackson
ajax at redhat.com
Mon Sep 30 09:11:23 PDT 2013
I broke this, back in:
commit a48dadc98a28c969741979b70b7a639f24f4cbbd
Author: Adam Jackson <ajax at redhat.com>
Date: Mon Mar 21 11:59:29 2011 -0400
glx: Reimplement context tags
In that, I changed the glx client state to not explicitly track the list
of current contexts for the client (since that was what we were deriving
tags from). The bug was that I removed the code for same from
glxClientCallback without noticing that it had the side effect of
effectively de-currenting those contexts, so that ContextGone could free
them. So, if you had a client exit with a context still current, the
context's memory would leak. Not a huge deal for direct clients, but
viciously bad for indirect, since the swrast context state at the bottom
of Mesa is like 15M.
Fix this by promoting Bool isCurrent to ClientPtr currentClient, so that
we have a back-pointer to chase when walking the list of contexts when
ClientStateGone happens.
Signed-off-by: Adam Jackson <ajax at redhat.com>
---
glx/createcontext.c | 2 +-
glx/glxcmds.c | 14 +++++++-------
glx/glxcontext.h | 10 +++++-----
glx/glxext.c | 36 +++++++++++++++++++++++++-----------
4 files changed, 38 insertions(+), 24 deletions(-)
diff --git a/glx/createcontext.c b/glx/createcontext.c
index 13d21cc..78792da 100644
--- a/glx/createcontext.c
+++ b/glx/createcontext.c
@@ -320,7 +320,7 @@ __glXDisp_CreateContextAttribsARB(__GLXclientState * cl, GLbyte * pc)
ctx->id = req->context;
ctx->share_id = req->shareList;
ctx->idExists = True;
- ctx->isCurrent = False;
+ ctx->currentClient = False;
ctx->isDirect = req->isDirect;
ctx->hasUnflushedCommands = False;
ctx->renderMode = GL_RENDER;
diff --git a/glx/glxcmds.c b/glx/glxcmds.c
index 663448a..288b417 100644
--- a/glx/glxcmds.c
+++ b/glx/glxcmds.c
@@ -290,7 +290,7 @@ DoCreateContext(__GLXclientState * cl, GLXContextID gcId,
glxc->id = gcId;
glxc->share_id = shareList;
glxc->idExists = GL_TRUE;
- glxc->isCurrent = GL_FALSE;
+ glxc->currentClient = NULL;
glxc->isDirect = isDirect;
glxc->hasUnflushedCommands = GL_FALSE;
glxc->renderMode = GL_RENDER;
@@ -398,7 +398,7 @@ __glXDisp_DestroyContext(__GLXclientState * cl, GLbyte * pc)
return err;
glxc->idExists = GL_FALSE;
- if (!glxc->isCurrent)
+ if (!glxc->currentClient)
FreeResourceByType(req->context, __glXContextRes, FALSE);
return Success;
@@ -431,7 +431,7 @@ static void
StopUsingContext(__GLXcontext * glxc)
{
if (glxc) {
- glxc->isCurrent = GL_FALSE;
+ glxc->currentClient = NULL;
if (!glxc->idExists) {
FreeResourceByType(glxc->id, __glXContextRes, FALSE);
}
@@ -441,7 +441,7 @@ StopUsingContext(__GLXcontext * glxc)
static void
StartUsingContext(__GLXclientState * cl, __GLXcontext * glxc)
{
- glxc->isCurrent = GL_TRUE;
+ glxc->currentClient = cl->client;
}
/**
@@ -571,7 +571,7 @@ DoMakeCurrent(__GLXclientState * cl,
if (!validGlxContext(client, contextId, DixUseAccess, &glxc, &error))
return error;
- if ((glxc != prevglxc) && glxc->isCurrent) {
+ if ((glxc != prevglxc) && glxc->currentClient) {
/* Context is current to somebody else */
return BadAccess;
}
@@ -633,7 +633,7 @@ DoMakeCurrent(__GLXclientState * cl,
return __glXError(GLXBadContext);
}
- glxc->isCurrent = GL_TRUE;
+ glxc->currentClient = client;
}
StopUsingContext(prevglxc);
@@ -854,7 +854,7 @@ __glXDisp_CopyContext(__GLXclientState * cl, GLbyte * pc)
/*
** The destination context must not be current for any client.
*/
- if (dst->isCurrent) {
+ if (dst->currentClient) {
client->errorValue = dest;
return BadAccess;
}
diff --git a/glx/glxcontext.h b/glx/glxcontext.h
index 96a4dd2..fd018d0 100644
--- a/glx/glxcontext.h
+++ b/glx/glxcontext.h
@@ -70,6 +70,11 @@ struct __GLXcontext {
__GLXscreen *pGlxScreen;
/*
+ ** If this context is current for a client, this will be that client
+ */
+ ClientPtr currentClient;
+
+ /*
** The XID of this context.
*/
XID id;
@@ -85,11 +90,6 @@ struct __GLXcontext {
GLboolean idExists;
/*
- ** Whether this context is current for some client.
- */
- GLboolean isCurrent;
-
- /*
** Whether this context is a direct rendering context.
*/
GLboolean isDirect;
diff --git a/glx/glxext.c b/glx/glxext.c
index 1bb884f..534c791 100644
--- a/glx/glxext.c
+++ b/glx/glxext.c
@@ -88,16 +88,15 @@ __glXResetLargeCommandStatus(__GLXclientState * cl)
}
/*
-** This procedure is called when the client who created the context goes
-** away OR when glXDestroyContext is called. In either case, all we do is
-** flag that the ID is no longer valid, and (maybe) free the context.
-** use.
-*/
+ * This procedure is called when the client who created the context goes away
+ * OR when glXDestroyContext is called. In either case, all we do is flag that
+ * the ID is no longer valid, and (maybe) free the context.
+ */
static int
ContextGone(__GLXcontext * cx, XID id)
{
cx->idExists = GL_FALSE;
- if (!cx->isCurrent) {
+ if (!cx->currentClient) {
__glXFreeContext(cx);
}
@@ -131,9 +130,10 @@ DrawableGone(__GLXdrawable * glxPriv, XID xid)
for (c = glxAllContexts; c; c = next) {
next = c->next;
- if (c->isCurrent && (c->drawPriv == glxPriv || c->readPriv == glxPriv)) {
+ if (c->currentClient &&
+ (c->drawPriv == glxPriv || c->readPriv == glxPriv)) {
(*c->loseCurrent) (c);
- c->isCurrent = GL_FALSE;
+ c->currentClient = NULL;
}
if (c->drawPriv == glxPriv)
c->drawPriv = NULL;
@@ -187,14 +187,14 @@ __glXRemoveFromContextList(__GLXcontext * cx)
GLboolean
__glXFreeContext(__GLXcontext * cx)
{
- if (cx->idExists || cx->isCurrent)
+ if (cx->idExists || cx->currentClient)
return GL_FALSE;
+ __glXRemoveFromContextList(cx);
+
free(cx->feedbackBuf);
free(cx->selectBuf);
- __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. */
@@ -271,6 +271,7 @@ glxClientCallback(CallbackListPtr *list, pointer closure, pointer data)
NewClientInfoRec *clientinfo = (NewClientInfoRec *) data;
ClientPtr pClient = clientinfo->client;
__GLXclientState *cl = glxGetClient(pClient);
+ __GLXcontext *c, *next;
switch (pClient->clientState) {
case ClientStateRunning:
@@ -282,6 +283,19 @@ glxClientCallback(CallbackListPtr *list, pointer closure, pointer data)
break;
case ClientStateGone:
+ /*
+ * reap any remaining contexts. we're running just before
+ * FreeClientResources, so all we need to do is detach the client
+ * from the context, and ContextGone will do the rest.
+ */
+ for (c = glxAllContexts; c; c = next) {
+ next = c->next;
+ if (c->currentClient == pClient) {
+ (*c->loseCurrent) (c);
+ c->currentClient = NULL;
+ }
+ }
+
free(cl->returnBuf);
free(cl->largeCmdBuf);
free(cl->GLClientextensions);
--
1.8.3.1
More information about the xorg-devel
mailing list