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>

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);
+    glxc->idExists = GL_FALSE;
+    if (!glxc->isCurrent)
+        FreeResourceByType(req->context, __glXContextRes, FALSE);
+
     return Success;
 }
 
-/*****************************************************************************/
-
 /*
-** For each client, the server keeps a table of all the contexts that are
+ * This will return "deleted" contexts, ie, where idExists is GL_FALSE.
-** current for that client (each thread of a client may have its own current
+ * Contrast validGlxContext, which will not.  We're cheating here and
-** context).  These routines add, change, and lookup contexts in the table.
+ * 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
-** Add a current context, and return the tag that will be used to refer to it.
+ * for new contexts but __glXLookupContextByTag for previous contexts.
-*/
+ */
-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;
-}
-
-/*
-** 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;
-}
-
-/*
-** 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.
-*/
 __GLXcontext *__glXLookupContextByTag(__GLXclientState *cl, GLXContextTag tag)
 {
-    int num = cl->numCurrentContexts;
+    __GLXcontext *ret;
 
-    if (tag < 1 || tag > num) {
+    if (dixLookupResourceByType((void **)&ret, tag, __glXContextRes,
-	return 0;
+                                cl->client, DixUseAccess) == Success)
-    } else {
+        return ret;
-	return cl->currentContexts[tag-1];
+
-    }
+    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) {
+    StopUsingContext(prevglxc);
-	ChangeCurrentContext(cl, glxc, tag);
-	StopUsingContext(prevglxc);
-    } else {
-	tag = AddCurrentContext(cl, glxc);
-    }
 
     if (glxc) {
 	StartUsingContext(cl, glxc);
-	reply.contextTag = tag;
+	reply.contextTag = glxc->id;
     } else {
 	reply.contextTag = 0;
     }

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;
 

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;