[Mesa-dev] [PATCH] st/mesa: don't leak pipe_surface if pipe_context is not current

[Brian Paul]

LGTM.  Reviewed-by: Brian Paul <brianp at vmware.com>

On 01/08/2019 09:17 AM, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
> 
> We have found some pipe_surface leaks internally.
> 
> This is the same code as surface_destroy in radeonsi.
> Ideally, surface_destroy would be in pipe_screen.
> No, pipe_surfaces are not context objects.
> 
> Cc: 18.3 19.0 <mesa-stable at lists.freedesktop.org>
> ---
>   src/gallium/auxiliary/util/u_inlines.h | 19 +++++++++++++++++++
>   src/mesa/state_tracker/st_cb_fbo.c     |  5 ++++-
>   2 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/src/gallium/auxiliary/util/u_inlines.h b/src/gallium/auxiliary/util/u_inlines.h
> index b06fb111709..fa1e920b509 100644
> --- a/src/gallium/auxiliary/util/u_inlines.h
> +++ b/src/gallium/auxiliary/util/u_inlines.h
> @@ -147,20 +147,39 @@ pipe_resource_reference(struct pipe_resource **dst, struct pipe_resource *src)
>   
>            old_dst->screen->resource_destroy(old_dst->screen, old_dst);
>            old_dst = next;
>         } while (pipe_reference_described(&old_dst->reference, NULL,
>                                           (debug_reference_descriptor)
>                                           debug_describe_resource));
>      }
>      *dst = src;
>   }
>   
> +/**
> + * Same as pipe_surface_release, but used when pipe_context doesn't exist
> + * anymore.
> + */
> +static inline void
> +pipe_surface_release_no_context(struct pipe_surface **ptr)
> +{
> +   struct pipe_surface *surf = *ptr;
> +
> +   if (pipe_reference_described(&surf->reference, NULL,
> +                                (debug_reference_descriptor)
> +                                debug_describe_surface)) {
> +      /* trivially destroy pipe_surface */
> +      pipe_resource_reference(&surf->texture, NULL);
> +      free(surf);
> +   }
> +   *ptr = NULL;
> +}
> +
>   /**
>    * Set *dst to \p src with proper reference counting.
>    *
>    * The caller must guarantee that \p src and *dst were created in
>    * the same context (if they exist), and that this must be the current context.
>    */
>   static inline void
>   pipe_sampler_view_reference(struct pipe_sampler_view **dst,
>                               struct pipe_sampler_view *src)
>   {
> diff --git a/src/mesa/state_tracker/st_cb_fbo.c b/src/mesa/state_tracker/st_cb_fbo.c
> index 8901a8680ef..8d099f7b0f9 100644
> --- a/src/mesa/state_tracker/st_cb_fbo.c
> +++ b/src/mesa/state_tracker/st_cb_fbo.c
> @@ -278,22 +278,25 @@ st_renderbuffer_alloc_storage(struct gl_context * ctx,
>    * gl_renderbuffer::Delete()
>    */
>   static void
>   st_renderbuffer_delete(struct gl_context *ctx, struct gl_renderbuffer *rb)
>   {
>      struct st_renderbuffer *strb = st_renderbuffer(rb);
>      if (ctx) {
>         struct st_context *st = st_context(ctx);
>         pipe_surface_release(st->pipe, &strb->surface_srgb);
>         pipe_surface_release(st->pipe, &strb->surface_linear);
> -      strb->surface = NULL;
> +   } else {
> +      pipe_surface_release_no_context(&strb->surface_srgb);
> +      pipe_surface_release_no_context(&strb->surface_linear);
>      }
> +   strb->surface = NULL;
>      pipe_resource_reference(&strb->texture, NULL);
>      free(strb->data);
>      _mesa_delete_renderbuffer(ctx, rb);
>   }
>   
>   
>   /**
>    * Called via ctx->Driver.NewRenderbuffer()
>    */
>   static struct gl_renderbuffer *
>