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

[PATCH 04/10] commit-graph: move slab-clearing to close_commit_graph()

From
Jeff King <peff@peff.net>
Date
Oct 3, 2023, 20:27 UTC
Message-ID
<20231003202752.GD7812@coredump.intra.peff.net>
In-Reply-To
<20231003202504.GA7697@coredump.intra.peff.net>

When closing and freeing a commit-graph, the main entry point is close_commit_graph(), which then uses close_commit_graph_one() to recurse through the base_graph links and free each one.

Commit 957ba814bf (commit-graph: when closing the graph, also release the slab, 2021-09-08) put the call to clear the slab into the recursive function, but this is pointless: there's only a single global slab variable. It works OK in practice because clearing the slab is idempotent, but it makes the code harder to reason about and refactor.

Move it into the parent function so it's only called once (and there are no other direct callers of the recursive close_commit_graph_one(), so we are not hurting them).

Signed-off-by: Jeff King <peff@peff.net>
---
 commit-graph.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/commit-graph.c b/commit-graph.c
index 5e8a3a5085..dc54ef4776 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -728,13 +728,13 @@ static void close_commit_graph_one(struct commit_graph *g)
 	if (!g)
 		return;
 
-	clear_commit_graph_data_slab(&commit_graph_data_slab);
 	close_commit_graph_one(g->base_graph);
 	free_commit_graph(g);
 }
 
 void close_commit_graph(struct raw_object_store *o)
 {
+	clear_commit_graph_data_slab(&commit_graph_data_slab);
 	close_commit_graph_one(o->commit_graph);
 	o->commit_graph = NULL;
 }
-- 
2.42.0.810.gbc538a0ee6
Previous: Taylor BlauNext: Taylor Blau
Message 7 of 24 in “some commit-graph leak fixes”
  1. 0/10 some commit-graph leak fixesJeff King, Oct 3, 2023
  2. 01/10 t6700: mark test as leak-freeJeff King, Oct 3, 2023
  3. Taylor BlauOct 5, 2023
  4. 02/10 commit-reach: free temporary list in get_octopus_merge_bases()Jeff King, Oct 3, 2023
  5. 03/10 merge: free result of repo_get_merge_bases()Jeff King, Oct 3, 2023
  6. Taylor BlauOct 5, 2023
  7. 04/10 commit-graph: move slab-clearing to close_commit_graph()Jeff King, Oct 3, 2023
  8. Taylor BlauOct 5, 2023
  9. 05/10 commit-graph: free all elements of graph chainJeff King, Oct 3, 2023
  10. 06/10 commit-graph: delay base_graph assignment in add_graph_to_chain()Jeff King, Oct 3, 2023
  11. Taylor BlauOct 5, 2023
  12. 07/10 commit-graph: free graph struct that was not added to chainJeff King, Oct 3, 2023
  13. 08/10 commit-graph: free write-context entries before overwritingJeff King, Oct 3, 2023
  14. Taylor BlauOct 5, 2023
  15. Jeff KingOct 5, 2023
  16. 09/10 commit-graph: free write-context base_graph_name during cleanupJeff King, Oct 3, 2023
  17. 10/10 commit-graph: clear oidset after finishing writeJeff King, Oct 3, 2023
  18. Is SANITIZE=leak make test unreliable for anyone else?Eric W. Biederman, Oct 4, 2023
  19. Jeff KingOct 4, 2023
  20. Eric W. BiedermanOct 4, 2023
  21. Jeff KingOct 4, 2023
  22. Eric W. BiedermanOct 4, 2023
  23. Taylor BlauOct 5, 2023
  24. Junio C HamanoOct 6, 2023

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.