Revert "glapi: Fix a possible race in getting current context/dispatch."
authorBrian Paul <brianp@vmware.com>
Tue, 25 Aug 2009 15:37:43 +0000 (09:37 -0600)
committerBrian Paul <brianp@vmware.com>
Tue, 25 Aug 2009 15:37:43 +0000 (09:37 -0600)
This reverts commit 17090cf3efb0db8fa01b502a9c0df27cbd1a67da.

We're reverting this because it causes ABI breakage with the X server.
Maybe re-attempt with another patch.

src/mesa/glapi/glapi.c
src/mesa/glapi/glapi.h
src/mesa/glapi/glthread.h

index 4bb10df1d84ee8477e89cc4be70c081eba6643e2..e36fccb354cfa840115ebb6141aef62754d69d6e 100644 (file)
@@ -160,20 +160,26 @@ static GLint NoOpUnused(void)
  * the variables \c _glapi_Dispatch and \c _glapi_Context are used for this
  * purpose.
  *
- * 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.
+ * 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.
  *
+ * 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.  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.
+ * \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.
  */
 /*@{*/
 #if defined(GLX_USE_TLS)
@@ -188,9 +194,6 @@ 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)
@@ -205,9 +208,9 @@ _glthread_DECLARE_STATIC_MUTEX(ThreadCheckMutex);
 #define CHECK_MULTITHREAD_UNLOCK() _glthread_UNLOCK_MUTEX(ThreadCheckMutex)
 #endif
 
-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 */
+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 */
 
 #if defined(WIN32_THREADS)
 void FreeTSD(_glthread_TSD *p);
@@ -241,7 +244,7 @@ _glapi_check_multithread(void)
    static unsigned long knownID;
    static GLboolean firstCall = GL_TRUE;
 
-   if (!_glapi_SingleThreaded)
+   if (ThreadSafe)
       return;
 
    CHECK_MULTITHREAD_LOCK();
@@ -254,12 +257,9 @@ _glapi_check_multithread(void)
       firstCall = GL_FALSE;
    }
    else if (knownID != _glthread_GetID()) {
-      /*
-       * 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;
+      ThreadSafe = GL_TRUE;
+      _glapi_set_dispatch(NULL);
+      _glapi_set_context(NULL);
    }
    CHECK_MULTITHREAD_UNLOCK();
 #endif
@@ -279,10 +279,8 @@ _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
@@ -301,8 +299,12 @@ _glapi_get_context(void)
 #if defined(GLX_USE_TLS)
    return _glapi_tls_Context;
 #elif defined(THREADS)
-   return (_glapi_SingleThreaded)
-      ? _glapi_Context : _glthread_GetTSD(&ContextTSD);
+   if (ThreadSafe) {
+      return _glthread_GetTSD(&ContextTSD);
+   }
+   else {
+      return _glapi_Context;
+   }
 #else
    return _glapi_Context;
 #endif
@@ -517,9 +519,8 @@ _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*/
@@ -533,15 +534,17 @@ _glapi_set_dispatch(struct _glapi_table *dispatch)
 PUBLIC struct _glapi_table *
 _glapi_get_dispatch(void)
 {
+   struct _glapi_table * api;
 #if defined(GLX_USE_TLS)
-   return _glapi_tls_Dispatch;
+   api = _glapi_tls_Dispatch;
 #elif defined(THREADS)
-   return (_glapi_SingleThreaded)
-      ? _glapi_Dispatch
-      : (struct _glapi_table *) _glthread_GetTSD(&_gl_DispatchTSD);
+   api = (ThreadSafe)
+     ? (struct _glapi_table *) _glthread_GetTSD(&_gl_DispatchTSD)
+     : _glapi_Dispatch;
 #else
-   return _glapi_Dispatch;
+   api = _glapi_Dispatch;
 #endif
+   return api;
 }
 
 
index b2a1fe6ee934cdc0d5da24c12e029dd2cb3e99ac..8f2cf66218389edf0eea852661f46bf3f8b9d067 100644 (file)
@@ -94,10 +94,7 @@ extern void *_glapi_Context;
 extern struct _glapi_table *_glapi_Dispatch;
 
 # ifdef THREADS
-/* 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())
+#  define GET_CURRENT_CONTEXT(C)  GLcontext *C = (GLcontext *) (_glapi_Context ? _glapi_Context : _glapi_get_context())
 # else
 #  define GET_CURRENT_CONTEXT(C)  GLcontext *C = (GLcontext *) _glapi_Context
 # endif
index a36bea7176281976efb0e46f408c027a91dcec48..8ec933a85148fef41efcac5e10db9c263ff771fb 100644 (file)
@@ -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_SingleThreaded, 1)) \
+   ((__builtin_expect( _glapi_Dispatch != NULL, 1 )) \
        ? _glapi_Dispatch : _glapi_get_dispatch())
 # else
 #  define GET_DISPATCH() _glapi_Dispatch