Skip to content

Commit 58ac8b1

Browse files
authored
maintenance(geometric): avoid deadlocks on Windows 10 (#6215)
This PR is a companion of gitgitgadget#2103. 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. This fixes #6210. Tested-by: Patryk Miś <foss@patrykmis.com>
2 parents d5b8d9b + 12ebd5c commit 58ac8b1

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
@@ -1590,6 +1590,7 @@ static int maintenance_task_geometric_repack(struct maintenance_run_opts *opts,
15901590
pack_geometry_split(&geometry);
15911591

15921592
child.git_cmd = 1;
1593+
child.odb_to_close = the_repository->objects;
15931594

15941595
strvec_pushl(&child.args, "repack", "-d", "-l", NULL);
15951596
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
@@ -532,7 +532,16 @@ run_and_verify_geometric_pack () {
532532

533533
# And verify that there are no loose objects anymore.
534534
git count-objects -v >count &&
535-
test_grep '^count: 0$' count
535+
test_grep '^count: 0$' count &&
536+
537+
# Verify that no orphaned .idx files were left behind. On
538+
# Windows, a missing odb_to_close causes the parent to hold
539+
# mmap handles on .idx files, silently preventing their
540+
# deletion by the child git-repack process.
541+
ls .git/objects/pack/pack-*.idx .git/objects/pack/pack-*.pack |
542+
sed "s/\.pack$/.idx/" |
543+
sort | uniq -u >orphaned-idx &&
544+
test_must_be_empty orphaned-idx
536545
}
537546

538547
test_expect_success 'geometric repacking task' '
@@ -580,8 +589,19 @@ test_expect_success 'geometric repacking task' '
580589
581590
# And these two small packs should now be merged via the
582591
# geometric repack. The large packfile should remain intact.
592+
cp -R .git/objects .git/objects.save &&
583593
run_and_verify_geometric_pack 2 &&
584594
595+
# On Windows, verify the same with legacy delete semantics
596+
# that reject deletion of mmap-held .idx files.
597+
if test_have_prereq MINGW
598+
then
599+
rm -rf .git/objects &&
600+
mv .git/objects.save .git/objects &&
601+
test_env GIT_TEST_LEGACY_DELETE=1 \
602+
run_and_verify_geometric_pack 2
603+
fi &&
604+
585605
# If we now add two more objects and repack twice we should
586606
# then see another all-into-one repack. This time around
587607
# though, as we have unreachable objects, we should also see a

0 commit comments

Comments
 (0)