revision: avoid leaking bloom keyvecs with multiple traversals

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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/revision.c b/revision.c
index 599b3a66c369ca3b0099311250cf758788fdf6ce..c2f3276e486f0cd3dae982a61ddd545e9e744de2 100644 (file)
--- a/revision.c
+++ b/revision.c
@@ -708,6 +708,8 @@ cleanup:
 
 static void prepare_to_use_bloom_filter(struct rev_info *revs)
 {
+       release_revisions_bloom_keyvecs(revs);
+
        if (!revs->commits)
                return;