From: Patrick Steinhardt <ps@pks•im>
To: git@vger•kernel.org
Cc: Justin Tobler <jltobler@gmail•com>,
Eric Sunshine <sunshine@sunshineco•com>
Subject: [PATCH v2 2/2] odb: properly close sources before freeing them
Date: Thu, 11 Dec 2025 08:19:59 +0100 [thread overview]
Message-ID: <20251211-odb-related-fixes-v2-2-bdf875ce51fc@pks.im> (raw)
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
next prev parent reply other threads:[~2025-12-11 7:20 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-05 8:19 [PATCH 0/3] Some random object database related fixes Patrick Steinhardt
2025-12-05 8:19 ` [PATCH 1/3] builtin/repack: fix geometric repacks with promisor remotes Patrick Steinhardt
2025-12-10 19:31 ` Justin Tobler
2025-12-11 5:46 ` Patrick Steinhardt
2025-12-05 8:19 ` [PATCH 2/3] builtin/gc: fix condition for whether to write commit graphs Patrick Steinhardt
2025-12-10 19:49 ` Justin Tobler
2025-12-11 5:48 ` Patrick Steinhardt
2025-12-05 8:20 ` [PATCH 3/3] odb: properly close sources before freeing them Patrick Steinhardt
2025-12-05 23:14 ` Eric Sunshine
2025-12-06 11:38 ` Patrick Steinhardt
2025-12-06 11:43 ` Eric Sunshine
2025-12-06 12:04 ` Patrick Steinhardt
2025-12-11 7:19 ` [PATCH v2 0/2] Some random object database related fixes Patrick Steinhardt
2025-12-11 7:19 ` [PATCH v2 1/2] builtin/gc: fix condition for whether to write commit graphs Patrick Steinhardt
2025-12-11 14:16 ` Toon Claes
2026-01-06 11:27 ` Karthik Nayak
2026-01-06 11:55 ` Patrick Steinhardt
2026-01-06 16:33 ` Karthik Nayak
2025-12-11 7:19 ` Patrick Steinhardt [this message]
2025-12-12 15:01 ` [PATCH v2 0/2] Some random object database related fixes Justin Tobler
2026-01-06 11:29 ` Karthik Nayak
2026-01-06 12:58 ` [PATCH v3 " Patrick Steinhardt
2026-01-06 12:58 ` [PATCH v3 1/2] builtin/gc: fix condition for whether to write commit graphs Patrick Steinhardt
2026-01-06 12:58 ` [PATCH v3 2/2] odb: properly close sources before freeing them Patrick Steinhardt
2026-01-06 16:30 ` [PATCH v3 0/2] Some random object database related fixes Karthik Nayak
2026-01-07 3:51 ` 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=20251211-odb-related-fixes-v2-2-bdf875ce51fc@pks.im \
--to=ps@pks$(echo .)im \
--cc=git@vger$(echo .)kernel.org \
--cc=jltobler@gmail$(echo .)com \
--cc=sunshine@sunshineco$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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