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

> Patrick Steinhardt <ps@pks.im> 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'?