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

From: Derrick Stolee <stolee@gmail.com>
Date: Feb 22, 2021, 14:15 UTC
Message-ID: <6ef5487b-905d-8f34-a53c-d1138f5528d9@gmail.com>
In-Reply-To: <xmqqa6rz9zrx.fsf@gitster.g>

On 2/19/2021 10:36 PM, Junio C Hamano wrote:

Show 16 quoted lines

> "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.

Show 8 quoted lines

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