Hi,
This patch series was sent about 1 month ago. It went through several
iterations, and should have resolved all concerns. The patches were
scattered in that thread.
This mail resends the series, with updated commit logs. It fixes random
crashes at initialization time in progs/xdemos/glthreads and
progs/egl/xeglthreads.
--
Regards,
olv
>From c954c1eaabd7dc325c7799d7910c2fa6f255b6d4 Mon Sep 17 00:00:00 2001
From: Chia-I Wu <olva...@gmail.com>
Date: Fri, 10 Jul 2009 15:21:42 +0800
Subject: [PATCH 1/4] 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 <ian.d.roman...@intel.com>
Signed-off-by: Chia-I Wu <olva...@gmail.com>
---
src/mesa/glapi/glapi.c | 29 +++++++++++++++--------------
1 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/src/mesa/glapi/glapi.c b/src/mesa/glapi/glapi.c
index 2b105d0..30aec20 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
}
--
1.6.2.4
>From fd8e296c2cd59bed864c999eb7e1294170e19bab Mon Sep 17 00:00:00 2001
From: Chia-I Wu <olva...@gmail.com>
Date: Fri, 10 Jul 2009 17:35:11 +0800
Subject: [PATCH 2/4] 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 (wronly) initialize it more than once. This patch
solves the race by initializing context/dispatch TSDs early.
Acked-by: Ian Romanick <ian.d.roman...@intel.com>
Signed-off-by: Chia-I Wu <olva...@gmail.com>
---
src/mesa/glapi/glapi.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/src/mesa/glapi/glapi.c b/src/mesa/glapi/glapi.c
index 30aec20..b9ab9c0 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;
}
--
1.6.2.4
>From 91c21b823b45782f811ebc6e1ad35232a944ec87 Mon Sep 17 00:00:00 2001
From: Chia-I Wu <olva...@gmail.com>
Date: Tue, 14 Jul 2009 13:17:25 +0800
Subject: [PATCH 3/4] 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 <olva...@gmail.com>
---
src/mesa/glapi/glapi.c | 13 +++++++++++--
1 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/mesa/glapi/glapi.c b/src/mesa/glapi/glapi.c
index b9ab9c0..e36fccb 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
}
--
1.6.2.4
>From 98b0e891bd8cf262020b6dae7ebd8f142555a6ed Mon Sep 17 00:00:00 2001
From: Chia-I Wu <olva...@gmail.com>
Date: Fri, 10 Jul 2009 15:28:55 +0800
Subject: [PATCH 4/4] 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 <olva...@gmail.com>
---
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 e36fccb..4bb10df 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 8f2cf66..b2a1fe6 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 8ec933a..a36bea7 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
--
1.6.2.4
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
