glx: Reflow MakeContextCurrent a little

First, move a few early-out checks up since they don't need to take the
GLX lock. Second, move garbage collecting deleted contexts up to
immediately after they are unbound. This fixes a memory leak, albeit a
difficult one to hit, in the case where you switch away from a deleted
context but switching to the new one errors out. In that case we would
leak the deleted context, since it's been unbound from all threads and
there's no longer an XID for it.

Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20549>

1 files changed, 26 insertions, 27 deletions

diff --git a/src/glx/glxcurrent.c b/src/glx/glxcurrent.c
index a0a0aa3ca9a..19b70194ac6 100644
--- a/src/glx/glxcurrent.c
+++ b/src/glx/glxcurrent.c
@@ -101,6 +101,7 @@ MakeContextCurrent(Display * dpy, GLXDrawable draw,
 {
    struct glx_context *gc = (struct glx_context *) gc_user;
    struct glx_context *oldGC = __glXGetCurrentContext();
+   int ret = GL_TRUE;
 
    /* Make sure that the new context has a nonzero ID.  In the request,
     * a zero context ID is used only to mean that we bind to no current
@@ -110,25 +111,32 @@ MakeContextCurrent(Display * dpy, GLXDrawable draw,
       return GL_FALSE;
    }
 
-   __glXLock();
-   if (oldGC == gc &&
-       gc->currentDrawable == draw && gc->currentReadable == read) {
-      __glXUnlock();
-      return True;
-   }
-
    /* can't have only one be 0 */
    if (!!draw != !!read) {
-      __glXUnlock();
       __glXSendError(dpy, BadMatch, None, opcode, True);
       return False;
    }
 
+   if (oldGC == gc &&
+       gc->currentDrawable == draw && gc->currentReadable == read)
+      return True;
+
+   __glXLock();
+
    if (oldGC != &dummyContext) {
       oldGC->vtable->unbind(oldGC);
       oldGC->currentDpy = NULL;
+
+      if (oldGC->xid == None) {
+         /* We are switching away from a context that was
+          * previously destroyed, so we need to free the memory
+          * for the old handle. */
+         oldGC->vtable->destroy(oldGC);
+      }
    }
 
+   __glXSetCurrentContextNull();
+
    if (gc) {
       /* Attempt to bind the context.  We do this before mucking with
        * gc and __glXSetCurrentContext to properly handle our state in
@@ -139,30 +147,21 @@ MakeContextCurrent(Display * dpy, GLXDrawable draw,
        * figuring out how to handle setting a valid context.
        */
       if (gc->vtable->bind(gc, draw, read) != Success) {
-         __glXSetCurrentContextNull();
-         __glXUnlock();
-         __glXSendError(dpy, GLXBadContext, None, opcode, False);
-         return GL_FALSE;
+         ret = GL_FALSE;
+      } else {
+         gc->currentDpy = dpy;
+         gc->currentDrawable = draw;
+         gc->currentReadable = read;
+         __glXSetCurrentContext(gc);
       }
-
-      gc->currentDpy = dpy;
-      gc->currentDrawable = draw;
-      gc->currentReadable = read;
-      __glXSetCurrentContext(gc);
-   } else {
-      __glXSetCurrentContextNull();
-   }
-
-   if (oldGC->currentDpy == NULL && oldGC != &dummyContext && oldGC->xid == None) {
-      /* We are switching away from a context that was
-       * previously destroyed, so we need to free the memory
-       * for the old handle. */
-      oldGC->vtable->destroy(oldGC);
    }
 
    __glXUnlock();
 
-   return GL_TRUE;
+   if (!ret)
+      __glXSendError(dpy, GLXBadContext, None, opcode, False);
+
+   return ret;
 }
 
 _GLX_PUBLIC Bool