From 2b8f97d0ff0836b1d1c8753a81a8810df385b21d Mon Sep 17 00:00:00 2001 From: Timothy Arceri Date: Thu, 17 Nov 2016 12:26:08 +1100 Subject: [PATCH] st/mesa/i965: simplify gl_program references and stop leaking In i965 we were calling _mesa_reference_program() after creating gl_program and then later calling it again with NULL as a param to get the refcount back down to 1. This changes things to not use _mesa_reference_program() at all and just have gl_linked_shader take ownership of gl_program since refcount starts at 1. The st and ir_to_mesa linkers were worse as they were both getting in a state were the refcount would never get to 0 and we would leak the program. Reviewed-by: Emil Velikov --- src/mesa/drivers/dri/i965/brw_link.cpp | 5 ++--- src/mesa/program/ir_to_mesa.cpp | 7 ++++--- src/mesa/state_tracker/st_glsl_to_nir.cpp | 3 ++- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 9 +++------ 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp b/src/mesa/drivers/dri/i965/brw_link.cpp index fbb834be1a0..a0c9e20807c 100644 --- a/src/mesa/drivers/dri/i965/brw_link.cpp +++ b/src/mesa/drivers/dri/i965/brw_link.cpp @@ -227,7 +227,8 @@ brw_link_shader(struct gl_context *ctx, struct gl_shader_program *shProg) if (!prog) return false; - _mesa_reference_program(ctx, &shader->Program, prog); + /* Don't use _mesa_reference_program() just take ownership */ + shader->Program = prog; prog->Parameters = _mesa_new_parameter_list(); @@ -276,8 +277,6 @@ brw_link_shader(struct gl_context *ctx, struct gl_shader_program *shProg) prog->nir = brw_create_nir(brw, shProg, prog, (gl_shader_stage) stage, compiler->scalar_stage[stage]); - - _mesa_reference_program(ctx, &prog, NULL); } if ((ctx->_Shader->Flags & GLSL_DUMP) && shProg->Name != 0) { diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp index b042c86f939..7f4212333f1 100644 --- a/src/mesa/program/ir_to_mesa.cpp +++ b/src/mesa/program/ir_to_mesa.cpp @@ -2929,7 +2929,8 @@ get_mesa_program(struct gl_context *ctx, prog->info.fs.depth_layout = shader_program->FragDepthLayout; } - _mesa_reference_program(ctx, &shader->Program, prog); + /* Don't use _mesa_reference_program() just take ownership */ + shader->Program = prog; if ((ctx->_Shader->Flags & GLSL_NO_OPT) == 0) { _mesa_optimize_program(ctx, prog, prog); @@ -3033,11 +3034,11 @@ _mesa_ir_link_shader(struct gl_context *ctx, struct gl_shader_program *prog) if (!ctx->Driver.ProgramStringNotify(ctx, _mesa_shader_stage_to_program(i), linked_prog)) { + _mesa_reference_program(ctx, &prog->_LinkedShaders[i]->Program, + NULL); return GL_FALSE; } } - - _mesa_reference_program(ctx, &linked_prog, NULL); } build_program_resource_list(ctx, prog); diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp b/src/mesa/state_tracker/st_glsl_to_nir.cpp index 36531ad3047..2d8cf81dcce 100644 --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp @@ -375,7 +375,8 @@ st_nir_get_mesa_program(struct gl_context *ctx, if (!prog) return NULL; - _mesa_reference_program(ctx, &shader->Program, prog); + /* Don't use _mesa_reference_program() just take ownership */ + shader->Program = prog; prog->Parameters = _mesa_new_parameter_list(); diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp index 7d67a598f0a..b47e1470e3f 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp @@ -6419,7 +6419,8 @@ get_mesa_program_tgsi(struct gl_context *ctx, if (!prog) return NULL; - _mesa_reference_program(ctx, &shader->Program, prog); + /* Don't use _mesa_reference_program() just take ownership */ + shader->Program = prog; prog->Parameters = _mesa_new_parameter_list(); v = new glsl_to_tgsi_visitor(); @@ -6541,6 +6542,7 @@ get_mesa_program_tgsi(struct gl_context *ctx, _mesa_associate_uniform_storage(ctx, shader_program, prog->Parameters); if (!shader_program->LinkStatus) { free_glsl_to_tgsi_visitor(v); + _mesa_reference_program(ctx, &shader->Program, NULL); return NULL; } @@ -6883,19 +6885,14 @@ st_link_shader(struct gl_context *ctx, struct gl_shader_program *prog) linked_prog = get_mesa_program(ctx, prog, prog->_LinkedShaders[i]); if (linked_prog) { - _mesa_reference_program(ctx, &prog->_LinkedShaders[i]->Program, - linked_prog); if (!ctx->Driver.ProgramStringNotify(ctx, _mesa_shader_stage_to_program(i), linked_prog)) { _mesa_reference_program(ctx, &prog->_LinkedShaders[i]->Program, NULL); - _mesa_reference_program(ctx, &linked_prog, NULL); return GL_FALSE; } } - - _mesa_reference_program(ctx, &linked_prog, NULL); } return GL_TRUE; -- 2.30.2