[Mesa-dev] [PATCH] mesa/shaderobj: Fix races on refcounts

[Timothy Arceri]

On Tue, 2016-12-27 at 14:45 -0800, Chad Versace wrote:
> Use atomic ops when updating gl_shader::RefCount.
> 
> Fixes intermittent failures and crashes in
> 'dEQP-EGL.functional.sharing.gles2.multithread.*'.
> All tests in that group now pass except
> 'dEQP-
> EGL.functional.sharing.gles2.multithread.simple_egl_server_sync.textu
> res.copyteximage2d_texsubimage2d_render'.
> 
> Tested with:
>   mesa: branch 'master' at d6545f2
>   deqp: branch 'nougat-cts-dev' at 4acf725 with additional local
> fixes
>   DEQP_TARGET: x11_egl
>   hw: Intel Broadwell 0x1616
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99085
> Cc: Mark Janes <mark.a.janes at intel.com>
> Cc: Tapani Pälli <tapani.palli at intel.com>
> Cc: Haixia Shi <hshi at chromium.org>
> Cc: mesa-stable at lists.freedesktop.org


Looks good to me. I'd been meaning to fix up the patch Steinar sent in
November and commit it. But looks like I no longer have to :)

Reviewed-by: Timothy Arceri <timothy.arceri at collabora.com>



> ---
>  src/mesa/main/shaderobj.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c
> index 6fcdf44282..2071ffff45 100644
> --- a/src/mesa/main/shaderobj.c
> +++ b/src/mesa/main/shaderobj.c
> @@ -65,14 +65,11 @@ _mesa_reference_shader(struct gl_context *ctx,
> struct gl_shader **ptr,
>     }
>     if (*ptr) {
>        /* Unreference the old shader */
> -      GLboolean deleteFlag = GL_FALSE;
>        struct gl_shader *old = *ptr;
>  
>        assert(old->RefCount > 0);
> -      old->RefCount--;
> -      deleteFlag = (old->RefCount == 0);
>  
> -      if (deleteFlag) {
> +      if (p_atomic_dec_zero(&old->RefCount)) {
>  	 if (old->Name != 0)
>  	    _mesa_HashRemove(ctx->Shared->ShaderObjects, old->Name);
>           _mesa_delete_shader(ctx, old);
> @@ -84,7 +81,7 @@ _mesa_reference_shader(struct gl_context *ctx,
> struct gl_shader **ptr,
>  
>     if (sh) {
>        /* reference new */
> -      sh->RefCount++;
> +      p_atomic_inc(&sh->RefCount);
>        *ptr = sh;
>     }
>  }
> @@ -257,14 +254,11 @@ _mesa_reference_shader_program_(struct
> gl_context *ctx,
>     }
>     if (*ptr) {
>        /* Unreference the old shader program */
> -      GLboolean deleteFlag = GL_FALSE;
>        struct gl_shader_program *old = *ptr;
>  
>        assert(old->RefCount > 0);
> -      old->RefCount--;
> -      deleteFlag = (old->RefCount == 0);
>  
> -      if (deleteFlag) {
> +      if (p_atomic_dec_zero(&old->RefCount)) {
>  	 if (old->Name != 0)
>  	    _mesa_HashRemove(ctx->Shared->ShaderObjects, old->Name);
>           _mesa_delete_shader_program(ctx, old);
> @@ -275,7 +269,7 @@ _mesa_reference_shader_program_(struct gl_context
> *ctx,
>     assert(!*ptr);
>  
>     if (shProg) {
> -      shProg->RefCount++;
> +      p_atomic_inc(&shProg->RefCount);
>        *ptr = shProg;
>     }
>  }