From: Jeff King <peff@peff.net>
Date: Thu, 05 Mar 2026 22:02:14 GMT
Subject: Re: memory leak when cloning a repository
Message-ID: <20260305220214.GB736322@coredump.intra.peff.net>
In-Reply-To: <b9fa930e-7d5e-47f1-8896-1997cf7c0cdb@intel.com>

On Thu, Mar 05, 2026 at 12:51:17PM -0800, Jacob Keller wrote:

> I tried digging into why this leak occurs but so far I don't have a good idea.
> 
> This happens when running on next: 7842e34a6654 ("Sync with 'master'")

I can reproduce it on master. This seems to fix it:

diff --git a/connected.c b/connected.c
index 79403108dd..e0f8ff38cb 100644
--- a/connected.c
+++ b/connected.c
@@ -90,6 +90,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
 promisor_pack_found:
 			;
 		} while ((oid = fn(cb_data)) != NULL);
+		close_pack(new_pack);
 		free(new_pack);
 		return 0;
 	}
@@ -128,6 +129,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
 		rev_list.no_stderr = opt->quiet;
 
 	if (start_command(&rev_list)) {
+		close_pack(new_pack);
 		free(new_pack);
 		return error(_("Could not run 'git rev-list'"));
 	}
@@ -162,6 +164,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
 		err = error_errno(_("failed to close rev-list's stdin"));
 
 	sigchain_pop(SIGPIPE);
+	close_pack(new_pack);
 	free(new_pack);
 	return finish_command(&rev_list) || err;
 }


I think this has been leaky forever, but it's usually leaking a single
mmap, so nobody notices. But I noticed something odd about your trace:

> Direct leak of 27168 byte(s) in 1 object(s) allocated from:
>     #0 0x7f0e100e6f2b in malloc (/lib64/libasan.so.8+0xe6f2b) (BuildId: 25975f766867e9e604dc5a71a8befeaed3301942)
>     #1 0x00000122ab77 in git_mmap ../compat/mmap.c:15
>     #2 0x000001169466 in xmmap_gently ../wrapper.c:884
>     #3 0x00000116959b in xmmap ../wrapper.c:907
>     #4 0x000000d168fd in check_packed_git_idx ../packfile.c:179
>     #5 0x000000d16cce in open_pack_index ../packfile.c:282
>     #6 0x000000d25273 in find_pack_entry_one ../packfile.c:2078
>     #7 0x00000099f969 in check_connected ../connected.c:148

We're in the compat git_mmap, which implies you're building with
NO_MMAP. We turn that on automatically when building with ASan (so that
we can detect single-byte overflows even when mmap would round up to a
page boundary). But as a side effect, the "mmap" for index and pack data
is done with a heap-allocated buffer. So now ASan/LSan will notice and
complain about it.

We usually disable leak-checking for our ASan builds, so we wouldn't run
the tests with the compat mmap. And our leak-checking builds use LSan,
which doesn't set NO_MMAP. But if you combine them with:

  make SANITIZE=address,leak

or even just build with:

  make NO_MMAP=MallocHarder SANITIZE=leak

then the leak will be reported. I guess maybe you're building with
SANITIZE=address, but then running the result independently, without
setting ASAN_OPTIONS=detect_leaks=0.

Anyway, I think the solution is probably something like the patch above,
though probably it needs to cover the case where new_pack is NULL.

-Peff