From: Mark Jamsek <mark@jamsek.com>
Subject: Re: Got Status Memory Leak pt. 2
To: Kyle Ackerman <kackerman0102@gmail.com>
Cc: gameoftrees@openbsd.org
Date: Tue, 05 Dec 2023 17:32:05 +1100

Kyle Ackerman <kackerman0102@gmail.com> wrote:
> Hi all,
> 
> Same methodology as the previous thread.  Here is the kdump from `got status`.
> 
> ******** Start dump got *******
> M=8 I=1 F=0 U=0 J=1 R=0 X=0 C=0 cache=64 G=0
> Leak report:
>                  f     sum      #    avg
>      0x88dc57d64f0      64      1     64 addr2line -e /usr/lib/libc.so.97.1 0xcf4f0
>      0x88dc5762621     112      7     16 addr2line -e /usr/lib/libc.so.97.1 0x5b621
>      0x88dc5757623   65536      1  65536 addr2line -e /usr/lib/libc.so.97.1 0x50623
>      0x88b2ac03eff   16800    350     48 addr2line -e /home/kyle/bin/got 0x5beff
>      0x88b2ac38d68      48      3     16 addr2line -e /home/kyle/bin/got 0x90d68
> 
> ******** End dump got *******
> 
> The patch below will clean up the missing_children TAILQ in
> worktree_status().  I was hesitant to use GOT_PATHLIST_FREE_NONE but it
> looks like find_missing_children() sets the data field to NULL and the
> path field is assigned to one of the fileindex->entries.  In got_worktree_status,
> the fileindex is cleaned up with "got_fileindex_free(fileindex)".  Using
> GOT_PATHLIST_FREE_PATH above results in a double free.  We could use
> GOT_PATHLIST_FREE_DATA but I ~think~ that is redundant as data is set to
> NULL (stated above). All regression tests have passed on my end. 

Yes, I agree with your analysis. Thanks, Kyle! I've just committed this.


> diff /home/kyle/src/got-dev
> commit - d0b9836b2ff0ddbe37b3dea6be10afdb14b65175
> path + /home/kyle/src/got-dev
> blob - c419422fe5a36542e706bb231e2eddf6e3e0fd63
> file + lib/worktree.c
> --- lib/worktree.c
> +++ lib/worktree.c
> @@ -4211,6 +4211,7 @@ worktree_status(struct got_worktree *worktree, const c
>  	}
>  done:
>  	free_ignores(&ignores);
> +	got_pathlist_free(&missing_children, GOT_PATHLIST_FREE_NONE);
>  	if (fd != -1 && close(fd) == -1 && err == NULL)
>  		err = got_error_from_errno("close");
>  	free(ondisk_path);
> 
> 
> Here is the kdump after the patch is applied.
> 
> ******** Start dump got *******
> M=8 I=1 F=0 U=0 J=1 R=0 X=0 C=0 cache=64 G=0
> Leak report:
>                  f     sum      #    avg
>      0xa4c8fe454f0      64      1     64 addr2line -e /usr/lib/libc.so.97.1 0xcf4f0
>      0xa4c8fdd1621     112      7     16 addr2line -e /usr/lib/libc.so.97.1 0x5b621
>      0xa4c8fdc6623   65536      1  65536 addr2line -e /usr/lib/libc.so.97.1 0x50623
>      0xa4a4cc14d88      48      3     16 addr2line -e /home/kyle/bin/got 0x90d88
> 
> ******** End dump got *******
> 
> Comments/ok?


-- 
Mark Jamsek <https://bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68