glx: Fix memory leak in context garbage collection (v2)

I broke this, back in:

    commit a48dadc98a28c969741979b70b7a639f24f4cbbd
    Author: Adam Jackson <ajax@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.

v2: Explicitly call __glXFreeContext on the ClientStateGone path.  Our
current context might be one we got from EXT_import_context and whose
creating client has since died.  Without the explicit call, the creating
client's FreeClientResources would not free the context because it's
still current, and the using client's FreeClientResources would not free
the context because it's not an XID it created.  This matches the logic
from a48dadc.

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Signed-off-by: Adam Jackson <ajax@redhat.com>

1 files changed, 22 insertions, 11 deletions

diff --git a/glx/glxext.c b/glx/glxext.c
index 2c3f5edf0..dcefecdac 100644
--- a/glx/glxext.c
+++ b/glx/glxext.c
@@ -95,16 +95,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);
     }
 
@@ -138,9 +137,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 == __glXLastContext)
                 __glXFlushContextCache();
         }
@@ -196,17 +196,17 @@ __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);
     if (cx == __glXLastContext) {
         __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. */
@@ -283,6 +283,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:
@@ -290,6 +291,16 @@ glxClientCallback(CallbackListPtr *list, pointer closure, pointer data)
         break;
 
     case ClientStateGone:
+        /* detach from all current contexts */
+        for (c = glxAllContexts; c; c = next) {
+            next = c->next;
+            if (c->currentClient == pClient) {
+                c->loseCurrent(c);
+                c->currentClient = NULL;
+                __glXFreeContext(c);
+            }
+        }
+
         free(cl->returnBuf);
         free(cl->largeCmdBuf);
         free(cl->GLClientextensions);