Junio C HamanoWhen pushing to a set of remotes using a nickname for the group, the client initializes the connection to each remote, talks to the remote and reads and parses capabilities line, and holds the capabilities in a file-scope static variable server_capabilities_v1.
There are a few other such file-scope static variables, and these connections cannot be parallelized until they are refactored to a structure that keeps track of active connections.
Which is *not* the theme of this patch ;-)
For a single connection, the server_capabilities_v1 variable is initialized to NULL (at the program initialization), populated when we talk to the other side, used to look up capabilities of the other sdie possible multiple times, and the memory is held by the variable until program exit, without leaking. When talking to multiple remotes, however, the server capabilities from the second connection overwrites without freeing the one from the first connection, which leaks.
==1080970==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 421 byte(s) in 2 object(s) allocated from: #0 0x5615305f849e in strdup (/home/gitster/g/git-jch/bin/bin/git+0x2b349e) (BuildId: 54d149994c9e85374831958f694bd0aa3b8b1e26) #1 0x561530e76cc4 in xstrdup /home/gitster/w/build/wrapper.c:43:14 #2 0x5615309cd7fa in process_capabilities /home/gitster/w/build/connect.c:243:27 #3 0x5615309cd502 in get_remote_heads /home/gitster/w/build/connect.c:366:4 #4 0x561530e2cb0b in handshake /home/gitster/w/build/transport.c:372:3 #5 0x561530e29ed7 in get_refs_via_connect /home/gitster/w/build/transport.c:398:9 #6 0x561530e26464 in transport_push /home/gitster/w/build/transport.c:1421:16 #7 0x561530800bec in push_with_options /home/gitster/w/build/builtin/push.c:387:8 #8 0x5615307ffb99 in do_push /home/gitster/w/build/builtin/push.c:442:7 #9 0x5615307fe926 in cmd_push /home/gitster/w/build/builtin/push.c:664:7 #10 0x56153065673f in run_builtin /home/gitster/w/build/git.c:506:11 #11 0x56153065342f in handle_builtin /home/gitster/w/build/git.c:779:9 #12 0x561530655b89 in run_argv /home/gitster/w/build/git.c:862:4 #13 0x561530652cba in cmd_main /home/gitster/w/build/git.c:984:19 #14 0x5615308dda0a in main /home/gitster/w/build/common-main.c:9:11 #15 0x7f051651bca7 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
SUMMARY: AddressSanitizer: 421 byte(s) leaked in 2 allocation(s).
Free the capablities data for the previous server before overwriting it with the next server to plug this leak.
The added test fails without the freeing with SANITIZE=leak; I somehow couldn't get it fail reliably with SANITIZE=leak,address though.
Signed-off-by: Junio C Hamano <gitster@pobox.com> --- connect.c | 2 ++ t/meson.build | 1 + t/t5565-push-multiple.sh | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+) create mode 100755 t/t5565-push-multiple.sh
Show changes to 3 files +42 −0
connect.c, t/meson.build, t/t5565-push-multiple.sh
diff --git a/connect.c b/connect.c index 8352b71faf..c6f76e3082 100644 --- a/connect.c +++ b/connect.c @@ -240,6 +240,8 @@ static void process_capabilities(struct packet_reader *reader, size_t *linelen) size_t nul_location = strlen(line); if (nul_location == *linelen) return; + + free(server_capabilities_v1); server_capabilities_v1 = xstrdup(line + nul_location + 1); *linelen = nul_location; diff --git a/t/meson.build b/t/meson.build index d3d0be2822..459c52a489 100644 --- a/t/meson.build +++ b/t/meson.build @@ -690,6 +690,7 @@ integration_tests = [ 't5562-http-backend-content-length.sh', 't5563-simple-http-auth.sh', 't5564-http-proxy.sh', + 't5565-push-multiple.sh', 't5570-git-daemon.sh', 't5571-pre-push-hook.sh', 't5572-pull-submodule.sh', diff --git a/t/t5565-push-multiple.sh b/t/t5565-push-multiple.sh new file mode 100755 index 0000000000..7e93668566 --- /dev/null +++ b/t/t5565-push-multiple.sh @@ -0,0 +1,39 @@ +#!/bin/sh + +test_description='push to group' + +. ./test-lib.sh + +test_expect_success setup ' + for i in 1 2 3 + do + git init dest-$i && + git -C dest-$i symbolic-ref HEAD refs/heads/not-a-branch || + return 1 + done && + test_tick && + git commit --allow-empty -m "initial" && + git config set --append remote.them.pushurl "file://$(pwd)/dest-1" && + git config set --append remote.them.pushurl "file://$(pwd)/dest-2" && + git config set --append remote.them.pushurl "file://$(pwd)/dest-3" && + git config set --append remote.them.push "+refs/heads/*:refs/heads/*" +' + +test_expect_success 'push to group' ' + git push them && + j= && + for i in 1 2 3 + do + git -C dest-$i for-each-ref >actual-$i && + if test -n "$j" + then + test_cmp actual-$j actual-$i + else + cat actual-$i + fi && + j=$i || + return 1 + done +' + +test_done
-- 2.52.0-313-gc4a767987d
