git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 6/6] object: fix leaking packfiles when closing object store

From
Patrick Steinhardt <ps@pks.im>
Date
Aug 7, 2024, 12:44 UTC
Message-ID
<8dc7cc76d588ffe143d1d35e78e6eb86ed6d0b20.1723032100.git.ps@pks.im>
In-Reply-To
<cover.1723032100.git.ps@pks.im>

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 998a5d82e9..812496dc3b 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.dirty
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 20 of 35 in “2.46 submodule breakage”
  1. Jeppe ØlandAug 5, 2024
  2. Jeppe ØlandAug 6, 2024
  3. Eric SunshineAug 6, 2024
  4. Patrick SteinhardtAug 7, 2024
  5. Patrick SteinhardtAug 7, 2024
  6. Junio C HamanoAug 7, 2024
  7. 0/6 Improvements for ref storage formats with submodulesPatrick Steinhardt, Aug 7, 2024
  8. 1/6 builtin/submodule: allow cloning with different ref storage formatPatrick Steinhardt, Aug 7, 2024
  9. Jeppe ØlandAug 7, 2024
  10. Junio C HamanoAug 7, 2024
  11. Patrick SteinhardtAug 8, 2024
  12. Junio C HamanoAug 8, 2024
  13. Patrick SteinhardtAug 8, 2024
  14. Junio C HamanoAug 8, 2024
  15. 2/6 builtin/clone: propagate ref storage format to submodulesPatrick Steinhardt, Aug 7, 2024
  16. Junio C HamanoAug 7, 2024
  17. 3/6 refs: fix ref storage format for submodule ref storesPatrick Steinhardt, Aug 7, 2024
  18. 4/6 submodule: fix leaking fetch tasksPatrick Steinhardt, Aug 7, 2024
  19. 5/6 submodule: fix leaking seen submodule namesPatrick Steinhardt, Aug 7, 2024
  20. 6/6 object: fix leaking packfiles when closing object storePatrick Steinhardt, Aug 7, 2024
  21. Patrick SteinhardtAug 7, 2024
  22. Junio C HamanoAug 8, 2024
  23. Patrick SteinhardtAug 8, 2024
  24. 0/8 Improvements for ref storage formats with submodulesPatrick Steinhardt, Aug 8, 2024
  25. 1/8 git-submodule.sh: break overly long command linesPatrick Steinhardt, Aug 8, 2024
  26. 2/8 builtin/submodule: allow cloning with different ref storage formatPatrick Steinhardt, Aug 8, 2024
  27. 3/8 builtin/clone: propagate ref storage format to submodulesPatrick Steinhardt, Aug 8, 2024
  28. Kristoffer HaugsbakkAug 8, 2024
  29. Patrick SteinhardtAug 8, 2024
  30. 4/8 refs: fix ref storage format for submodule ref storesPatrick Steinhardt, Aug 8, 2024
  31. 5/8 builtin/submodule: allow "add" to use different ref storage formatPatrick Steinhardt, Aug 8, 2024
  32. 6/8 submodule: fix leaking fetch tasksPatrick Steinhardt, Aug 8, 2024
  33. 7/8 submodule: fix leaking seen submodule namesPatrick Steinhardt, Aug 8, 2024
  34. 8/8 object: fix leaking packfiles when closing object storePatrick Steinhardt, Aug 8, 2024
  35. Junio C HamanoAug 8, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.