format-patch: fix leak of rev_info in prepare_bases()

In prepare_bases() we do a custom revision walk, separate from the main
format-patch walk. After we finish, we fail to call release_revisions(),
possibly leaking its contents.

We failed to notice it so far because the revision machinery doesn't
always allocate. But at least one case can trigger the leak: if a commit
graph is present, then the topo-walk allocates revs.topo_walk_info and
some associated data structures. You can see it in the test suite by
running:

  make SANITIZE=leak
  cd t
  GIT_TEST_COMMIT_GRAPH=1 ./t4014-format-patch.sh

which yields many entries like:

  ==git==3687620==ERROR: LeakSanitizer: detected memory leaks
  Direct leak of 200 byte(s) in 1 object(s) allocated from:
      #0 0x7f4ccba185cb in malloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:74
      #1 0x55cd452cdd0b in do_xmalloc wrapper.c:55
      #2 0x55cd452cdd9d in xmalloc wrapper.c:76
      #3 0x55cd45255473 in init_topo_walk revision.c:3845
      #4 0x55cd45255bef in prepare_revision_walk revision.c:4017
      #5 0x55cd44ffec40 in prepare_bases builtin/log.c:1872
      #6 0x55cd450010ec in cmd_format_patch builtin/log.c:2439

The un-released rev_info has been there since the code was added in
fa2ab86d18 (format-patch: add '--base' option to record base tree info,
2016-04-26), but back then we didn't even have a way to release rev_info
resources! The actual leak probably started around f0d9cc4196
(revision.c: begin refactoring --topo-order logic, 2018-11-01), but it's
hard to bisect because there were so many other unrelated leaks back
then.

So I'm not sure exactly when the leak started beyond "long ago", but it
is easy-ish to find now (since we've plugged all those other leaks) and
the solution is clear.

I didn't add a new test since we can demonstrate it with the existing
ones, but it does require tweaking a test variable. We might consider
ways to get more automatic leak-checking coverage there, but I think it
should be done outside of this fix.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/log.c b/builtin/log.c
index 8c0939dd42ada28bc4b7bd60aa33e6806dfbb067..baf98b75e11bbe19729e60f5c148140cc3fbc289 100644 (file)
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1884,6 +1884,7 @@ static void prepare_bases(struct base_tree_info *bases,
                bases->nr_patch_id++;
        }
        clear_commit_base(&commit_base);
+       release_revisions(&revs);
 }
 
 static void print_bases(struct base_tree_info *bases, FILE *file)