[Mesa-dev] [PATCH] st/mesa: fix texture deletion context mix-up issues (v2)

[Roland Scheidegger]

Looks alright to me, it's all quite tricky stuff...
Reviewed-by: Roland Scheidegger <sroland at vmware.com>


Am 22.03.19 um 20:51 schrieb Brian Paul:
> When we destroy a context, we need to temporarily make that context
> the current one for the thread.
> 
> That's because during context tear-down we make many calls to
> _mesa_reference_texobj(&texObj, NULL).  Note there's no context
> parameter.  If the texture's refcount goes to zero and we need to
> delete it, we use the thread's current context.  But if that context
> isn't the context we're tearing down, we get into trouble when
> deallocating sampler views.  See patch 593e36f956 ("st/mesa:
> implement "zombie" sampler views (v2)") for background information.
> 
> Also, we need to release any sampler views attached to the fallback
> textures.
> 
> Fixes a crash on exit with a glretrace of the Nobel Clinician
> application.
> 
> v2: at end of st_destroy_context(), check if save_ctx == ctx and
> unbind the context if so.
> ---
>  src/mesa/state_tracker/st_context.c | 51 ++++++++++++++++++++++++++++---------
>  1 file changed, 39 insertions(+), 12 deletions(-)
> 
> diff --git a/src/mesa/state_tracker/st_context.c b/src/mesa/state_tracker/st_context.c
> index f037384..09d467a 100644
> --- a/src/mesa/state_tracker/st_context.c
> +++ b/src/mesa/state_tracker/st_context.c
> @@ -917,15 +917,39 @@ st_destroy_context(struct st_context *st)
>  {
>     struct gl_context *ctx = st->ctx;
>     struct st_framebuffer *stfb, *next;
> +   struct gl_framebuffer *save_drawbuffer;
> +   struct gl_framebuffer *save_readbuffer;
> +
> +   /* Save the current context and draw/read buffers*/
> +   GET_CURRENT_CONTEXT(save_ctx);
> +   if (save_ctx) {
> +      save_drawbuffer = save_ctx->WinSysDrawBuffer;
> +      save_readbuffer = save_ctx->WinSysReadBuffer;
> +   } else {
> +      save_drawbuffer = save_readbuffer = NULL;
> +   }
>  
> -   GET_CURRENT_CONTEXT(curctx);
> +   /*
> +    * We need to bind the context we're deleting so that
> +    * _mesa_reference_texobj_() uses this context when deleting textures.
> +    * Similarly for framebuffer objects, etc.
> +    */
> +   _mesa_make_current(ctx, NULL, NULL);
>  
> -   if (curctx == NULL) {
> -      /* No current context, but we need one to release
> -       * renderbuffer surface when we release framebuffer.
> -       * So temporarily bind the context.
> -       */
> -      _mesa_make_current(ctx, NULL, NULL);
> +   /* This must be called first so that glthread has a chance to finish */
> +   _mesa_glthread_destroy(ctx);
> +
> +   _mesa_HashWalk(ctx->Shared->TexObjects, destroy_tex_sampler_cb, st);
> +
> +   /* For the fallback textures, free any sampler views belonging to this
> +    * context.
> +    */
> +   for (unsigned i = 0; i < NUM_TEXTURE_TARGETS; i++) {
> +      struct st_texture_object *stObj =
> +         st_texture_object(ctx->Shared->FallbackTex[i]);
> +      if (stObj) {
> +         st_texture_release_context_sampler_view(st, stObj);
> +      }
>     }
>  
>     st_context_free_zombie_objects(st);
> @@ -933,11 +957,6 @@ st_destroy_context(struct st_context *st)
>     mtx_destroy(&st->zombie_sampler_views.mutex);
>     mtx_destroy(&st->zombie_shaders.mutex);
>  
> -   /* This must be called first so that glthread has a chance to finish */
> -   _mesa_glthread_destroy(ctx);
> -
> -   _mesa_HashWalk(ctx->Shared->TexObjects, destroy_tex_sampler_cb, st);
> -
>     st_reference_fragprog(st, &st->fp, NULL);
>     st_reference_prog(st, &st->gp, NULL);
>     st_reference_vertprog(st, &st->vp, NULL);
> @@ -965,4 +984,12 @@ st_destroy_context(struct st_context *st)
>     st = NULL;
>  
>     free(ctx);
> +
> +   if (save_ctx == ctx) {
> +      /* unbind the context we just deleted */
> +      _mesa_make_current(NULL, NULL, NULL);
> +   } else {
> +      /* Restore the current context and draw/read buffers (may be NULL) */
> +      _mesa_make_current(save_ctx, save_drawbuffer, save_readbuffer);
> +   }
>  }
>