-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Chia-I Wu wrote:
> 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. To
> avoid the race in a lockless fashion, no threads should reset
> _glapi_Context (or _glapi_Dispatch).
>
> This patch renames ThreadSafe to _glapi_ThreadSafe and makes it global
> so that it can be tested for thread-safe mode. This also removes a
> __builtin_expect so that multithreaded apps are not punished.
> ---
> src/mesa/glapi/glapi.c | 32 ++++++++++++++++----------------
> src/mesa/glapi/glapi.h | 3 ++-
> src/mesa/glapi/glthread.h | 2 +-
> 3 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/src/mesa/glapi/glapi.c b/src/mesa/glapi/glapi.c
> index b9ab9c0..b05f80c 100644
> --- a/src/mesa/glapi/glapi.c
> +++ b/src/mesa/glapi/glapi.c
> @@ -172,8 +172,8 @@ static GLint NoOpUnused(void)
> * 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.
> + * of that, \c _glapi_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
> @@ -199,7 +199,7 @@ PUBLIC const void *_glapi_Context = NULL;
> #if defined(THREADS)
>
> _glthread_DECLARE_STATIC_MUTEX(ThreadCheckMutex);
> -static GLboolean ThreadSafe = GL_FALSE; /**< In thread-safe mode? */
> +GLboolean _glapi_ThreadSafe = GL_FALSE; /**< In thread-safe mode? */
I've never been a fan of this variable name. Unless Brian objects, I'd
like to see this changed to _glapi_SingleThreaded or something similar.
That makes the real meaning a bit more clear, I think. Also, shouldn't
this be declared volatile? (I'm not 100% sure one way or the other.)
> _glthread_TSD _gl_DispatchTSD; /**< Per-thread dispatch pointer */
> static _glthread_TSD ContextTSD; /**< Per-thread context pointer */
>
> @@ -235,7 +235,7 @@ _glapi_check_multithread(void)
> static unsigned long knownID;
> static GLboolean firstCall = GL_TRUE;
>
> - if (ThreadSafe)
> + if (_glapi_ThreadSafe)
> return;
>
> _glthread_LOCK_MUTEX(ThreadCheckMutex);
> @@ -248,9 +248,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.
> + */
> + _glapi_ThreadSafe = GL_TRUE;
> }
> _glthread_UNLOCK_MUTEX(ThreadCheckMutex);
> #endif
> @@ -270,8 +273,9 @@ _glapi_set_context(void *context)
> #if defined(GLX_USE_TLS)
> _glapi_tls_Context = context;
> #elif defined(THREADS)
> + if (!_glapi_ThreadSafe)
> + _glapi_Context = context;
> _glthread_SetTSD(&ContextTSD, context);
> - _glapi_Context = (ThreadSafe) ? NULL : context;
> #else
> _glapi_Context = context;
> #endif
> @@ -290,12 +294,7 @@ _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_ThreadSafe) ? _glthread_GetTSD(&ContextTSD) :
> _glapi_Context;
> #else
> return _glapi_Context;
> #endif
> @@ -510,8 +509,9 @@ _glapi_set_dispatch(struct _glapi_table *dispatch)
> #if defined(GLX_USE_TLS)
> _glapi_tls_Dispatch = dispatch;
> #elif defined(THREADS)
> + if (!_glapi_ThreadSafe)
> + _glapi_Dispatch = dispatch;
> _glthread_SetTSD(&_gl_DispatchTSD, (void *) dispatch);
> - _glapi_Dispatch = (ThreadSafe) ? NULL : dispatch;
> #else /*THREADS*/
> _glapi_Dispatch = dispatch;
> #endif /*THREADS*/
> @@ -529,7 +529,7 @@ _glapi_get_dispatch(void)
> #if defined(GLX_USE_TLS)
> api = _glapi_tls_Dispatch;
> #elif defined(THREADS)
> - api = (ThreadSafe)
> + api = (_glapi_ThreadSafe)
> ? (struct _glapi_table *) _glthread_GetTSD(&_gl_DispatchTSD)
> : _glapi_Dispatch;
> #else
> diff --git a/src/mesa/glapi/glapi.h b/src/mesa/glapi/glapi.h
> index 8f2cf66..e935195 100644
> --- a/src/mesa/glapi/glapi.h
> +++ b/src/mesa/glapi/glapi.h
> @@ -91,10 +91,11 @@ extern __thread void * _glapi_tls_Context
> #else
>
> extern void *_glapi_Context;
> +extern GLboolean _glapi_ThreadSafe;
> extern struct _glapi_table *_glapi_Dispatch;
>
> # ifdef THREADS
> -# define GET_CURRENT_CONTEXT(C) GLcontext *C = (GLcontext *)
> (_glapi_Context ? _glapi_Context : _glapi_get_context())
> +# define GET_CURRENT_CONTEXT(C) GLcontext *C = (GLcontext *)
> ((!_glapi_ThreadSafe) ? _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 dfe09a9..5b19883 100644
> --- a/src/mesa/glapi/glthread.h
> +++ b/src/mesa/glapi/glthread.h
> @@ -364,7 +364,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 )) \
> + ((!_glapi_ThreadSafe) \
> ? _glapi_Dispatch : _glapi_get_dispatch())
Keep the __builtin_expect. Applications almost always use OpenGL from a
single thread even if the application is multithreaded.
> # else
> # define GET_DISPATCH() _glapi_Dispatch
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iEYEARECAAYFAkpXj/EACgkQX1gOwKyEAw89LwCeMeBmEfJpdiqXa7WKyphV6DCA
pI0AnAuZK2Q718ZBtkXbTiiiK4eCUqmi
=/3jW
-----END PGP SIGNATURE-----
------------------------------------------------------------------------------
Enter the BlackBerry Developer Challenge
This is your chance to win up to $100,000 in prizes! For a limited time,
vendors submitting new applications to BlackBerry App World(TM) will have
the opportunity to enter the BlackBerry Developer Challenge. See full prize
details at: http://p.sf.net/sfu/Challenge
_______________________________________________
Mesa3d-dev mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
