]> git.ipfire.org Git - thirdparty/git.git/commitdiff
builtin/clone: fix leaking repo state when cloning with bundle URIs
authorPatrick Steinhardt <ps@pks.im>
Mon, 30 Sep 2024 09:13:35 +0000 (11:13 +0200)
committerJunio C Hamano <gitster@pobox.com>
Mon, 30 Sep 2024 18:23:04 +0000 (11:23 -0700)
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 <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/clone.c
t/t5730-protocol-v2-bundle-uri-file.sh
t/t5731-protocol-v2-bundle-uri-git.sh
t/t5732-protocol-v2-bundle-uri-http.sh

index e77339c84720f244f7789cef55f1cbd97c6ab9c8..59fcb317a68a77eee3ca96a60720c556e044c369 100644 (file)
@@ -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);
index 37bdb725bcac2a8e3f8d00cd816bbec456f6ee26..38396df95b1fa487f1b3d642eda4c31347461043 100755 (executable)
@@ -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
index 8add1b37abc9850c667737a7588987ea752c13dd..c199e955fecd9cf7c93e7bfc2751b6b759d5526e 100755 (executable)
@@ -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
index 129daa02269ed409f26fd15fe416ef06cd114541..a9403e94c6d35039ea0243dc8a492b1b32035395 100755 (executable)
@@ -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