From a94d66e85748a6ac2b2d4700b6504cc89d1e1945 Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Tue, 25 Aug 2009 09:37:43 -0600 Subject: [PATCH] Revert "glapi: Fix a possible race in getting current context/dispatch." 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 | 75 ++++++++++++++++++++------------------- src/mesa/glapi/glapi.h | 5 +-- src/mesa/glapi/glthread.h | 2 +- 3 files changed, 41 insertions(+), 41 deletions(-) diff --git a/src/mesa/glapi/glapi.c b/src/mesa/glapi/glapi.c index 4bb10df1d84..e36fccb354c 100644 --- a/src/mesa/glapi/glapi.c +++ b/src/mesa/glapi/glapi.c @@ -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; } diff --git a/src/mesa/glapi/glapi.h b/src/mesa/glapi/glapi.h index b2a1fe6ee93..8f2cf662183 100644 --- a/src/mesa/glapi/glapi.h +++ b/src/mesa/glapi/glapi.h @@ -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 diff --git a/src/mesa/glapi/glthread.h b/src/mesa/glapi/glthread.h index a36bea71762..8ec933a8514 100644 --- 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_SingleThreaded, 1)) \ + ((__builtin_expect( _glapi_Dispatch != NULL, 1 )) \ ? _glapi_Dispatch : _glapi_get_dispatch()) # else # define GET_DISPATCH() _glapi_Dispatch -- 2.30.2