Hi,
Sorry for the late review.
Your patches look good, with the exception of relying on
_glthread_DECLARE_STATIC_MUTEX which is broken on windows, and windows
too fits on the THREADS && !GLX_USE_TLS use case.
So we need to fix _glthread_DECLARE_STATIC_MUTEX on windows first, or
these lines should be added
#ifdef _WIN32
/* FIXME: _glthread_DECLARE_STATIC_MUTEX is broken on windows */
static int onetime = 0;
if(!onetime) {
_glthread_INIT_MUTEX(ThreadCheckMutex);
onetime = 1;
}
#endif
Which re-introduces the race condition, but at least it prevents it from
crashing on the first access to the ThreadCheckMutex. This until we find
a proper way of implementing _glthread_DECLARE_STATIC_MUTEX on windows.
Michal, I recall you mentioned some way of statically initializing
CRITICAL_SECTIONs on windows, but I can't find anything on it on google.
The only ways I can find are either using atomic operations on a flag,
or using constructors/DLL_PROCESS_ATTACH etc.
Jose
On Fri, 2009-07-10 at 20:44 -0700, Chia-I Wu wrote:
> 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 <[email protected]>
> Signed-off-by: Chia-I Wu <[email protected]>
> ---
> 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
> }
>
------------------------------------------------------------------------------
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
