[Mesa-dev] [PATCH] st/mesa: destroy pipe_context before destroying st_context (v2)

[Nicolai Hähnle]

Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>

On 23.01.2017 18:32, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
>
> If radeonsi starts compiling an optimized shader variant asynchronously
> with a GL debug callback set and the application destroys the GL context,
> radeonsi crashes when trying to write shader stats into the debug output
> of a non-existent context after compilation, because st/mesa was destroyed
> before pipe_context.
>
> Firefox with WebGL2 enabled hits this bug.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99456
>
> v2: protect against a double destroy in st_create_context_priv and callers.
>
> Cc: 17.0 <mesa-stable at lists.freedesktop.org>
> ---
>  src/mesa/state_tracker/st_context.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_context.c b/src/mesa/state_tracker/st_context.c
> index 0eae971..5523734 100644
> --- a/src/mesa/state_tracker/st_context.c
> +++ b/src/mesa/state_tracker/st_context.c
> @@ -271,21 +271,21 @@ void st_invalidate_state(struct gl_context * ctx, GLbitfield new_state)
>        st->dirty |= st->active_states & ST_NEW_CONSTANTS;
>
>     /* This is the only core Mesa module we depend upon.
>      * No longer use swrast, swsetup, tnl.
>      */
>     _vbo_InvalidateState(ctx, new_state);
>  }
>
>
>  static void
> -st_destroy_context_priv(struct st_context *st)
> +st_destroy_context_priv(struct st_context *st, bool destroy_pipe)
>  {
>     uint shader, i;
>
>     st_destroy_atoms( st );
>     st_destroy_draw( st );
>     st_destroy_clear(st);
>     st_destroy_bitmap(st);
>     st_destroy_drawpix(st);
>     st_destroy_drawtex(st);
>     st_destroy_perfmon(st);
> @@ -307,20 +307,24 @@ st_destroy_context_priv(struct st_context *st)
>     }
>
>     /* free glDrawPixels cache data */
>     free(st->drawpix_cache.image);
>     pipe_resource_reference(&st->drawpix_cache.texture, NULL);
>
>     /* free glReadPixels cache data */
>     st_invalidate_readpix_cache(st);
>
>     cso_destroy_context(st->cso_context);
> +
> +   if (st->pipe && destroy_pipe)
> +      st->pipe->destroy(st->pipe);
> +
>     free( st );
>  }
>
>
>  static struct st_context *
>  st_create_context_priv( struct gl_context *ctx, struct pipe_context *pipe,
>  		const struct st_config_options *options)
>  {
>     struct pipe_screen *screen = pipe->screen;
>     uint i;
> @@ -496,21 +500,21 @@ st_create_context_priv( struct gl_context *ctx, struct pipe_context *pipe,
>     st->shader_has_one_variant[MESA_SHADER_TESS_EVAL] = st->has_shareable_shaders;
>     st->shader_has_one_variant[MESA_SHADER_GEOMETRY] = st->has_shareable_shaders;
>     st->shader_has_one_variant[MESA_SHADER_COMPUTE] = st->has_shareable_shaders;
>
>     _mesa_compute_version(ctx);
>
>     if (ctx->Version == 0) {
>        /* This can happen when a core profile was requested, but the driver
>         * does not support some features of GL 3.1 or later.
>         */
> -      st_destroy_context_priv(st);
> +      st_destroy_context_priv(st, false);
>        return NULL;
>     }
>
>     _mesa_initialize_dispatch_tables(ctx);
>     _mesa_initialize_vbo_vtxfmt(ctx);
>
>     return st;
>  }
>
>  static void st_init_driver_flags(struct gl_driver_flags *f)
> @@ -572,21 +576,20 @@ static void
>  destroy_tex_sampler_cb(GLuint id, void *data, void *userData)
>  {
>     struct gl_texture_object *texObj = (struct gl_texture_object *) data;
>     struct st_context *st = (struct st_context *) userData;
>
>     st_texture_release_sampler_view(st, st_texture_object(texObj));
>  }
>
>  void st_destroy_context( struct st_context *st )
>  {
> -   struct pipe_context *pipe = st->pipe;
>     struct gl_context *ctx = st->ctx;
>     GLuint i;
>
>     _mesa_HashWalk(ctx->Shared->TexObjects, destroy_tex_sampler_cb, st);
>
>     st_reference_fragprog(st, &st->fp, NULL);
>     st_reference_geomprog(st, &st->gp, NULL);
>     st_reference_vertprog(st, &st->vp, NULL);
>     st_reference_tesscprog(st, &st->tcp, NULL);
>     st_reference_tesseprog(st, &st->tep, NULL);
> @@ -601,25 +604,23 @@ void st_destroy_context( struct st_context *st )
>     pipe_resource_reference(&st->pixel_xfer.pixelmap_texture, NULL);
>
>     _vbo_DestroyContext(ctx);
>
>     st_destroy_program_variants(st);
>
>     _mesa_free_context_data(ctx);
>
>     /* This will free the st_context too, so 'st' must not be accessed
>      * afterwards. */
> -   st_destroy_context_priv(st);
> +   st_destroy_context_priv(st, true);
>     st = NULL;
>
> -   pipe->destroy( pipe );
> -
>     free(ctx);
>  }
>
>  static void
>  st_emit_string_marker(struct gl_context *ctx, const GLchar *string, GLsizei len)
>  {
>     struct st_context *st = ctx->st;
>     st->pipe->emit_string_marker(st->pipe, string, len);
>  }
>
>