git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: "Eric Sunshine" <sunshine@sunshineco.com>,
	"Jeppe Øland" <joland@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>
Subject: [PATCH v2 8/8] object: fix leaking packfiles when closing object store
Date: Thu, 8 Aug 2024 09:36:00 +0200	[thread overview]
Message-ID: <d05737c75f0e027e2134ec5ace1899aff2bc97ab.1723102259.git.ps@pks.im> (raw)
In-Reply-To: <cover.1723102259.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 1983 bytes --]

When calling `raw_object_store_clear()`, we close and free several
resources associated with the object store. Part of that is to close and
free all the packfiles, which is handled by `close_object_store()`.

That function really only ends up closing the packfiles though, but it
doesn't free them. And in fact it can't, as that function is being
called via `run_command()` when `close_object_store = 1`, which is done
e.g. when we execute git-maintenance(1). At that point, other structures
may still have references on those packfiles, and thus we cannot free
them here. So while it is in fact intentional that we really only close
them, the result is a memory leak because `raw_object_store_clear()`
does not free them, either.

Fix the leak by freeing the packfiles in `raw_object_store_clear()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 object.c                               | 9 +++++++++
 t/t7424-submodule-mixed-ref-formats.sh | 1 +
 2 files changed, 10 insertions(+)

diff --git a/object.c b/object.c
index 0c0fcb76c0..d756c7f2ea 100644
--- a/object.c
+++ b/object.c
@@ -614,6 +614,15 @@ void raw_object_store_clear(struct raw_object_store *o)
 
 	INIT_LIST_HEAD(&o->packed_git_mru);
 	close_object_store(o);
+
+	/*
+	 * `close_object_store()` only closes the packfiles, but doesn't free
+	 * them. We thus have to do this manually.
+	 */
+	for (struct packed_git *p = o->packed_git, *next; p; p = next) {
+		next = p->next;
+		free(p);
+	}
 	o->packed_git = NULL;
 
 	hashmap_clear(&o->pack_map);
diff --git a/t/t7424-submodule-mixed-ref-formats.sh b/t/t7424-submodule-mixed-ref-formats.sh
index 559713b607..b43ef2ba67 100755
--- a/t/t7424-submodule-mixed-ref-formats.sh
+++ b/t/t7424-submodule-mixed-ref-formats.sh
@@ -2,6 +2,7 @@
 
 test_description='submodules handle mixed ref storage formats'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_ref_format () {
-- 
2.46.0.46.g406f326d27.dirty


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  parent reply	other threads:[~2024-08-08  7:36 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-05 11:31 2.46 submodule breakage Jeppe Øland
2024-08-06 13:18 ` Jeppe Øland
2024-08-06 18:26   ` Eric Sunshine
2024-08-07  6:40     ` Patrick Steinhardt
2024-08-07  7:38       ` Patrick Steinhardt
2024-08-07 16:09         ` Junio C Hamano
2024-08-07 12:43 ` [PATCH 0/6] Improvements for ref storage formats with submodules Patrick Steinhardt
2024-08-07 12:43   ` [PATCH 1/6] builtin/submodule: allow cloning with different ref storage format Patrick Steinhardt
2024-08-07 14:45     ` [PATCH 0/6] Improvements for ref storage formats with submodules Jeppe Øland
2024-08-07 22:55     ` [PATCH 1/6] builtin/submodule: allow cloning with different ref storage format Junio C Hamano
2024-08-08  7:00       ` Patrick Steinhardt
2024-08-08 16:08         ` Junio C Hamano
2024-08-08 16:19           ` Patrick Steinhardt
2024-08-08 17:26             ` Junio C Hamano
2024-08-07 12:43   ` [PATCH 2/6] builtin/clone: propagate ref storage format to submodules Patrick Steinhardt
2024-08-07 23:07     ` Junio C Hamano
2024-08-07 12:43   ` [PATCH 3/6] refs: fix ref storage format for submodule ref stores Patrick Steinhardt
2024-08-07 12:44   ` [PATCH 4/6] submodule: fix leaking fetch tasks Patrick Steinhardt
2024-08-07 12:44   ` [PATCH 5/6] submodule: fix leaking seen submodule names Patrick Steinhardt
2024-08-07 12:44   ` [PATCH 6/6] object: fix leaking packfiles when closing object store Patrick Steinhardt
2024-08-07 13:18   ` [PATCH 0/6] Improvements for ref storage formats with submodules Patrick Steinhardt
2024-08-08  1:09   ` Junio C Hamano
2024-08-08  7:00     ` Patrick Steinhardt
2024-08-08  7:35 ` [PATCH v2 0/8] " Patrick Steinhardt
2024-08-08  7:35   ` [PATCH v2 1/8] git-submodule.sh: break overly long command lines Patrick Steinhardt
2024-08-08  7:35   ` [PATCH v2 2/8] builtin/submodule: allow cloning with different ref storage format Patrick Steinhardt
2024-08-08  7:35   ` [PATCH v2 3/8] builtin/clone: propagate ref storage format to submodules Patrick Steinhardt
2024-08-08  8:03     ` Kristoffer Haugsbakk
2024-08-08 13:29       ` Patrick Steinhardt
2024-08-08  7:35   ` [PATCH v2 4/8] refs: fix ref storage format for submodule ref stores Patrick Steinhardt
2024-08-08  7:35   ` [PATCH v2 5/8] builtin/submodule: allow "add" to use different ref storage format Patrick Steinhardt
2024-08-08  7:35   ` [PATCH v2 6/8] submodule: fix leaking fetch tasks Patrick Steinhardt
2024-08-08  7:35   ` [PATCH v2 7/8] submodule: fix leaking seen submodule names Patrick Steinhardt
2024-08-08  7:36   ` Patrick Steinhardt [this message]
2024-08-08 16:24   ` [PATCH v2 0/8] Improvements for ref storage formats with submodules Junio C Hamano

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=d05737c75f0e027e2134ec5ace1899aff2bc97ab.1723102259.git.ps@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=joland@gmail.com \
    --cc=sunshine@sunshineco.com \
    /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).