[Mesa-dev] [Bug 98172] Concurrent call to glClientWaitSync results in segfault in one of the waiters.

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=98172

--- Comment #41 from Suzuki, Shinji <shinji.suzuki at gmail.com> ---
I think now I have better understanding of the problem we are dealing with
here.

>Not thread safe (race condition on so->fence):
>  screen->fence_reference(screen, &so->fence, NULL);
>
>Always thread safe (if fence is a local variable):
>  screen->fence_reference(screen, &fence, NULL);

I think above can be more concisely stated that
"screen->fence_reference(screen, &fence, NULL);
is thread-safe if calls are serialized otherwise not thread safe".

What's fundamentally wrong with the untouched mesa code is that
screen->fence_reference(screen, &so->fence, NULL) is potentially called more
than once. If the calls are serialized, no crash occurs because the second and
later calls behave as no-op. Protecting each call with a mutex is a way to
assure that serial execution. But that is an indirect resolution of the
problem. A direct resolution is to have screen->fence_reference() not to be
called more than once because that shared reference contributes to only one
increment in the reference count. Below is my latest attempt.

static void st_client_wait_sync(struct gl_context *ctx,
                                struct gl_sync_object *obj,
                                GLbitfield flags, GLuint64 timeout)
{
   struct pipe_screen *screen = st_context(ctx)->pipe->screen;                  
   struct st_sync_object *so = (struct st_sync_object*)obj;                     
   struct pipe_fence_handle *fence = NULL;                                      

   /* Duplicate the reference so that the fence object is guaranteed to
    * be alive at least until associated 'unref' below is executed.
    * This is important because multiple threads have to execute
    * fence_finish() concurrently even if they target same fence object
    * to deal with potentially different time-out settings.
    */
   screen->fence_reference(screen, &fence, so->fence);                          

   if (fence && screen->fence_finish(screen, fence, timeout)) {
      if( p_atomic_cmpxchg(&so->fence, fence, NULL) == fence ) {
         /* Get done with 'so->object'. This is a 'unref' op.
          * Borrow the value in 'fence' since so->fence is already
          * set to NULL by the cmpxchg above.
          */
         struct pipe_fence_handle * fence_copy = fence;                         
         screen->fence_reference(screen, &fence_copy, NULL);                    
      } 
   }
   so->b.StatusFlag = GL_TRUE;                       
   screen->fence_reference(screen, &fence, NULL);                               
}

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161021/ff464da7/attachment.html>