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

[PATCH 12/22] object: clear grafts when clearing parsed object pool

From
Patrick Steinhardt <ps@pks.im>
Date
Aug 26, 2024, 07:22 UTC
Message-ID
<922b8640a55c74e91ce69f6976d20f948be55765.1724656120.git.ps@pks.im>
In-Reply-To
<cover.1724656120.git.ps@pks.im>

We do not clear grafts part of the parsed object pool when clearing the pool itself, which can lead to memory leaks when a repository is being cleared.

Fix this by moving `reset_commit_grafts()` into "object.c" and making it part of the `struct parsed_object_pool` interface such that we can call it from `parsed_object_pool_clear()`. Adapt `parsed_object_pool_new()` to take and store a reference to its owning repository, which is needed by `unparse_commit()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 commit.c     | 14 +-------------
 commit.h     |  3 ++-
 object.c     | 14 +++++++++++++-
 object.h     |  4 +++-
 repository.c |  2 +-
 shallow.c    |  2 +-
 6 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/commit.c b/commit.c
index ec9efc189d5..bbef0e81c65 100644
--- a/commit.c
+++ b/commit.c
@@ -177,7 +177,7 @@ int commit_graft_pos(struct repository *r, const struct object_id *oid)
 		       commit_graft_oid_access);
 }
 
-static void unparse_commit(struct repository *r, const struct object_id *oid)
+void unparse_commit(struct repository *r, const struct object_id *oid)
 {
 	struct commit *c = lookup_commit(r, oid);
 
@@ -318,18 +318,6 @@ int for_each_commit_graft(each_commit_graft_fn fn, void *cb_data)
 	return ret;
 }
 
-void reset_commit_grafts(struct repository *r)
-{
-	int i;
-
-	for (i = 0; i < r->parsed_objects->grafts_nr; i++) {
-		unparse_commit(r, &r->parsed_objects->grafts[i]->oid);
-		free(r->parsed_objects->grafts[i]);
-	}
-	r->parsed_objects->grafts_nr = 0;
-	r->parsed_objects->commit_graft_prepared = 0;
-}
-
 struct commit_buffer {
 	void *buffer;
 	unsigned long size;
diff --git a/commit.h b/commit.h
index d62b1d93f95..5ba0f77b1eb 100644
--- a/commit.h
+++ b/commit.h
@@ -108,6 +108,8 @@ static inline int repo_parse_commit_no_graph(struct repository *r,
 
 void parse_commit_or_die(struct commit *item);
 
+void unparse_commit(struct repository *r, const struct object_id *oid);
+
 struct buffer_slab;
 struct buffer_slab *allocate_commit_buffer_slab(void);
 void free_commit_buffer_slab(struct buffer_slab *bs);
@@ -240,7 +242,6 @@ int commit_graft_pos(struct repository *r, const struct object_id *oid);
 int register_commit_graft(struct repository *r, struct commit_graft *, int);
 void prepare_commit_graft(struct repository *r);
 struct commit_graft *lookup_commit_graft(struct repository *r, const struct object_id *oid);
-void reset_commit_grafts(struct repository *r);
 
 struct commit *get_fork_point(const char *refname, struct commit *commit);
 
diff --git a/object.c b/object.c
index d756c7f2ea3..94ea8fb8d2c 100644
--- a/object.c
+++ b/object.c
@@ -545,11 +545,12 @@ void repo_clear_commit_marks(struct repository *r, unsigned int flags)
 	}
 }
 
-struct parsed_object_pool *parsed_object_pool_new(void)
+struct parsed_object_pool *parsed_object_pool_new(struct repository *repo)
 {
 	struct parsed_object_pool *o = xmalloc(sizeof(*o));
 	memset(o, 0, sizeof(*o));
 
+	o->repo = repo;
 	o->blob_state = allocate_alloc_state();
 	o->tree_state = allocate_alloc_state();
 	o->commit_state = allocate_alloc_state();
@@ -628,6 +629,16 @@ void raw_object_store_clear(struct raw_object_store *o)
 	hashmap_clear(&o->pack_map);
 }
 
+void parsed_object_pool_reset_commit_grafts(struct parsed_object_pool *o)
+{
+	for (int i = 0; i < o->grafts_nr; i++) {
+		unparse_commit(o->repo, &o->grafts[i]->oid);
+		free(o->grafts[i]);
+	}
+	o->grafts_nr = 0;
+	o->commit_graft_prepared = 0;
+}
+
 void parsed_object_pool_clear(struct parsed_object_pool *o)
 {
 	/*
@@ -659,6 +670,7 @@ void parsed_object_pool_clear(struct parsed_object_pool *o)
 	free_commit_buffer_slab(o->buffer_slab);
 	o->buffer_slab = NULL;
 
+	parsed_object_pool_reset_commit_grafts(o);
 	clear_alloc_state(o->blob_state);
 	clear_alloc_state(o->tree_state);
 	clear_alloc_state(o->commit_state);
diff --git a/object.h b/object.h
index 05691486ebf..17f32f1103e 100644
--- a/object.h
+++ b/object.h
@@ -7,6 +7,7 @@ struct buffer_slab;
 struct repository;
 
 struct parsed_object_pool {
+	struct repository *repo;
 	struct object **obj_hash;
 	int nr_objs, obj_hash_size;
 
@@ -31,8 +32,9 @@ struct parsed_object_pool {
 	struct buffer_slab *buffer_slab;
 };
 
-struct parsed_object_pool *parsed_object_pool_new(void);
+struct parsed_object_pool *parsed_object_pool_new(struct repository *repo);
 void parsed_object_pool_clear(struct parsed_object_pool *o);
+void parsed_object_pool_reset_commit_grafts(struct parsed_object_pool *o);
 
 struct object_list {
 	struct object *item;
diff --git a/repository.c b/repository.c
index 9825a308993..e6fc2c6aa9d 100644
--- a/repository.c
+++ b/repository.c
@@ -54,7 +54,7 @@ void initialize_repository(struct repository *repo)
 {
 	repo->objects = raw_object_store_new();
 	repo->remote_state = remote_state_new();
-	repo->parsed_objects = parsed_object_pool_new();
+	repo->parsed_objects = parsed_object_pool_new(repo);
 	ALLOC_ARRAY(repo->index, 1);
 	index_state_init(repo->index, repo);
 
diff --git a/shallow.c b/shallow.c
index b8cd051e3b6..a10cf9e9d5d 100644
--- a/shallow.c
+++ b/shallow.c
@@ -97,7 +97,7 @@ static void reset_repository_shallow(struct repository *r)
 {
 	r->parsed_objects->is_shallow = -1;
 	stat_validity_clear(r->parsed_objects->shallow_stat);
-	reset_commit_grafts(r);
+	parsed_object_pool_reset_commit_grafts(r->parsed_objects);
 }
 
 int commit_shallow_file(struct repository *r, struct shallow_lock *lk)
-- 
2.46.0.164.g477ce5ccd6.dirty
Previous: Junio C HamanoNext: Patrick Steinhardt
Message 22 of 70 in “Memory leak fixes (pt.6)”
  1. 00/22 Memory leak fixes (pt.6)Patrick Steinhardt, Aug 26, 2024
  2. 01/22 t/test-lib: allow skipping leak checks for passing testsPatrick Steinhardt, Aug 26, 2024
  3. Junio C HamanoAug 27, 2024
  4. Toon claesAug 29, 2024
  5. Patrick SteinhardtAug 30, 2024
  6. 02/22 fetch-pack: fix memory leaks on fetch negotiationPatrick Steinhardt, Aug 26, 2024
  7. 03/22 send-pack: fix leaking common object IDsPatrick Steinhardt, Aug 26, 2024
  8. 04/22 builtin/push: fix leaking refspec query resultPatrick Steinhardt, Aug 26, 2024
  9. Junio C HamanoAug 30, 2024
  10. Patrick SteinhardtSep 2, 2024
  11. 05/22 upload-pack: fix leaking child process data on reachability checksPatrick Steinhardt, Aug 26, 2024
  12. Junio C HamanoAug 30, 2024
  13. 06/22 submodule: fix leaking fetch task dataPatrick Steinhardt, Aug 26, 2024
  14. 07/22 builtin/submodule--helper: fix leaking refs on push-checkPatrick Steinhardt, Aug 26, 2024
  15. 08/22 remote: fix leaking tracking refsPatrick Steinhardt, Aug 26, 2024
  16. Junio C HamanoSep 4, 2024
  17. 09/22 remote: fix leak in reachability check of a remote-tracking refPatrick Steinhardt, Aug 26, 2024
  18. 10/22 send-pack: fix leaking push cert noncePatrick Steinhardt, Aug 26, 2024
  19. Junio C HamanoSep 4, 2024
  20. 11/22 gpg-interface: fix misdesigned signing key interfacesPatrick Steinhardt, Aug 26, 2024
  21. Junio C HamanoSep 4, 2024
  22. 12/22 object: clear grafts when clearing parsed object poolPatrick Steinhardt, Aug 26, 2024
  23. 13/22 shallow: free grafts when unregistering themPatrick Steinhardt, Aug 26, 2024
  24. 14/22 shallow: fix leaking members of `struct shallow_info`Patrick Steinhardt, Aug 26, 2024
  25. Toon claesAug 29, 2024
  26. Junio C HamanoAug 29, 2024
  27. Patrick SteinhardtAug 30, 2024
  28. 15/22 negotiator/skipping: fix leaking commit entriesPatrick Steinhardt, Aug 26, 2024
  29. Calvin WanAug 28, 2024
  30. Josh SteadmonAug 28, 2024
  31. Patrick SteinhardtAug 29, 2024
  32. Calvin WanAug 29, 2024
  33. 16/22 builtin/repack: fix leaking line buffer when packing promisorsPatrick Steinhardt, Aug 26, 2024
  34. Junio C HamanoSep 4, 2024
  35. 17/22 builtin/pack-objects: plug leaking list of keep-packsPatrick Steinhardt, Aug 26, 2024
  36. 18/22 builtin/grep: fix leaking object contextPatrick Steinhardt, Aug 26, 2024
  37. Junio C HamanoSep 4, 2024
  38. 19/22 builtin/fmt-merge-msg: fix leaking buffersPatrick Steinhardt, Aug 26, 2024
  39. 20/22 match-trees: fix leaking prefixes in `shift_tree()`Patrick Steinhardt, Aug 26, 2024
  40. Junio C HamanoSep 4, 2024
  41. 21/22 merge-ort: fix two leaks when handling directory rename modificationsPatrick Steinhardt, Aug 26, 2024
  42. Junio C HamanoSep 4, 2024
  43. Elijah NewrenSep 5, 2024
  44. 22/22 builtin/repack: fix leaking keep-pack listPatrick Steinhardt, Aug 26, 2024
  45. Junio C HamanoSep 4, 2024
  46. 00/22 Memory leak fixes (pt.6)Patrick Steinhardt, Sep 5, 2024
  47. 01/22 t/test-lib: allow skipping leak checks for passing testsPatrick Steinhardt, Sep 5, 2024
  48. 02/22 fetch-pack: fix memory leaks on fetch negotiationPatrick Steinhardt, Sep 5, 2024
  49. 03/22 send-pack: fix leaking common object IDsPatrick Steinhardt, Sep 5, 2024
  50. 04/22 builtin/push: fix leaking refspec query resultPatrick Steinhardt, Sep 5, 2024
  51. 05/22 upload-pack: fix leaking child process data on reachability checksPatrick Steinhardt, Sep 5, 2024
  52. 06/22 submodule: fix leaking fetch task dataPatrick Steinhardt, Sep 5, 2024
  53. 07/22 builtin/submodule--helper: fix leaking refs on push-checkPatrick Steinhardt, Sep 5, 2024
  54. 08/22 remote: fix leaking tracking refsPatrick Steinhardt, Sep 5, 2024
  55. 09/22 remote: fix leak in reachability check of a remote-tracking refPatrick Steinhardt, Sep 5, 2024
  56. 10/22 send-pack: fix leaking push cert noncePatrick Steinhardt, Sep 5, 2024
  57. 11/22 gpg-interface: fix misdesigned signing key interfacesPatrick Steinhardt, Sep 5, 2024
  58. 12/22 object: clear grafts when clearing parsed object poolPatrick Steinhardt, Sep 5, 2024
  59. 13/22 shallow: free grafts when unregistering themPatrick Steinhardt, Sep 5, 2024
  60. 14/22 shallow: fix leaking members of `struct shallow_info`Patrick Steinhardt, Sep 5, 2024
  61. 15/22 negotiator/skipping: fix leaking commit entriesPatrick Steinhardt, Sep 5, 2024
  62. 16/22 builtin/repack: fix leaking line buffer when packing promisorsPatrick Steinhardt, Sep 5, 2024
  63. 17/22 builtin/pack-objects: plug leaking list of keep-packsPatrick Steinhardt, Sep 5, 2024
  64. 18/22 builtin/grep: fix leaking object contextPatrick Steinhardt, Sep 5, 2024
  65. 19/22 builtin/fmt-merge-msg: fix leaking buffersPatrick Steinhardt, Sep 5, 2024
  66. 20/22 match-trees: fix leaking prefixes in `shift_tree()`Patrick Steinhardt, Sep 5, 2024
  67. 21/22 merge-ort: fix two leaks when handling directory rename modificationsPatrick Steinhardt, Sep 5, 2024
  68. 22/22 builtin/repack: fix leaking keep-pack listPatrick Steinhardt, Sep 5, 2024
  69. Junio C HamanoSep 8, 2024
  70. Junio C HamanoSep 12, 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.