glx: Reimplement context tags

This would let you do a constant-time context lookup, but if that's your
performance problem you have two problems.  Just use the context's XID
as the tag value instead.

In order to do this, we have to defer destroying a context until it
actually goes unreferenced, as you're allowed to mention a context tag
after you've (ostensibly) destroyed the context, as long as it's still
your current context.  Thus, change DestroyContext to merely mark the
context as dead if it's a current context, and call down to actual
resource destruction (and XID reclamation) in StopUsingContext.

Also, stop trying to delete context state from DrawableGone.  This was
always broken, as GLX does not say that contexts are destroyed when
their drawables are destroyed.  But with the above change to defer
context destruction, this would trigger a server crash on client exit as
we'd free the context state twice.

Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Adam Jackson <ajax@redhat.com>

3 files changed, 24 insertions, 112 deletions

diff --git a/glx/glxcmds.c b/glx/glxcmds.c
index 9b4bc9ec3..6585080a9 100644
--- a/glx/glxcmds.c
+++ b/glx/glxcmds.c
@@ -136,9 +136,9 @@ validGlxContext(ClientPtr client, XID id, int access_mode,
 {
     *err = dixLookupResourceByType((pointer *) context, id,
 				   __glXContextRes, client, access_mode);
-    if (*err != Success) {
+    if (*err != Success || (*context)->idExists == GL_FALSE) {
 	client->errorValue = id;
-	if (*err == BadValue)
+	if (*err == BadValue || *err == Success)
 	    *err = __glXError(GLXBadContext);
 	return FALSE;
     }
@@ -369,6 +369,7 @@ int __glXDisp_CreateContextWithConfigSGIX(__GLXclientState *cl, GLbyte *pc)
     return DoCreateContext(cl, req->context, req->shareList,
 			   config, pGlxScreen, req->isDirect);
 }
+
 int __glXDisp_DestroyContext(__GLXclientState *cl, GLbyte *pc)
 {
     ClientPtr client = cl->client;
@@ -382,77 +383,31 @@ int __glXDisp_DestroyContext(__GLXclientState *cl, GLbyte *pc)
 			 &glxc, &err))
 	    return err;
 
-    FreeResourceByType(req->context, __glXContextRes, FALSE);
-    return Success;
-}
-
-/*****************************************************************************/
-
-/*
-** For each client, the server keeps a table of all the contexts that are
-** current for that client (each thread of a client may have its own current
-** context).  These routines add, change, and lookup contexts in the table.
-*/
-
-/*
-** Add a current context, and return the tag that will be used to refer to it.
-*/
-static int AddCurrentContext(__GLXclientState *cl, __GLXcontext *glxc)
-{
-    int i;
-    int num = cl->numCurrentContexts;
-    __GLXcontext **table = cl->currentContexts;
-
-    if (!glxc) return -1;
-    
-    /*
-    ** Try to find an empty slot and use it.
-    */
-    for (i=0; i < num; i++) {
-	if (!table[i]) {
-	    table[i] = glxc;
-	    return i+1;
-	}
-    }
-    /*
-    ** Didn't find a free slot, so we'll have to grow the table.
-    */
-    if (!num) {
-	table = (__GLXcontext **) malloc(sizeof(__GLXcontext *));
-    } else {
-	table = (__GLXcontext **) realloc(table,
-					   (num+1)*sizeof(__GLXcontext *));
-    }
-    table[num] = glxc;
-    cl->currentContexts = table;
-    cl->numCurrentContexts++;
-    return num+1;
-}
+    glxc->idExists = GL_FALSE;
+    if (!glxc->isCurrent)
+        FreeResourceByType(req->context, __glXContextRes, FALSE);
 
-/*
-** Given a tag, change the current context for the corresponding entry.
-*/
-static void ChangeCurrentContext(__GLXclientState *cl, __GLXcontext *glxc,
-				GLXContextTag tag)
-{
-    __GLXcontext **table = cl->currentContexts;
-    table[tag-1] = glxc;
+    return Success;
 }
 
 /*
-** For this implementation we have chosen to simply use the index of the
-** context's entry in the table as the context tag.  A tag must be greater
-** than 0.
-*/
+ * This will return "deleted" contexts, ie, where idExists is GL_FALSE.
+ * Contrast validGlxContext, which will not.  We're cheating here and
+ * using the XID as the context tag, which is fine as long as we defer
+ * actually destroying the context until it's no longer referenced, and
+ * block clients from trying to MakeCurrent on contexts that are on the
+ * way to destruction.  Notice that DoMakeCurrent calls validGlxContext
+ * for new contexts but __glXLookupContextByTag for previous contexts.
+ */
 __GLXcontext *__glXLookupContextByTag(__GLXclientState *cl, GLXContextTag tag)
 {
-    int num = cl->numCurrentContexts;
+    __GLXcontext *ret;
 
-    if (tag < 1 || tag > num) {
-	return 0;
-    } else {
-	return cl->currentContexts[tag-1];
-    }
+    if (dixLookupResourceByType((void **)&ret, tag, __glXContextRes,
+                                cl->client, DixUseAccess) == Success)
+        return ret;
+
+    return NULL;
 }
 
 /*****************************************************************************/
