Skip to content

Commit 81cd1ee

Browse files
brandb97gitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent fcfe606 commit 81cd1ee

File tree

2 files changed

+19
-2
lines changed

2 files changed

+19
-2
lines changed

pack-bitmap.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1363,8 +1363,8 @@ static struct bitmap *find_boundary_objects(struct bitmap_index *bitmap_git,
13631363
bitmap_set(roots_bitmap, pos);
13641364
}
13651365

1366-
if (!cascade_pseudo_merges_1(bitmap_git, cb.base, roots_bitmap))
1367-
bitmap_free(roots_bitmap);
1366+
cascade_pseudo_merges_1(bitmap_git, cb.base, roots_bitmap);
1367+
bitmap_free(roots_bitmap);
13681368
}
13691369

13701370
/*

t/t5333-pseudo-merge-bitmaps.sh

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,4 +445,21 @@ test_expect_success 'pseudo-merge closure' '
445445
)
446446
'
447447

448+
test_expect_success 'use pseudo-merge in boundary traversal' '
449+
git init pseudo-merge-boundary-traversal &&
450+
(
451+
cd pseudo-merge-boundary-traversal &&
452+
453+
git config bitmapPseudoMerge.test.pattern refs/ &&
454+
git config pack.useBitmapBoundaryTraversal true &&
455+
456+
test_commit A &&
457+
git repack -adb &&
458+
test_commit B &&
459+
460+
nr=$(git rev-list --count --use-bitmap-index HEAD~1..HEAD) &&
461+
test 1 -eq "$nr"
462+
)
463+
'
464+
448465
test_done

0 commit comments

Comments
 (0)