Re: [PATCH v2 00/21] Various memory leak fixes

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2363 bytes --]

On Fri, May 24, 2024 at 07:10:09PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > this is the second version of my patch series that fixes various memory
> > leaks in Git. Changes compared to v1:
> >
> >   - t4153 and t7006 aren't marked as passing anymore. I thought they
> >     pass because most of these tests were skipped because of a missing
> >     TTY prerequisite both on my local machine, but also in our CI.
> >
> >   - Add another patch to install the Perl IO:Pty module on Alpine and
> >     Ubuntu. This fulfills the TTY prerequisite and thus surfaces the
> >     memory leaks in both of the above tests.
> >
> >   - Add another unit test for strvec that exercise replacing a string in
> >     the strvec with a copy of itself.
> >
> >   - A bunch of commit message improvements.
> 
> Looking very good.  This seems to reveal existing leaks when merged
> to 'seen'; other topics that are not in 'master' may be introducing
> these leaks.  I'll see if a trial merge to 'next' is leak-free (in
> which case I'll merge it down to 'next') or there are other topics
> in 'next' that are leaking (in which case we'll play by ear---either
> mark the tests again as non-leak-free, or plug the leak if it seems
> trivial).
> 
>  https://github.com/git/git/actions/runs/9231313414/job/25400998823
> 
> says t1400-update-ref has many "stdin symref-update" things are
> failing.

Indeed. The following diff fixes the leak:

    diff --git a/builtin/update-ref.c b/builtin/update-ref.c
    index 7d2a419230..e54be9c429 100644
    --- a/builtin/update-ref.c
    +++ b/builtin/update-ref.c
    @@ -130,6 +130,8 @@ static char *parse_next_arg(const char **next)
     
        if (arg.len)
            return strbuf_detach(&arg, NULL);
    +
    +	strbuf_release(&arg);
        return NULL;
     }
     

Karthik is out of office this week, so you may want to add this as a
"SQUASH???" commit on top of his topic branch to make "seen" pass.

> Also
> 
>  https://github.com/git/git/actions/runs/9231313414/job/25401102951
> 
> shows that t1460-refs-migrate fails on Windows.

Hm, this one is curious. There are no leak logs at all, and the exit
code is 139. Might be SIGSEGV, indicating that something else is going
on here than a memory leak.

I'll investigate.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]