Subject: Re: [PATCH 08/23] builtin/submodule--helper: fix leaking remote ref on errors

From: Junio C Hamano <[email-masked]>
Date: Wed, 25 Sep 2024 13:26:17 -0700

Patrick Steinhardt <[email-masked]> writes:

> On Mon, Sep 16, 2024 at 01:51:21PM -0500, Justin Tobler wrote:
>> On 24/09/16 01:45PM, Patrick Steinhardt wrote:
>> > When `update_submodule()` fails we return with `die_message()`.
>> > Curiously enough, this causes a memory leak because we use the
>> > `run_process_parallel()` interfaces here, which swap out the die
>> > routine.
>> 
>> Naive question, is `update_submodule()` itself being run in parallel
>> here? Is that why the die routine gets swapped out so a child process
>> dying is handled differently? Also is it correct to say leaks are not
>> considered when we "die" normally? 
>
> Hm. Revisiting this patch: my analysis was wrong. It's not the parallel
> subsystem that swaps out `die()`, but it's the fact that we call
> `die_message()`, which actually doesn't die. It really only prints the
> message you would see when we call `die()`, nothing more.
>
> I'll amend the commit message and send out the amended version once
> there is more feedback to address.

So it has been a week and half since the series was posted and it
seems that this is the only thing you might want to touch up.

What's next?  Just have an updated patch [08/23] and nothing else
and be done with it?  A v2 round of 23-patch series hopefully will
see somebody other than Justin and I lend an extra set of eyes to
double check before we merge it to 'next'?

Thanks.