glapi: Fix a possible race in getting current context/dispatch.

There is a possbile race that _glapi_Context is reset by another thread
after it is tested in GET_CURRENT_CONTEXT but before it is returned.  We
definitely do not want a lock here to solve the race.  To have correct
results even under a race, no other threads should reset _glapi_Context
(or _glapi_Dispatch).

This patch adds a new global variable _glapi_SingleThreaded.  Since
_glapi_Context or _glapi_Dispatch are no longer reset,
_glapi_SingleThreaded is tested instead, before accessing them.

DRI drivers compiled with this patch applied will not work with existing
libGL.so because of the missing new symbol.  If this turns out to be a
real problem, this patch should be reverted.

Signed-off-by: Chia-I Wu <olvaffe@gmail.com>

diff --git a/src/mesa/glapi/glapi.c b/src/mesa/glapi/glapi.c
index e36fccb354cfa840115ebb6141aef62754d69d6e..4bb10df1d84ee8477e89cc4be70c081eba6643e2 100644 (file)
--- a/src/mesa/glapi/glapi.c
+++ b/src/mesa/glapi/glapi.c
@@ -160,26 +160,20 @@ static GLint NoOpUnused(void)
  * the variables \c _glapi_Dispatch and \c _glapi_Context are used for this
  * purpose.
  *
- * In the "normal" threaded case, the variables \c _glapi_Dispatch and
- * \c _glapi_Context will be \c NULL if an application is detected as being
- * multithreaded.  Single-threaded applications will use \c _glapi_Dispatch
- * and \c _glapi_Context just like the case without any threading support.
- * When \c _glapi_Dispatch and \c _glapi_Context are \c NULL, the thread state
- * data \c _gl_DispatchTSD and \c ContextTSD are used.  Drivers and the
- * static dispatch functions access these variables via \c _glapi_get_dispatch
- * and \c _glapi_get_context.
+ * In the "normal" threaded case, the variable \c _glapi_SingleThreaded will be
+ * \c GL_FALSE if an application is detected as being multithreaded.
+ * Single-threaded applications will use \c _glapi_Dispatch and \c
+ * _glapi_Context just like the case without any threading support.  When \c
+ * _glapi_SingleThreaded is \c GL_FALSE, the thread state data \c
+ * _gl_DispatchTSD and \c ContextTSD are used.  Drivers and the static dispatch
+ * functions access these variables via \c _glapi_get_dispatch and \c
+ * _glapi_get_context.
  *
- * There is a race condition in setting \c _glapi_Dispatch to \c NULL.  It is
- * possible for the original thread to be setting it at the same instant a new
- * thread, perhaps running on a different processor, is clearing it.  Because
- * of that, \c ThreadSafe, which can only ever be changed to \c GL_TRUE, is
- * used to determine whether or not the application is multithreaded.
- * 
  * In the TLS case, the variables \c _glapi_Dispatch and \c _glapi_Context are
  * hardcoded to \c NULL.  Instead the TLS variables \c _glapi_tls_Dispatch and
- * \c _glapi_tls_Context are used.  Having \c _glapi_Dispatch and
- * \c _glapi_Context be hardcoded to \c NULL maintains binary compatability
- * between TLS enabled loaders and non-TLS DRI drivers.
+ * \c _glapi_tls_Context are used.  The variable \c _glapi_SingleThreaded,
+ * though not used, is defined and hardcoded to \c GL_FALSE to maintain binary
+ * compatability between TLS enabled loaders and non-TLS DRI drivers.
  */
 /*@{*/
 #if defined(GLX_USE_TLS)
@@ -194,6 +188,9 @@ PUBLIC __thread void * _glapi_tls_Context
 PUBLIC const struct _glapi_table *_glapi_Dispatch = NULL;
 PUBLIC const void *_glapi_Context = NULL;
 
+/* Unused, but maintain binary compatability with non-TLS DRI drivers */
+GLboolean _glapi_SingleThreaded = GL_FALSE;
+
 #else
 
 #if defined(THREADS)
@@ -208,9 +205,9 @@ _glthread_DECLARE_STATIC_MUTEX(ThreadCheckMutex);
 #define CHECK_MULTITHREAD_UNLOCK() _glthread_UNLOCK_MUTEX(ThreadCheckMutex)
 #endif
 
-static GLboolean ThreadSafe = GL_FALSE;  /**< In thread-safe mode? */
-_glthread_TSD _gl_DispatchTSD;           /**< Per-thread dispatch pointer */
-static _glthread_TSD ContextTSD;         /**< Per-thread context pointer */
+GLboolean _glapi_SingleThreaded = GL_TRUE;  /**< In single-thread mode? */
+_glthread_TSD _gl_DispatchTSD;              /**< Per-thread dispatch pointer */
+static _glthread_TSD ContextTSD;            /**< Per-thread context pointer */
 
 #if defined(WIN32_THREADS)
 void FreeTSD(_glthread_TSD *p);
@@ -244,7 +241,7 @@ _glapi_check_multithread(void)
    static unsigned long knownID;
    static GLboolean firstCall = GL_TRUE;
 
-   if (ThreadSafe)
+   if (!_glapi_SingleThreaded)
       return;
 
    CHECK_MULTITHREAD_LOCK();
