Skip to content

Commit 134bfed

Browse files
peffgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent cf8072e commit 134bfed

File tree

2 files changed

+11
-0
lines changed

2 files changed

+11
-0
lines changed

http-walker.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,8 +579,18 @@ static void cleanup(struct walker *walker)
579579
if (data) {
580580
alt = data->alt;
581581
while (alt) {
582+
struct packed_git *pack;
583+
582584
alt_next = alt->next;
583585

586+
pack = alt->packs;
587+
while (pack) {
588+
struct packed_git *pack_next = pack->next;
589+
close_pack(pack);
590+
free(pack);
591+
pack = pack_next;
592+
}
593+
584594
free(alt->base);
585595
free(alt);
586596

t/t5550-http-fetch-dumb.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ test_description='test dumb fetching over http via static file'
44
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
55
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
66

7+
TEST_PASSES_SANITIZE_LEAK=true
78
. ./test-lib.sh
89

910
if test_have_prereq !REFFILES

0 commit comments

Comments
 (0)