From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Patrick Steinhardt <ps@pks.im>
Subject: [PATCH 2/3] revision: avoid leaking bloom keyvecs with multiple traversals
Date: Wed, 1 Jul 2026 02:40:52 -0400 [thread overview]
Message-ID: <20260701064052.GB2580331@coredump.intra.peff.net> (raw)
In-Reply-To: <20260701063538.GA2579765@coredump.intra.peff.net>
In prepare_revision_walk(), we convert the pruning pathspecs into
bloom-filter "keyvecs" via prepare_to_use_bloom_filter(). This allocates
memory which is then freed eventually by release_revisions(), via
release_revisions_bloom_keyvecs().
But there's one case where we leak. If a caller uses the same rev_info
for multiple walks, calling prepare_revision_walk() multiple times, then
subsequent calls will overwrite the earlier keyvecs, leaking them. This
can happen with "git show foo bar", which does a separate no-walk
traversal for "foo" and "bar". Building with SANITIZE=leak and running
the test suite like:
GIT_TEST_COMMIT_GRAPH=1 \
GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=1 \
./t4013-diff-various.sh
will trigger a complaint from LSan. It does not happen without those
extra flags because we don't store on-disk bloom filters by default, and
thus we optimize out the keyvec computation.
We can fix the leak by discarding the old entries before generating new
ones.
There's an alternative fix, which is that prepare_to_use_bloom_filter()
could notice that we already have keyvec entries and just reuse them.
But this is less safe; the keyvec depends on the pruning pathspec, and
we don't know if that has changed.
I think it would _probably_ work in practice, since any caller using a
rev_info for multiple traversals is probably doing so with the same
pathspec. But it would also create a very subtle bug if that assumption
is violated. So we'll do the safer thing here, and generate fresh keyvec
entries for each traversal. The efficiency difference is probably not
noticeable, and this is what was happening already (we just weren't
bothering to free the old ones!).
Signed-off-by: Jeff King <peff@peff.net>
---
revision.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/revision.c b/revision.c
index e91d7e1f11..0ef9d895f0 100644
--- a/revision.c
+++ b/revision.c
@@ -707,6 +707,8 @@ static int convert_pathspec_to_bloom_keyvec(struct bloom_keyvec **out,
static void prepare_to_use_bloom_filter(struct rev_info *revs)
{
+ release_revisions_bloom_keyvecs(revs);
+
if (!revs->commits)
return;
--
2.55.0.394.gcf1c5597d2
next prev parent reply other threads:[~2026-07-01 6:40 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-01 6:35 [PATCH 0/3] bloom-related leak fixes Jeff King
2026-07-01 6:39 ` [PATCH 1/3] bloom: make bloom-filter slab initialization idempotent Jeff King
2026-07-01 13:53 ` Derrick Stolee
2026-07-01 15:50 ` Junio C Hamano
2026-07-01 6:40 ` Jeff King [this message]
2026-07-01 8:26 ` [PATCH 2/3] revision: avoid leaking bloom keyvecs with multiple traversals Patrick Steinhardt
2026-07-01 14:21 ` Derrick Stolee
2026-07-01 15:52 ` Junio C Hamano
2026-07-01 6:42 ` [PATCH 3/3] line-log: drop extra copy of range with bloom filters Jeff King
2026-07-01 14:32 ` [PATCH 0/3] bloom-related leak fixes Derrick Stolee
2026-07-01 17:33 ` 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=20260701064052.GB2580331@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=ps@pks.im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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