From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Subject: [PATCH 09/23] builtin/clone: fix leaking repo state when cloning with bundle URIs
Date: Mon, 30 Sep 2024 11:13:35 +0200 [thread overview]
Message-ID: <2fa76a00fc0469d0c64c66e81b96d622844b8f0d.1727687410.git.ps@pks.im> (raw)
In-Reply-To: <cover.1727687410.git.ps@pks.im>
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>
---
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 e77339c847..59fcb317a6 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 37bdb725bc..38396df95b 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 8add1b37ab..c199e955fe 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 129daa0226..a9403e94c6 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
--
2.46.2.852.g229c0bf0e5.dirty
next prev parent reply other threads:[~2024-09-30 9:13 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-30 9:13 [PATCH 00/23] Memory leak fixes (pt.8) Patrick Steinhardt
2024-09-30 9:13 ` [PATCH 01/23] builtin/annotate: fix leaking args vector Patrick Steinhardt
2024-09-30 9:13 ` [PATCH 02/23] read-cache: fix leaking hash context in `do_write_index()` Patrick Steinhardt
2024-09-30 14:05 ` Derrick Stolee
2024-09-30 9:13 ` [PATCH 03/23] scalar: fix leaking repositories Patrick Steinhardt
2024-09-30 9:13 ` [PATCH 04/23] shell: fix leaking strings Patrick Steinhardt
2024-09-30 9:13 ` [PATCH 05/23] wt-status: fix leaking buffer with sparse directories Patrick Steinhardt
2024-09-30 9:13 ` [PATCH 06/23] submodule: fix leaking submodule entry list Patrick Steinhardt
2024-09-30 9:13 ` [PATCH 07/23] builtin/stash: fix leaking `pathspec_from_file` Patrick Steinhardt
2024-09-30 9:13 ` [PATCH 08/23] builtin/pack-redundant: fix various memory leaks Patrick Steinhardt
2024-09-30 9:13 ` Patrick Steinhardt [this message]
2024-09-30 9:13 ` [PATCH 10/23] t/helper: fix leaking repository in partial-clone helper Patrick Steinhardt
2024-09-30 9:13 ` [PATCH 11/23] builtin/revert: fix leaking `gpg_sign` and `strategy` config Patrick Steinhardt
2024-09-30 9:13 ` [PATCH 12/23] diff: improve lifecycle management of diff queues Patrick Steinhardt
2024-09-30 9:13 ` [PATCH 13/23] line-log: fix several memory leaks Patrick Steinhardt
2024-09-30 9:13 ` [PATCH 14/23] pseudo-merge: fix various " Patrick Steinhardt
2024-09-30 16:21 ` Taylor Blau
2024-09-30 9:13 ` [PATCH 15/23] pseudo-merge: fix leaking strmap keys Patrick Steinhardt
2024-09-30 21:22 ` Taylor Blau
2024-10-07 9:41 ` Patrick Steinhardt
2024-10-08 8:54 ` Jeff King
2024-10-09 7:06 ` Patrick Steinhardt
2024-09-30 9:13 ` [PATCH 16/23] pack-bitmap-write: fix leaking OID array Patrick Steinhardt
2024-09-30 21:22 ` Taylor Blau
2024-09-30 9:14 ` [PATCH 17/23] midx-write: fix leaking buffer Patrick Steinhardt
2024-09-30 21:27 ` Taylor Blau
2024-10-07 9:41 ` Patrick Steinhardt
2024-09-30 9:14 ` [PATCH 18/23] revision: fix memory leaks when rewriting parents Patrick Steinhardt
2024-09-30 9:14 ` [PATCH 19/23] revision: fix leaking saved parents Patrick Steinhardt
2024-09-30 9:14 ` [PATCH 20/23] pack-write: fix return parameter of `write_rev_file_order()` Patrick Steinhardt
2024-09-30 9:14 ` [PATCH 21/23] t/helper: fix leaks in proc-receive helper Patrick Steinhardt
2024-09-30 9:14 ` [PATCH 22/23] remote: fix leaking push reports Patrick Steinhardt
2024-09-30 9:14 ` [PATCH 23/23] builtin/send-pack: fix leaking list of push options Patrick Steinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2fa76a00fc0469d0c64c66e81b96d622844b8f0d.1727687410.git.ps@pks.im \
--to=ps@pks.im \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).