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 <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
>  }
>  


------------------------------------------------------------------------------
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
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Reply via email to