From aa9ee78d2ea9d5f0fa0c6abc8e144904be207ba5 Mon Sep 17 00:00:00 2001 From: Adam Jackson Date: Sat, 3 Aug 2013 09:47:55 -0400 Subject: glx: Fix memory leak in context garbage collection I broke this, back in: commit a48dadc98a28c969741979b70b7a639f24f4cbbd Author: Adam Jackson 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 --- 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 13d21ccb9..78792dab6 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 35899d610..f97ef25b0 100644 --- a/glx/glxcmds.c +++ b/glx/glxcmds.c @@ -292,7 +292,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; @@ -400,7 +400,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; @@ -433,7 +433,7 @@ static void StopUsingContext(__GLXcontext * glxc) { if (glxc) { - glxc->isCurrent = GL_FALSE; + glxc->currentClient = NULL; if (!glxc->idExists) { FreeResourceByType(glxc->id, __glXContextRes, FALSE); } @@ -443,7 +443,7 @@ StopUsingContext(__GLXcontext * glxc) static void StartUsingContext(__GLXclientState * cl, __GLXcontext * glxc) { - glxc->isCurrent = GL_TRUE; + glxc->currentClient = cl->client; } /** @@ -577,7 +577,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; } @@ -639,7 +639,7 @@ DoMakeCurrent(__GLXclientState * cl, return __glXError(GLXBadContext); } - glxc->isCurrent = GL_TRUE; + glxc->currentClient = client; } StopUsingContext(prevglxc); @@ -860,7 +860,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 3b7f60d95..8cf333110 100644 --- a/glx/glxcontext.h +++ b/glx/glxcontext.h @@ -68,6 +68,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. */ @@ -83,11 +88,6 @@ struct __GLXcontext { */ GLboolean idExists; - /* - ** Whether this context is current for some client. - */ - GLboolean isCurrent; - /* ** Whether this context is a direct rendering context. */ diff --git a/glx/glxext.c b/glx/glxext.c index 81ed17a06..9fa7ef428 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); } @@ -129,9 +128,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; @@ -185,14 +185,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); - __glXleaveServer(GL_FALSE); cx->destroy(cx); __glXenterServer(GL_FALSE); @@ -259,6 +259,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: @@ -266,6 +267,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); -- cgit v1.2.3