Skip to content

Commit d1778a9

Browse files
authored
Avoid deadlocks in geometric repacking on windows (#903)
This is a companion of gitgitgadget#2103 and of git-for-windows#6215. On Windows, `maintenance_task_geometric_repack()` opens pack index files via `pack_geometry_init()` (which `mmap()`s the `.idx` files), then spawns `git repack` as a child process without setting `child.odb_to_close`. The parent's `mmap()`s prevent the child from deleting old `.idx` files. On Windows 10 builds before the POSIX delete semantics change (between Build 17134.1304 and 18363.657, see https://stackoverflow.com/a/60512798), this results in `Unlink of file '.git/objects/pack/pack-<hash>.idx' failed. Should I try again?` during fetch-triggered auto-maintenance with the geometric strategy. The fix adds the missing `child.odb_to_close = the_repository->objects` line, matching all other maintenance tasks. The first commit introduces a `GIT_TEST_LEGACY_DELETE` environment variable to simulate legacy (pre-POSIX) delete semantics on modern Windows, so the regression test can verify the fix even on Windows 11.
2 parents 2bb9f4f + 12ebd5c commit d1778a9

3 files changed

Lines changed: 67 additions & 3 deletions

File tree

builtin/gc.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1621,6 +1621,7 @@ static int maintenance_task_geometric_repack(struct maintenance_run_opts *opts,
16211621
pack_geometry_split(&geometry);
16221622

16231623
child.git_cmd = 1;
1624+
child.odb_to_close = the_repository->objects;
16241625

16251626
strvec_pushl(&child.args, "repack", "-d", "-l", NULL);
16261627
if (geometry.split < geometry.pack_nr)

compat/mingw.c

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -554,20 +554,63 @@ static wchar_t *normalize_ntpath(wchar_t *wbuf)
554554
return wbuf;
555555
}
556556

557+
/*
558+
* Use SetFileInformationByHandle(FileDispositionInfo) to force legacy
559+
* (non-POSIX) delete semantics. On Windows 11, DeleteFileW() uses POSIX
560+
* delete semantics internally, allowing deletion even with active
561+
* MapViewOfFile views. This helper simulates Windows 10 behavior where
562+
* deletion fails if a file mapping exists.
563+
*
564+
* Returns nonzero on success (like DeleteFileW), 0 on failure.
565+
*/
566+
static int legacy_delete_file(const wchar_t *wpathname)
567+
{
568+
FILE_DISPOSITION_INFO fdi = { TRUE };
569+
DWORD gle;
570+
HANDLE h = CreateFileW(wpathname, DELETE,
571+
FILE_SHARE_READ | FILE_SHARE_WRITE |
572+
FILE_SHARE_DELETE,
573+
NULL, OPEN_EXISTING,
574+
FILE_FLAG_OPEN_REPARSE_POINT, NULL);
575+
if (h == INVALID_HANDLE_VALUE)
576+
return 0;
577+
578+
if (SetFileInformationByHandle(h, FileDispositionInfo,
579+
&fdi, sizeof(fdi))) {
580+
CloseHandle(h);
581+
return 1;
582+
}
583+
gle = GetLastError();
584+
CloseHandle(h);
585+
SetLastError(gle);
586+
return 0;
587+
}
588+
589+
static int try_delete_file(const wchar_t *wpathname, int use_legacy)
590+
{
591+
if (use_legacy)
592+
return legacy_delete_file(wpathname);
593+
return DeleteFileW(wpathname);
594+
}
595+
557596
int mingw_unlink(const char *pathname, int handle_in_use_error)
558597
{
598+
static int use_legacy_delete = -1;
559599
int tries = 0;
560600
wchar_t wpathname[MAX_LONG_PATH];
561601
if (xutftowcs_long_path(wpathname, pathname) < 0)
562602
return -1;
563603

564-
if (DeleteFileW(wpathname))
604+
if (use_legacy_delete < 0)
605+
use_legacy_delete = !!getenv("GIT_TEST_LEGACY_DELETE");
606+
607+
if (try_delete_file(wpathname, use_legacy_delete))
565608
return 0;
566609

567610
do {
568611
/* read-only files cannot be removed */
569612
_wchmod(wpathname, 0666);
570-
if (!_wunlink(wpathname))
613+
if (try_delete_file(wpathname, use_legacy_delete))
571614
return 0;
572615
if (!is_file_in_use_error(GetLastError()))
573616
break;

t/t7900-maintenance.sh

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,16 @@ run_and_verify_geometric_pack () {
582582

583583
# And verify that there are no loose objects anymore.
584584
git count-objects -v >count &&
585-
test_grep '^count: 0$' count
585+
test_grep '^count: 0$' count &&
586+
587+
# Verify that no orphaned .idx files were left behind. On
588+
# Windows, a missing odb_to_close causes the parent to hold
589+
# mmap handles on .idx files, silently preventing their
590+
# deletion by the child git-repack process.
591+
ls .git/objects/pack/pack-*.idx .git/objects/pack/pack-*.pack |
592+
sed "s/\.pack$/.idx/" |
593+
sort | uniq -u >orphaned-idx &&
594+
test_must_be_empty orphaned-idx
586595
}
587596

588597
test_expect_success 'geometric repacking task' '
@@ -630,8 +639,19 @@ test_expect_success 'geometric repacking task' '
630639
631640
# And these two small packs should now be merged via the
632641
# geometric repack. The large packfile should remain intact.
642+
cp -R .git/objects .git/objects.save &&
633643
run_and_verify_geometric_pack 2 &&
634644
645+
# On Windows, verify the same with legacy delete semantics
646+
# that reject deletion of mmap-held .idx files.
647+
if test_have_prereq MINGW
648+
then
649+
rm -rf .git/objects &&
650+
mv .git/objects.save .git/objects &&
651+
test_env GIT_TEST_LEGACY_DELETE=1 \
652+
run_and_verify_geometric_pack 2
653+
fi &&
654+
635655
# If we now add two more objects and repack twice we should
636656
# then see another all-into-one repack. This time around
637657
# though, as we have unreachable objects, we should also see a

0 commit comments

Comments
 (0)