@@ -466,7 +421,7 @@ static void StopUsingContext(__GLXcontext *glxc)
 	}
 	glxc->isCurrent = GL_FALSE;
 	if (!glxc->idExists) {
-	    __glXFreeContext(glxc);
+            FreeResourceByType(glxc->id, __glXContextRes, FALSE);
 	}
     }
 }
@@ -669,16 +624,11 @@ DoMakeCurrent(__GLXclientState *cl,
 	glxc->isCurrent = GL_TRUE;
     }
 
-    if (prevglxc) {
-	ChangeCurrentContext(cl, glxc, tag);
-	StopUsingContext(prevglxc);
-    } else {
-	tag = AddCurrentContext(cl, glxc);
-    }
+    StopUsingContext(prevglxc);
 
     if (glxc) {
 	StartUsingContext(cl, glxc);
-	reply.contextTag = tag;
+	reply.contextTag = glxc->id;
     } else {
 	reply.contextTag = 0;
     }
diff --git a/glx/glxext.c b/glx/glxext.c
index 4bd5d6b6b..16315b876 100644
--- a/glx/glxext.c
+++ b/glx/glxext.c
@@ -138,34 +138,15 @@ static Bool DrawableGone(__GLXdrawable *glxPriv, XID xid)
     for (c = glxAllContexts; c; c = next) {
 	next = c->next;
 	if (c->isCurrent && (c->drawPriv == glxPriv || c->readPriv == glxPriv)) {
-	    int i;
-
 	    (*c->loseCurrent)(c);
 	    c->isCurrent = GL_FALSE;
 	    if (c == __glXLastContext)
 		__glXFlushContextCache();
-
-	    for (i = 1; i < currentMaxClients; i++) {
-		if (clients[i]) {
-		    __GLXclientState *cl = glxGetClient(clients[i]);
-
-		    if (cl->inUse) {
-			int j;
-
-			for (j = 0; j < cl->numCurrentContexts; j++) {
-			    if (cl->currentContexts[j] == c)
-				cl->currentContexts[j] = NULL;
-			}
-		    }
-		}
-	    }
 	}
 	if (c->drawPriv == glxPriv)
 	    c->drawPriv = NULL;
 	if (c->readPriv == glxPriv)
 	    c->readPriv = NULL;
-	if (!c->idExists && !c->isCurrent)
-	    __glXFreeContext(c);
     }
 
     glxPriv->destroy(glxPriv);
@@ -283,8 +264,6 @@ glxClientCallback (CallbackListPtr	*list,
     NewClientInfoRec	*clientinfo = (NewClientInfoRec *) data;
     ClientPtr		pClient = clientinfo->client;
     __GLXclientState	*cl = glxGetClient(pClient);
-    __GLXcontext	*cx;
-    int i;
 
     switch (pClient->clientState) {
     case ClientStateRunning:
@@ -298,18 +277,8 @@ glxClientCallback (CallbackListPtr	*list,
 	break;
 
     case ClientStateGone:
-	for (i = 0; i < cl->numCurrentContexts; i++) {
-	    cx = cl->currentContexts[i];
-	    if (cx) {
-		cx->isCurrent = GL_FALSE;
-		if (!cx->idExists)
-		    __glXFreeContext(cx);
-	    }
-	}
-
 	free(cl->returnBuf);
 	free(cl->largeCmdBuf);
-	free(cl->currentContexts);
 	free(cl->GLClientextensions);
 	break;
 
diff --git a/glx/glxserver.h b/glx/glxserver.h
index 1daf97758..f10c8fe12 100644
--- a/glx/glxserver.h
+++ b/glx/glxserver.h
@@ -158,13 +158,6 @@ struct __GLXclientStateRec {
     GLbyte *largeCmdBuf;
     GLint largeCmdBufSize;
 
-    /*
-    ** Keep a list of all the contexts that are current for this client's
-    ** threads.
-    */
-    __GLXcontext **currentContexts;
-    GLint numCurrentContexts;
-
     /* Back pointer to X client record */
     ClientPtr client;