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

[Roland Scheidegger]

Am 08.01.19 um 17:17 schrieb Marek Olšák:
> 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.
Well they are supposed to be...
But yes mesa/st doesn't play by the rules there, so I guess that's
better than a leak...

Roland


> 
> 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 *
>