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

[Tapani Pälli]

Reviewed-by: Tapani Pälli <tapani.palli at intel.com>

On 12/28/2016 12:45 AM, 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.textures.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
> ---
>   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;
>      }
>   }