Skip to content

maintenance(geometric): avoid deadlocks on Windows 10

Johannes Schindelin via GitGitGadget git 2 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v2 Latest2 patches 1 replyIn review
  2. v12 patches 4 repliesSuperseded

Changes in v2 from the cover letter

* The code now uses git_env_bool() as appropriate (thanks Patrick!)

Patches in v2 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/2mingw: optionally use legacy (non-POSIX) delete semanticscompat/mingw.c+45 -20 replies
2/2maintenance(geometric): do release the `.idx` files before repackingbuiltin/gc.c t/t7900-maintenance.sh+22 -10 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v2 yet.

Read the v2 thread

Cover letter [PATCH v2 0/2] maintenance(geometric): avoid deadlocks on Windows 10

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 https://github.com/git-for-windows/git/issues/6210.

Changes since v1:

* The code now uses git_env_bool() as appropriate (thanks Patrick!)

Johannes Schindelin (2):
mingw: optionally use legacy (non-POSIX) delete semantics maintenance(geometric): do release the .idx files before repacking

 builtin/gc.c           |  1 +
 compat/mingw.c         | 47 ++++++++++++++++++++++++++++++++++++++++--
 t/t7900-maintenance.sh | 22 +++++++++++++++++++-
 3 files changed, 67 insertions(+), 3 deletions(-)

base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2103%2Fdscho%2Favoid-deadlocks-in-geometric-repacking-on-windows-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2103/dscho/avoid-deadlocks-in-geometric-repacking-on-windows-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2103

Range-diff vs v1:

1: 97508e91b6 ! 1: 8ee749dd2f mingw: optionally use legacy (non-POSIX) delete semantics

     @@ compat/mingw.c: static wchar_t *normalize_ntpath(wchar_t *wbuf)
       
      -	if (DeleteFileW(wpathname))
      +	if (use_legacy_delete < 0)
     -+		use_legacy_delete = !!getenv("GIT_TEST_LEGACY_DELETE");
     ++		use_legacy_delete = git_env_bool("GIT_TEST_LEGACY_DELETE", 0);
      +
      +	if (try_delete_file(wpathname, use_legacy_delete))
       		return 0;

2: 12ebd5c56f = 2: 66219b79fa maintenance(geometric): do release the .idx files before repacking

Signature · 1 line
gitgitgadget