glx: Demand success from CreateContext requests (v2)

GLXCreate{,New}Context, like most X resource creation requests, does not
emit a reply and therefore is emitted into the X stream asynchronously.
However, unlike most resource creation requests, the GLXContext we
return is a handle to library state instead of an XID. So if context
creation fails for any reason - say, the server doesn't support indirect
contexts - then we will fail in strange places for strange reasons.

We could make every GLX entrypoint robust against half-created contexts,
or we could just verify that context creation worked. Reuse the
__glXIsDirect code to do this, as a cheap way of verifying that the
XID is real.

glXCreateContextAttribsARB solves this by using the _checked version of
the xcb command, so effectively this change makes the classic context
creation paths as robust as CreateContextAttribs.

v2: Better use of Bool, check that error != NULL first (Olivier Fourdan)

Signed-off-by: Adam Jackson <[email protected]>
Reviewed-by: Michel Dänzer <[email protected]>

1 files changed, 55 insertions, 38 deletions

diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
index 4db0228eaba..3ed960fbf3c 100644
--- a/src/glx/glxcmds.c
+++ b/src/glx/glxcmds.c
@@ -272,6 +272,44 @@ glx_context_init(struct glx_context *gc,
    return True;
 }
 
+/**
+ * Determine if a context uses direct rendering.
+ *
+ * \param dpy        Display where the context was created.
+ * \param contextID  ID of the context to be tested.
+ * \param error      Out parameter, set to True on error if not NULL
+ *
+ * \returns \c True if the context is direct rendering or not.
+ */
+static Bool
+__glXIsDirect(Display * dpy, GLXContextID contextID, Bool *error)
+{
+   CARD8 opcode;
+   xcb_connection_t *c;
+   xcb_generic_error_t *err;
+   xcb_glx_is_direct_reply_t *reply;
+   Bool is_direct;
+
+   opcode = __glXSetupForCommand(dpy);
+   if (!opcode) {
+      return False;
+   }
+
+   c = XGetXCBConnection(dpy);
+   reply = xcb_glx_is_direct_reply(c, xcb_glx_is_direct(c, contextID), &err);
+   is_direct = (reply != NULL && reply->is_direct) ? True : False;
+
+   if (err != NULL) {
+      if (error)
+         *error = True;
+      __glXSendErrorForXcb(dpy, err);
+      free(err);
+   }
+
+   free(reply);
+
+   return is_direct;
+}
 
 /**
  * Create a new context.
@@ -376,6 +414,21 @@ CreateContext(Display *dpy, int generic_id, struct glx_config *config,
    gc->share_xid = shareList ? shareList->xid : None;
    gc->imported = GL_FALSE;
 
+   /* Unlike most X resource creation requests, we're about to return a handle
+    * with client-side state, not just an XID. To simplify error handling
+    * elsewhere in libGL, force a round-trip here to ensure the CreateContext
+    * request above succeeded.
+    */
+   {
+      Bool error = False;
+      int isDirect = __glXIsDirect(dpy, gc->xid, &error);
+
+      if (error != False || isDirect != gc->isDirect) {
+         gc->vtable->destroy(gc);
+         gc = NULL;
+      }
+   }
+
    return (GLXContext) gc;
 }
 
@@ -613,42 +666,6 @@ glXCopyContext(Display * dpy, GLXContext source_user,
 
 
 /**
- * Determine if a context uses direct rendering.
- *
- * \param dpy        Display where the context was created.
- * \param contextID  ID of the context to be tested.
- *
- * \returns \c True if the context is direct rendering or not.
- */
-static Bool
-__glXIsDirect(Display * dpy, GLXContextID contextID)
-{
-   CARD8 opcode;
-   xcb_connection_t *c;
-   xcb_generic_error_t *err;
-   xcb_glx_is_direct_reply_t *reply;
-   Bool is_direct;
-
-   opcode = __glXSetupForCommand(dpy);
-   if (!opcode) {
-      return False;
-   }
-
-   c = XGetXCBConnection(dpy);
-   reply = xcb_glx_is_direct_reply(c, xcb_glx_is_direct(c, contextID), &err);
-   is_direct = (reply != NULL && reply->is_direct) ? True : False;
-
-   if (err != NULL) {
-      __glXSendErrorForXcb(dpy, err);
-      free(err);
-   }
-
-   free(reply);
-
-   return is_direct;
-}
-
-/**
  * \todo
  * Shouldn't this function \b always return \c False when
  * \c GLX_DIRECT_RENDERING is not defined?  Do we really need to bother with
@@ -668,7 +685,7 @@ glXIsDirect(Display * dpy, GLXContext gc_user)
 #ifdef GLX_USE_APPLEGL  /* TODO: indirect on darwin */
    return False;
 #else
-   return __glXIsDirect(dpy, gc->xid);
+   return __glXIsDirect(dpy, gc->xid, NULL);
 #endif
 }
 
@@ -1428,7 +1445,7 @@ glXImportContextEXT(Display *dpy, GLXContextID contextID)
       return NULL;
    }
 
-   if (__glXIsDirect(dpy, contextID))
+   if (__glXIsDirect(dpy, contextID, NULL))
       return NULL;
 
    opcode = __glXSetupForCommand(dpy);