From: Mark Jamsek Subject: Re: Got Status Memory Leak pt. 3 To: Kyle Ackerman Cc: gameoftrees@openbsd.org Date: Fri, 08 Dec 2023 17:25:12 +1100 Kyle Ackerman 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 GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68