From 17090cf3efb0db8fa01b502a9c0df27cbd1a67da Mon Sep 17 00:00:00 2001 From: Chia-I Wu Date: Fri, 10 Jul 2009 15:28:55 +0800 Subject: 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 --- src/mesa/glapi/glapi.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'src/mesa/glapi/glapi.h') diff --git a/src/mesa/glapi/glapi.h b/src/mesa/glapi/glapi.h index 8f2cf66218..b2a1fe6ee9 100644 --- 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 -- cgit v1.2.3 From a94d66e85748a6ac2b2d4700b6504cc89d1e1945 Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Tue, 25 Aug 2009 09:37:43 -0600 Subject: 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(-) (limited to 'src/mesa/glapi/glapi.h') diff --git a/src/mesa/glapi/glapi.c b/src/mesa/glapi/glapi.c index 4bb10df1d8..e36fccb354 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 b2a1fe6ee9..8f2cf66218 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 a36bea7176..8ec933a851 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 -- cgit v1.2.3 From 1814d6b49c8144180231c3e43ff6b5dc9c32e12b Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Fri, 28 Aug 2009 20:12:36 -0700 Subject: Put 'extern' first on the line to silence GCC warnings. --- src/mesa/glapi/glapi.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/mesa/glapi/glapi.h') diff --git a/src/mesa/glapi/glapi.h b/src/mesa/glapi/glapi.h index 8f2cf66218..5fb5401229 100644 --- a/src/mesa/glapi/glapi.h +++ b/src/mesa/glapi/glapi.h @@ -80,8 +80,8 @@ typedef void (*_glapi_warning_func)(void *ctx, const char *str, ...); **/ #if defined (GLX_USE_TLS) -const extern void *_glapi_Context; -const extern struct _glapi_table *_glapi_Dispatch; +extern const void *_glapi_Context; +extern const struct _glapi_table *_glapi_Dispatch; extern __thread void * _glapi_tls_Context __attribute__((tls_model("initial-exec"))); -- cgit v1.2.3