@@ -257,9 +254,12 @@ _glapi_check_multithread(void)
       firstCall = GL_FALSE;
    }
    else if (knownID != _glthread_GetID()) {
-      ThreadSafe = GL_TRUE;
-      _glapi_set_dispatch(NULL);
-      _glapi_set_context(NULL);
+      /*
+       * switch to thread-safe mode.  _glapi_Context and _glapi_Dispatch are no
+       * longer accessed after this point, except for raced by the first
+       * thread.  Because of the race, they cannot be reset to NULL.
+       */
+      _glapi_SingleThreaded = GL_FALSE;
    }
    CHECK_MULTITHREAD_UNLOCK();
 #endif
@@ -279,8 +279,10 @@ _glapi_set_context(void *context)
 #if defined(GLX_USE_TLS)
    _glapi_tls_Context = context;
 #elif defined(THREADS)
+   if (_glapi_SingleThreaded)
+      _glapi_Context = context;
+   /* always update TSD because we might switch to it at any time */
    _glthread_SetTSD(&ContextTSD, context);
-   _glapi_Context = (ThreadSafe) ? NULL : context;
 #else
    _glapi_Context = context;
 #endif
@@ -299,12 +301,8 @@ _glapi_get_context(void)
 #if defined(GLX_USE_TLS)
    return _glapi_tls_Context;
 #elif defined(THREADS)
-   if (ThreadSafe) {
-      return _glthread_GetTSD(&ContextTSD);
-   }
-   else {
-      return _glapi_Context;
-   }
+   return (_glapi_SingleThreaded)
+      ? _glapi_Context : _glthread_GetTSD(&ContextTSD);
 #else
    return _glapi_Context;
 #endif
@@ -519,8 +517,9 @@ _glapi_set_dispatch(struct _glapi_table *dispatch)
 #if defined(GLX_USE_TLS)
    _glapi_tls_Dispatch = dispatch;
 #elif defined(THREADS)
+   if (_glapi_SingleThreaded)
+      _glapi_Dispatch = dispatch;
    _glthread_SetTSD(&_gl_DispatchTSD, (void *) dispatch);
-   _glapi_Dispatch = (ThreadSafe) ? NULL : dispatch;
 #else /*THREADS*/
    _glapi_Dispatch = dispatch;
 #endif /*THREADS*/
@@ -534,17 +533,15 @@ _glapi_set_dispatch(struct _glapi_table *dispatch)
 PUBLIC struct _glapi_table *
 _glapi_get_dispatch(void)
 {
-   struct _glapi_table * api;
 #if defined(GLX_USE_TLS)
-   api = _glapi_tls_Dispatch;
+   return _glapi_tls_Dispatch;
 #elif defined(THREADS)
-   api = (ThreadSafe)
-     ? (struct _glapi_table *) _glthread_GetTSD(&_gl_DispatchTSD)
-     : _glapi_Dispatch;
+   return (_glapi_SingleThreaded)
+      ? _glapi_Dispatch
+      : (struct _glapi_table *) _glthread_GetTSD(&_gl_DispatchTSD);
 #else
-   api = _glapi_Dispatch;
+   return _glapi_Dispatch;
 #endif
-   return api;
 }
 
 

diff --git a/src/mesa/glapi/glapi.h b/src/mesa/glapi/glapi.h
index 8f2cf66218389edf0eea852661f46bf3f8b9d067..b2a1fe6ee934cdc0d5da24c12e029dd2cb3e99ac 100644 (file)
--- a/src/mesa/glapi/glapi.h
+++ b/src/mesa/glapi/glapi.h
@@ -94,7 +94,10 @@ extern void *_glapi_Context;
 extern struct _glapi_table *_glapi_Dispatch;
 
 # ifdef THREADS
-#  define GET_CURRENT_CONTEXT(C)  GLcontext *C = (GLcontext *) (_glapi_Context ? _glapi_Context : _glapi_get_context())
+/* this variable is here only for quick access to current context/dispatch */
+extern GLboolean _glapi_SingleThreaded;
+#  define GET_CURRENT_CONTEXT(C)  GLcontext *C = (GLcontext *) \
+       ((_glapi_SingleThreaded) ? _glapi_Context : _glapi_get_context())
 # else
 #  define GET_CURRENT_CONTEXT(C)  GLcontext *C = (GLcontext *) _glapi_Context
 # endif

diff --git a/src/mesa/glapi/glthread.h b/src/mesa/glapi/glthread.h
index 8ec933a85148fef41efcac5e10db9c263ff771fb..a36bea7176281976efb0e46f408c027a91dcec48 100644 (file)
--- a/src/mesa/glapi/glthread.h
+++ b/src/mesa/glapi/glthread.h
@@ -322,7 +322,7 @@ extern __thread struct _glapi_table * _glapi_tls_Dispatch
 #elif !defined(GL_CALL)
 # if defined(THREADS)
 #  define GET_DISPATCH() \
-   ((__builtin_expect( _glapi_Dispatch != NULL, 1 )) \
+   ((__builtin_expect(_glapi_SingleThreaded, 1)) \
        ? _glapi_Dispatch : _glapi_get_dispatch())
 # else
 #  define GET_DISPATCH() _glapi_Dispatch