From 6361dea6e8f4476b495319a9d3eeed2e2e694f8a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 30 Sep 2024 11:13:35 +0200 Subject: [PATCH] builtin/clone: fix leaking repo state when cloning with bundle URIs When cloning with bundle URIs we re-initialize `the_repository` after having fetched the bundle. This causes a bunch of memory leaks though because we do not release its previous state. These leaks can be plugged by calling `repo_clear()` before we call `repo_init()`. But this causes another issue because the remote that we used is tied to the lifetime of the repository's remote state, which would also get released. We thus have to make sure that it does not get free'd under our feet. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/clone.c | 27 ++++++++++++++++++++++++++ t/t5730-protocol-v2-bundle-uri-file.sh | 1 + t/t5731-protocol-v2-bundle-uri-git.sh | 1 + t/t5732-protocol-v2-bundle-uri-http.sh | 1 + 4 files changed, 30 insertions(+) diff --git a/builtin/clone.c b/builtin/clone.c index e77339c8472..59fcb317a68 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -1403,8 +1403,17 @@ int cmd_clone(int argc, * data from the --bundle-uri option. */ if (bundle_uri) { + struct remote_state *state; int has_heuristic = 0; + /* + * We need to save the remote state as our remote's lifetime is + * tied to it. + */ + state = the_repository->remote_state; + the_repository->remote_state = NULL; + repo_clear(the_repository); + /* At this point, we need the_repository to match the cloned repo. */ if (repo_init(the_repository, git_dir, work_tree)) warning(_("failed to initialize the repo, skipping bundle URI")); @@ -1413,6 +1422,10 @@ int cmd_clone(int argc, bundle_uri); else if (has_heuristic) git_config_set_gently("fetch.bundleuri", bundle_uri); + + remote_state_clear(the_repository->remote_state); + free(the_repository->remote_state); + the_repository->remote_state = state; } else { /* * Populate transport->got_remote_bundle_uri and @@ -1422,12 +1435,26 @@ int cmd_clone(int argc, if (transport->bundles && hashmap_get_size(&transport->bundles->bundles)) { + struct remote_state *state; + + /* + * We need to save the remote state as our remote's + * lifetime is tied to it. + */ + state = the_repository->remote_state; + the_repository->remote_state = NULL; + repo_clear(the_repository); + /* At this point, we need the_repository to match the cloned repo. */ if (repo_init(the_repository, git_dir, work_tree)) warning(_("failed to initialize the repo, skipping bundle URI")); else if (fetch_bundle_list(the_repository, transport->bundles)) warning(_("failed to fetch advertised bundles")); + + remote_state_clear(the_repository->remote_state); + free(the_repository->remote_state); + the_repository->remote_state = state; } else { clear_bundle_list(transport->bundles); FREE_AND_NULL(transport->bundles); diff --git a/t/t5730-protocol-v2-bundle-uri-file.sh b/t/t5730-protocol-v2-bundle-uri-file.sh index 37bdb725bca..38396df95b1 100755 --- a/t/t5730-protocol-v2-bundle-uri-file.sh +++ b/t/t5730-protocol-v2-bundle-uri-file.sh @@ -7,6 +7,7 @@ TEST_NO_CREATE_REPO=1 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh # Test protocol v2 with 'file://' transport diff --git a/t/t5731-protocol-v2-bundle-uri-git.sh b/t/t5731-protocol-v2-bundle-uri-git.sh index 8add1b37abc..c199e955fec 100755 --- a/t/t5731-protocol-v2-bundle-uri-git.sh +++ b/t/t5731-protocol-v2-bundle-uri-git.sh @@ -7,6 +7,7 @@ TEST_NO_CREATE_REPO=1 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh # Test protocol v2 with 'git://' transport diff --git a/t/t5732-protocol-v2-bundle-uri-http.sh b/t/t5732-protocol-v2-bundle-uri-http.sh index 129daa02269..a9403e94c6d 100755 --- a/t/t5732-protocol-v2-bundle-uri-http.sh +++ b/t/t5732-protocol-v2-bundle-uri-http.sh @@ -7,6 +7,7 @@ TEST_NO_CREATE_REPO=1 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh # Test protocol v2 with 'http://' transport