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 <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Sep 30, 2024 at 11:13 UTC
6361dea6e8f4476b495319a9d3eeed2e2e694f8a
4 files changed
+30
builtin/clone.c
+27
@@ -1403,8 +1403,17 @@ int cmd_clone(int argc,
1403
* data from the --bundle-uri option.
1404
*/
1405
if (bundle_uri) {
1406
+ struct remote_state *state;
1407
int has_heuristic = 0;
1408
1409
+ /*
1410
+ * We need to save the remote state as our remote's lifetime is
1411
+ * tied to it.
1412
+ */
1413
+ state = the_repository->remote_state;
1414
+ the_repository->remote_state = NULL;
1415
+ repo_clear(the_repository);
1416
+
1417
/* At this point, we need the_repository to match the cloned repo. */
1418
if (repo_init(the_repository, git_dir, work_tree))
1419
warning(_("failed to initialize the repo, skipping bundle URI"));
@@ -1413,6 +1422,10 @@ int cmd_clone(int argc,
1422
bundle_uri);
1423
else if (has_heuristic)
1424
git_config_set_gently("fetch.bundleuri", bundle_uri);
1425
+
1426
+ remote_state_clear(the_repository->remote_state);
1427
+ free(the_repository->remote_state);
1428
+ the_repository->remote_state = state;
1429
} else {
1430
/*
1431
* Populate transport->got_remote_bundle_uri and
@@ -1422,12 +1435,26 @@ int cmd_clone(int argc,
1435
1436
if (transport->bundles &&
1437
hashmap_get_size(&transport->bundles->bundles)) {
1438
+ struct remote_state *state;
1439
+
1440
+ /*
1441
+ * We need to save the remote state as our remote's
1442
+ * lifetime is tied to it.
1443
+ */
1444
+ state = the_repository->remote_state;
1445
+ the_repository->remote_state = NULL;
1446
+ repo_clear(the_repository);
1447
+
1448
/* At this point, we need the_repository to match the cloned repo. */
1449
if (repo_init(the_repository, git_dir, work_tree))
1450
warning(_("failed to initialize the repo, skipping bundle URI"));
1451
else if (fetch_bundle_list(the_repository,
1452
transport->bundles))
1453
warning(_("failed to fetch advertised bundles"));
1454
+
1455
+ remote_state_clear(the_repository->remote_state);
1456
+ free(the_repository->remote_state);
1457
+ the_repository->remote_state = state;
1458
} else {
1459
clear_bundle_list(transport->bundles);
1460
FREE_AND_NULL(transport->bundles);
t/t5730-protocol-v2-bundle-uri-file.sh
+1
@@ -7,6 +7,7 @@ TEST_NO_CREATE_REPO=1
7
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
8
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
9
10
+TEST_PASSES_SANITIZE_LEAK=true
11
. ./test-lib.sh
12
13
# Test protocol v2 with 'file://' transport
t/t5731-protocol-v2-bundle-uri-git.sh
+1
@@ -7,6 +7,7 @@ TEST_NO_CREATE_REPO=1
7
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
8
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
9
10
+TEST_PASSES_SANITIZE_LEAK=true
11
. ./test-lib.sh
12
13
# Test protocol v2 with 'git://' transport
t/t5732-protocol-v2-bundle-uri-http.sh
+1
@@ -7,6 +7,7 @@ TEST_NO_CREATE_REPO=1
7
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
8
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
9
10
+TEST_PASSES_SANITIZE_LEAK=true
11
. ./test-lib.sh
12
13
# Test protocol v2 with 'http://' transport