http-walker: free fake packed_git list

The dumb-http walker code creates a "fake" packed_git list representing
packs we've downloaded from the remote (I call it "fake" because
generally that struct is only used and managed by the local repository
struct). But during our cleanup phase we don't touch those at all,
causing a leak.

There's no support here from the rest of the object-database API, as
these structs are not meant to be freed, except when closing the object
store completely. But we can see that raw_object_store_clear() just
calls free() on them, and that's enough here to fix the leak.

I also added a call to close_pack() before each. In the regular code
this happens via close_object_store(), which we do as part of
raw_object_store_clear(). This is necessary to prevent leaking mmap'd
data (like the pack idx) or descriptors. The leak-checker won't catch
either of these itself, but I did confirm with some hacky warning()
calls and running t5550 that it's easy to leak at least index data.

This is all much more intimate with the packed_git struct than I'd like,
but I think fixing it would be a pretty big refactor. And it's just not
worth it for dumb-http code which is rarely used these days. If we can
silence the leak-checker without creating too much hassle, we should
just do that.

This lets us mark t5550 as leak-free.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/http-walker.c b/http-walker.c
index 9c1e5c37e6e299904304dd29ce4bb6f9c5a5cf3e..fb2d86d5e70f0e3b7a12ac74d910e032dff9a3a7 100644 (file)
--- a/http-walker.c
+++ b/http-walker.c
@@ -579,8 +579,18 @@ static void cleanup(struct walker *walker)
        if (data) {
                alt = data->alt;
                while (alt) {
+                       struct packed_git *pack;
+
                        alt_next = alt->next;
 
+                       pack = alt->packs;
+                       while (pack) {
+                               struct packed_git *pack_next = pack->next;
+                               close_pack(pack);
+                               free(pack);
+                               pack = pack_next;
+                       }
+
                        free(alt->base);
                        free(alt);
 

diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index ea8e48f627315f115a2dbcaf08b47f704c755eaa..58189c9f7dc9bd8dc715ea903013ea8018dad360 100755 (executable)
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -4,6 +4,7 @@ test_description='test dumb fetching over http via static file'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 if test_have_prereq !REFFILES