pack-bitmap: remove checks before bitmap_free

In pack-bitmap.c:find_boundary_objects(), the roots_bitmap is only freed
if cascade_pseudo_merges_1() fails. However, cascade_pseudo_merges_1()
uses roots_bitmap as a mutable reference without taking ownership of it.
As a result, if cascade_pseudo_merges_1() succeeds, roots_bitmap is leaked.
And this leak currently lacks a dedicated test to detect it.

To fix this leak, remove if cascade_pseudo_merges_1() succeed check and
always calling bitmap_free(roots_bitmap);

To trigger this leak, we need roots_bitmap that contains at least one
pseudo merge. So that we can use pseudo merge bitmap when we compute roots
reachable bitmap. Here we create two commits: first A then B. Add A
to the pseudo-merge and perform a traversal over the range A..B.
In this scenario, the "haves" set will be {A}, and cascade_pseudo_merges_1
will succeed, thereby exposing the leak due to the missing roots_bitmap
cleanup.

Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/pack-bitmap.c b/pack-bitmap.c
index ac6d62b980c5a8d086bb1b71f46df721d11b3ad4..8727f316de92543fb7151fb58ebdc9a25a41a4d6 100644 (file)
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1363,8 +1363,8 @@ static struct bitmap *find_boundary_objects(struct bitmap_index *bitmap_git,
                        bitmap_set(roots_bitmap, pos);
                }
 
-               if (!cascade_pseudo_merges_1(bitmap_git, cb.base, roots_bitmap))
-                       bitmap_free(roots_bitmap);
+               cascade_pseudo_merges_1(bitmap_git, cb.base, roots_bitmap);
+               bitmap_free(roots_bitmap);
        }
 
        /*

diff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh
index 56674db562f948891261c7f40f13f9d13f4ec6a7..ba5ae6a00c984776862e37d7258fecb0e40ec498 100755 (executable)
--- a/t/t5333-pseudo-merge-bitmaps.sh
+++ b/t/t5333-pseudo-merge-bitmaps.sh
@@ -445,4 +445,21 @@ test_expect_success 'pseudo-merge closure' '
        )
 '
 
+test_expect_success 'use pseudo-merge in boundary traversal' '
+       git init pseudo-merge-boundary-traversal &&
+       (
+               cd pseudo-merge-boundary-traversal &&
+
+               git config bitmapPseudoMerge.test.pattern refs/ &&
+               git config pack.useBitmapBoundaryTraversal true &&
+
+               test_commit A &&
+               git repack -adb &&
+               test_commit B &&
+
+               nr=$(git rev-list --count --use-bitmap-index HEAD~1..HEAD) &&
+               test 1 -eq "$nr"
+       )
+'
+
 test_done