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

[PATCH v2 2/2] odb: properly close sources before freeing them

From
Patrick Steinhardt <ps@pks.im>
Date
Dec 11, 2025, 07:19 UTC
Message-ID
<20251211-odb-related-fixes-v2-2-bdf875ce51fc@pks.im>
In-Reply-To
<20251211-odb-related-fixes-v2-0-bdf875ce51fc@pks.im>

It is possible to hit a memory leak when reading data from a submodule via git-grep(1):

  Direct leak of 192 byte(s) in 1 object(s) allocated from:
    #0 0x55555562e726 in calloc (git+0xda726)
    #1 0x555555964734 in xcalloc ../wrapper.c:154:8
    #2 0x555555835136 in load_multi_pack_index_one ../midx.c:135:2
    #3 0x555555834fd6 in load_multi_pack_index ../midx.c:382:6
    #4 0x5555558365b6 in prepare_multi_pack_index_one ../midx.c:716:17
    #5 0x55555586c605 in packfile_store_prepare ../packfile.c:1103:3
    #6 0x55555586c90c in packfile_store_reprepare ../packfile.c:1118:2
    #7 0x5555558546b3 in odb_reprepare ../odb.c:1106:2
    #8 0x5555558539e4 in do_oid_object_info_extended ../odb.c:715:4
    #9 0x5555558533d1 in odb_read_object_info_extended ../odb.c:862:8
    #10 0x5555558540bd in odb_read_object ../odb.c:920:6
    #11 0x55555580a330 in grep_source_load_oid ../grep.c:1934:12
    #12 0x55555580a13a in grep_source_load ../grep.c:1986:10
    #13 0x555555809103 in grep_source_is_binary ../grep.c:2014:7
    #14 0x555555807574 in grep_source_1 ../grep.c:1625:8
    #15 0x555555807322 in grep_source ../grep.c:1837:10
    #16 0x5555556a5c58 in run ../builtin/grep.c:208:10
    #17 0x55555562bb42 in void* ThreadStartFunc<false>(void*) lsan_interceptors.cpp.o
    #18 0x7ffff7a9a979 in start_thread (/nix/store/xx7cm72qy2c0643cm1ipngd87aqwkcdp-glibc-2.40-66/lib/libc.so.6+0x9a979) (BuildId: cddea92d6cba8333be952b5a02fd47d61054c5ab)
    #19 0x7ffff7b22d2b in __GI___clone3 (/nix/store/xx7cm72qy2c0643cm1ipngd87aqwkcdp-glibc-2.40-66/lib/libc.so.6+0x122d2b) (BuildId: cddea92d6cba8333be952b5a02fd47d61054c5ab)

The root caues of this leak is the way we set up and release the submodule:

  1. We use `repo_submodule_init()` to initialize a new repository. This
     repository is stored in `repos_to_free`.
  2. We now read data from the submodule repository.
  3. We then call `repo_clear()` on the submodule repositories.
  4. `repo_clear()` calls `odb_free()`.
  5. `odb_free()` calls `odb_free_sources()` followed by `odb_close()`.

The issue here is the 5th step: we call `odb_free_sources()` _before_ we call `odb_close()`. But `odb_free_sources()` already frees all sources, so the logic that closes them in `odb_close()` now becomes a no-op. As a consequence, we never explicitly close sources at all.

Fix the leak by closing the store before we free the sources.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/odb.c b/odb.c
index dc8f292f3d..8e67afe185 100644
--- a/odb.c
+++ b/odb.c
@@ -1132,13 +1132,13 @@ void odb_free(struct object_database *o)
 	oidmap_clear(&o->replace_map, 1);
 	pthread_mutex_destroy(&o->replace_mutex);
 
+	odb_close(o);
 	odb_free_sources(o);
 
 	for (size_t i = 0; i < o->cached_object_nr; i++)
 		free((char *) o->cached_objects[i].value.buf);
 	free(o->cached_objects);
 
-	odb_close(o);
 	packfile_store_free(o->packfiles);
 	string_list_clear(&o->submodule_source_paths, 0);
 
-- 
2.52.0.270.g3f4935d65f.dirty
Previous: Karthik NayakNext: Justin Tobler
Message 19 of 26 in “Some random object database related fixes”
  1. 0/3 Some random object database related fixesPatrick Steinhardt, Dec 5, 2025
  2. 1/3 builtin/repack: fix geometric repacks with promisor remotesPatrick Steinhardt, Dec 5, 2025
  3. Justin ToblerDec 10, 2025
  4. Patrick SteinhardtDec 11, 2025
  5. 2/3 builtin/gc: fix condition for whether to write commit graphsPatrick Steinhardt, Dec 5, 2025
  6. Justin ToblerDec 10, 2025
  7. Patrick SteinhardtDec 11, 2025
  8. 3/3 odb: properly close sources before freeing themPatrick Steinhardt, Dec 5, 2025
  9. Eric SunshineDec 5, 2025
  10. Patrick SteinhardtDec 6, 2025
  11. Eric SunshineDec 6, 2025
  12. Patrick SteinhardtDec 6, 2025
  13. 0/2 Some random object database related fixesPatrick Steinhardt, Dec 11, 2025
  14. 1/2 builtin/gc: fix condition for whether to write commit graphsPatrick Steinhardt, Dec 11, 2025
  15. Toon ClaesDec 11, 2025
  16. Karthik NayakJan 6, 2026
  17. Patrick SteinhardtJan 6, 2026
  18. Karthik NayakJan 6, 2026
  19. 2/2 odb: properly close sources before freeing themPatrick Steinhardt, Dec 11, 2025
  20. Justin ToblerDec 12, 2025
  21. Karthik NayakJan 6, 2026
  22. 0/2 Some random object database related fixesPatrick Steinhardt, Jan 6, 2026
  23. 1/2 builtin/gc: fix condition for whether to write commit graphsPatrick Steinhardt, Jan 6, 2026
  24. 2/2 odb: properly close sources before freeing themPatrick Steinhardt, Jan 6, 2026
  25. Karthik NayakJan 6, 2026
  26. Junio C HamanoJan 7, 2026

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.