Re: [PATCH] commit-graph: avoid leaking topo_levels slab in write_commit_graph()

[Derrick Stolee <stolee@gmail.com>]

On 2/19/2021 10:36 PM, Junio C Hamano wrote:
> "Andrzej Hunt via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Andrzej Hunt <ajrhunt@google.com>
>>
>> write_commit_graph initialises topo_levels using init_topo_level_slab(),
>> next it calls compute_topological_levels() which can cause the slab to
>> grow, we therefore need to clear the slab again using
>> clear_topo_level_slab() when we're done.
>>
>> First introduced in 72a2bfcaf01860ce8dd6921490d903dc0ad59c89 - which
>> is currently only in master and not on maint.
> 
> Thanks.  
> 
> Forwarding to those who were involved in the said commit for
> insights.

>> index ed31843fa522..9529ec552139 100644
>> --- a/commit-graph.c
>> +++ b/commit-graph.c
>> @@ -2471,6 +2471,7 @@ int write_commit_graph(struct object_directory *odb,
>>  	free(ctx->graph_name);
>>  	free(ctx->commits.list);
>>  	oid_array_clear(&ctx->oids);
>> +	clear_topo_level_slab(&topo_levels);

This change looks like a sane change to me. It definitely fixes a leak.
The leak "wasn't hurting anybody" because write_commit_graph() is only
called at most once per process, and the process closes itself out
shortly after. Still, it's good to have good memory hygiene here.

Thanks,
-Stolee