"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Mark Jamsek <mark@jamsek.com>
Subject:
Re: Got Status Memory Leak pt. 3
To:
Kyle Ackerman <kackerman0102@gmail.com>
Cc:
gameoftrees@openbsd.org
Date:
Fri, 08 Dec 2023 17:25:12 +1100

Download raw body.

Thread
Kyle Ackerman <kackerman0102@gmail.com> wrote:
> Here is the grand finale of this series. 
> 
> Here is the kdump from running `got status`
> 
> ******** Start dump got *******
> M=8 I=1 F=1 U=1 J=2 R=0 X=0 C=0xfe64220e cache=0 G=4096
> Leak report:
>                  f     sum      #    avg
>       0x17999bcc60      64      1     64 addr2line -e /usr/lib/libc.so.97.1 0x49c60
>       0x17999e3da1     112      7     16 addr2line -e /usr/lib/libc.so.97.1 0x70da1
>       0x1799a14b53   69632      1  69632 addr2line -e /usr/lib/libc.so.97.1 0xa1b53
>       0x14f1abad78      32      2     16 addr2line -e /home/kyle/bin/got 0x90d78
> 
> ******** End dump got *******
> 
> The got_pathlist_head never gets cleaned up in free_ignores().  The patch
> below will free it.  While freeing the ignorelist in the TAILQ_FOREACH
> loop, the head of the list doesn't get freed.  The puzzling part to me
> is that got_pathlist_free(ignores, GOT_PATHLIST_FREE_ALL) does not
> plug the leak, seems to reduce it though.  But freeing it after the
> ignorelist seems to work.  This patch has passed all regressions on my end.

Thanks, Kyle!

I see what you mean. This is because the GOT_PATHLIST_FREE_ALL flag
needs to be set on the second got_pathlist_free(ignores, ...) call in
free_ignores() as per the below diff, as it's the data pointer in the
ignores pathlist queue (i.e., pe->data in the loop) that is being leaked
and not the data pointer of the entries in the ignorelist queue.

It's the same result either way so whichever diff is ok for me!

diff /home/mark/src/got
commit - 6ecb0b8c6b2aa36b6af31c856909b1ddccdb301c
path + /home/mark/src/got
blob - 4415674d74999e7b6fef5dd651fa76682cc19ca1
file + lib/worktree.c
--- lib/worktree.c
+++ lib/worktree.c
@@ -3722,7 +3722,7 @@ free_ignores(struct got_pathlist_head *ignores)
 
 		got_pathlist_free(ignorelist, GOT_PATHLIST_FREE_PATH);
 	}
-	got_pathlist_free(ignores, GOT_PATHLIST_FREE_PATH);
+	got_pathlist_free(ignores, GOT_PATHLIST_FREE_ALL);
 }
 
 static const struct got_error *
@@ -3777,6 +3777,7 @@ done:
 	if (err || pe == NULL) {
 		free(dirpath);
 		got_pathlist_free(ignorelist, GOT_PATHLIST_FREE_PATH);
+		free(ignorelist);
 	}
 	return err;
 }


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