From 99939982ec76cb1fee84da9027375c55888dca20 Mon Sep 17 00:00:00 2001 From: Chia-I Wu Date: Fri, 10 Jul 2009 15:21:42 +0800 Subject: glapi: Protect _glapi_check_multithread by a mutex. Multiple threads might call _glapi_check_multithread at roughly the same time. It is possbile that all of them are wrongly regarded as firstCall if there is no mutex. This bug causes xeglthreads to crash sometimes. Acked-by: Ian Romanick Signed-off-by: Chia-I Wu --- src/mesa/glapi/glapi.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) (limited to 'src/mesa/glapi/glapi.c') diff --git a/src/mesa/glapi/glapi.c b/src/mesa/glapi/glapi.c index 2b105d0f17..30aec209e7 100644 --- a/src/mesa/glapi/glapi.c +++ b/src/mesa/glapi/glapi.c @@ -198,6 +198,7 @@ PUBLIC const void *_glapi_Context = NULL; #if defined(THREADS) +_glthread_DECLARE_STATIC_MUTEX(ThreadCheckMutex); 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 */ @@ -231,23 +232,23 @@ void _glapi_check_multithread(void) { #if defined(THREADS) && !defined(GLX_USE_TLS) - if (!ThreadSafe) { - static unsigned long knownID; - static GLboolean firstCall = GL_TRUE; - if (firstCall) { - knownID = _glthread_GetID(); - firstCall = GL_FALSE; - } - else if (knownID != _glthread_GetID()) { - ThreadSafe = GL_TRUE; - _glapi_set_dispatch(NULL); - _glapi_set_context(NULL); - } + static unsigned long knownID; + static GLboolean firstCall = GL_TRUE; + + if (ThreadSafe) + return; + + _glthread_LOCK_MUTEX(ThreadCheckMutex); + if (firstCall) { + knownID = _glthread_GetID(); + firstCall = GL_FALSE; } - else if (!_glapi_get_dispatch()) { - /* make sure that this thread's dispatch pointer isn't null */ + else if (knownID != _glthread_GetID()) { + ThreadSafe = GL_TRUE; _glapi_set_dispatch(NULL); + _glapi_set_context(NULL); } + _glthread_UNLOCK_MUTEX(ThreadCheckMutex); #endif } -- cgit v1.2.3 From fc2feea685d86c520fb01199caa5a46eae20c7aa Mon Sep 17 00:00:00 2001 From: Chia-I Wu Date: Fri, 10 Jul 2009 17:35:11 +0800 Subject: glapi: Fix a race in accessing context/dispatch TSD. If multiple threads set/get a TSD at roughly same time for the first time, glthread might (wrongly) initialize it more than once. This patch solves the race by initializing context/dispatch TSDs early. Acked-by: Ian Romanick Signed-off-by: Chia-I Wu --- src/mesa/glapi/glapi.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/mesa/glapi/glapi.c') diff --git a/src/mesa/glapi/glapi.c b/src/mesa/glapi/glapi.c index 30aec209e7..b9ab9c07be 100644 --- a/src/mesa/glapi/glapi.c +++ b/src/mesa/glapi/glapi.c @@ -240,6 +240,10 @@ _glapi_check_multithread(void) _glthread_LOCK_MUTEX(ThreadCheckMutex); if (firstCall) { + /* initialize TSDs */ + (void) _glthread_GetTSD(&ContextTSD); + (void) _glthread_GetTSD(&_gl_DispatchTSD); + knownID = _glthread_GetID(); firstCall = GL_FALSE; } -- cgit v1.2.3 From 3076d1617db67d49ff773096123c1fa47af58272 Mon Sep 17 00:00:00 2001 From: Chia-I Wu Date: Tue, 14 Jul 2009 13:17:25 +0800 Subject: glapi: Static mutex does not work on WIN32_THREADS. This re-introduces the race in _glapi_check_multithread, but avoids a crash on windows. Signed-off-by: Chia-I Wu --- src/mesa/glapi/glapi.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'src/mesa/glapi/glapi.c') diff --git a/src/mesa/glapi/glapi.c b/src/mesa/glapi/glapi.c index b9ab9c07be..e36fccb354 100644 --- a/src/mesa/glapi/glapi.c +++ b/src/mesa/glapi/glapi.c @@ -198,7 +198,16 @@ PUBLIC const void *_glapi_Context = NULL; #if defined(THREADS) +#ifdef WIN32_THREADS +/* _glthread_DECLARE_STATIC_MUTEX is broken on windows. There will be race! */ +#define CHECK_MULTITHREAD_LOCK() +#define CHECK_MULTITHREAD_UNLOCK() +#else _glthread_DECLARE_STATIC_MUTEX(ThreadCheckMutex); +#define CHECK_MULTITHREAD_LOCK() _glthread_LOCK_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 */ @@ -238,7 +247,7 @@ _glapi_check_multithread(void) if (ThreadSafe) return; - _glthread_LOCK_MUTEX(ThreadCheckMutex); + CHECK_MULTITHREAD_LOCK(); if (firstCall) { /* initialize TSDs */ (void) _glthread_GetTSD(&ContextTSD); @@ -252,7 +261,7 @@ _glapi_check_multithread(void) _glapi_set_dispatch(NULL); _glapi_set_context(NULL); } - _glthread_UNLOCK_MUTEX(ThreadCheckMutex); + CHECK_MULTITHREAD_UNLOCK(); #endif } -- cgit v1.2.3 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.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.c') diff --git a/src/mesa/glapi/glapi.c b/src/mesa/glapi/glapi.c index e36fccb354..4bb10df1d8 100644 --- 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 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 diff --git a/src/mesa/glapi/glthread.h b/src/mesa/glapi/glthread.h index 8ec933a851..a36bea7176 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_Dispatch != NULL, 1 )) \ + ((__builtin_expect(_glapi_SingleThreaded, 1)) \ ? _glapi_Dispatch : _glapi_get_dispatch()) # else # define GET_DISPATCH() _glapi_Dispatch -- cgit v1.2.3