git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 0/6] midx-write: fix segfault and do several cleanups

From
Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Date
Sep 5, 2025, 19:26 UTC
Message-ID
<pull.1965.v3.git.1757100378.gitgitgadget@gmail.com>
In-Reply-To
<pull.1965.v2.git.1756589007.gitgitgadget@gmail.com>

I was motivated to start looking closely at midx-write.c due to multiple users reporting Git crashes in their background maintenance, specifically during git multi-pack-index repack calls. I was eventually able to reproduce it in git multi-pack-index expire as well.

Patch 1 is the only change we need to fix this bug. It includes a test case that will fail under --stress with SANITIZE=address. It requires creating many packfiles (50 was not enough, but 100 is enough). As far as I can tell, this bug has existed since Git 2.47.0 in October 2024, but I started hearing reports of this from users in July 2025 (and took a while to get a dump/repro).

The remaining patches are cleanups based on my careful rereading of midx-write.c. There are some issues about error handling that needed some cleanup as well as a removal of the DISABLE_SIGN_COMPARE_WARNINGS macro.

Updates in V3 =============

 * Use test_grep over grep.
 * Translate an error message.
 * Clarify a commit message.

Updates in V2 =============

 * A stale comment to an unsubmitted version of the test is removed.
 * More cases needing open_pack_index() are patched.
 * Typos fixed.
 * A new patch assumes error and sets result to zero only on the few
   successful paths.
Thanks, -Stolee
Derrick Stolee (6):
  midx-write: only load initialized packs
  midx-write: put failing response value back
  midx-write: use cleanup when incremental midx fails
  midx-write: use uint32_t for preferred_pack_idx
  midx-write: reenable signed comparison errors
  midx-write: simplify error cases
 midx-write.c                | 134 +++++++++++++++++-------------------
 t/t5319-multi-pack-index.sh |  22 +++++-
 2 files changed, 86 insertions(+), 70 deletions(-)
base-commit: c44beea485f0f2feaf460e2ac87fdd5608d63cf0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1965%2Fderrickstolee%2Fmidx-write-cleanup-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1965/derrickstolee/midx-write-cleanup-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1965
Range-diff vs v2:
 1:  e02a444315 = 1:  e02a444315 midx-write: only load initialized packs
 2:  a1dd3ed874 ! 2:  1e5f43a417 midx-write: put failing response value back
     @@ t/t5319-multi-pack-index.sh: test_expect_success 'load reverse index when missin
      +		git cat-file --batch-check="%(objectsize:disk)" <tip &&
      +
      +		test_must_fail git multi-pack-index write 2>err &&
     -+		grep "could not load pack" err
     ++		test_grep "could not load pack" err
       	)
       '
       
 3:  c4f75cca09 = 3:  414ae51024 midx-write: use cleanup when incremental midx fails
 4:  2290e27ded ! 4:  b113b3f012 midx-write: use uint32_t for preferred_pack_idx
     @@ Commit message
          For now, replace the use of -1 with a 'NO_PREFERRED_PACK' macro and an
          equality check. The macro stores the max value of a uint32_t, so we
          cannot store a preferred pack that appears last in a list of 2^32 total
     -    packs, but that's expected to be unreasonable already. This improves the
     -    range from 2^31 already.
     +    packs, but that's expected to be unreasonable already. Furthermore, with
     +    this change we end up extending the range from 2^31 possible packs to
     +    2^32-1.
      
          There are some careful things to worry about with initializing the
          preferred pack in the struct and using that value when searching for a
 5:  35302f5228 ! 5:  7c68f2535c midx-write: reenable signed comparison errors
     @@ midx-write.c: static int write_midx_internal(struct repository *r, const char *o
       	 */
       
      +	if (ctx.num_multi_pack_indexes_before == UINT32_MAX)
     -+		die("too many multi-pack-indexes");
     ++		die(_("too many multi-pack-indexes"));
      +
       	CALLOC_ARRAY(keep_hashes, ctx.num_multi_pack_indexes_before + 1);
       
 6:  7be25cf534 = 6:  224be4ee5c midx-write: simplify error cases
-- 
gitgitgadget
Previous: Junio C HamanoNext: Derrick Stolee via GitGitGadget
Message 37 of 47 in “midx-write: fix segfault and do several cleanups”
  1. 0/5 midx-write: fix segfault and do several cleanupsDerrick Stolee via GitGitGadget, Aug 28, 2025
  2. 1/5 midx-write: only load initialized packsDerrick Stolee via GitGitGadget, Aug 28, 2025
  3. Junio C HamanoAug 28, 2025
  4. Taylor BlauAug 29, 2025
  5. Derrick StoleeAug 30, 2025
  6. 2/5 midx-write: put failing response value backDerrick Stolee via GitGitGadget, Aug 28, 2025
  7. Junio C HamanoAug 28, 2025
  8. Taylor BlauAug 29, 2025
  9. 3/5 midx-write: use cleanup when incremental midx failsDerrick Stolee via GitGitGadget, Aug 28, 2025
  10. Junio C HamanoAug 28, 2025
  11. Taylor BlauAug 29, 2025
  12. Derrick StoleeAug 30, 2025
  13. 4/5 midx-write: use uint32_t for preferred_pack_idxDerrick Stolee via GitGitGadget, Aug 28, 2025
  14. Junio C HamanoAug 28, 2025
  15. Taylor BlauAug 29, 2025
  16. 5/5 midx-write: reenable signed comparison errorsDerrick Stolee via GitGitGadget, Aug 28, 2025
  17. Junio C HamanoAug 28, 2025
  18. Taylor BlauAug 29, 2025
  19. Taylor BlauAug 29, 2025
  20. 0/6 midx-write: fix segfault and do several cleanupsDerrick Stolee via GitGitGadget, Aug 30, 2025
  21. 1/6 midx-write: only load initialized packsDerrick Stolee via GitGitGadget, Aug 30, 2025
  22. Patrick SteinhardtSep 3, 2025
  23. Derrick StoleeSep 5, 2025
  24. 2/6 midx-write: put failing response value backDerrick Stolee via GitGitGadget, Aug 30, 2025
  25. Patrick SteinhardtSep 3, 2025
  26. Derrick StoleeSep 5, 2025
  27. 3/6 midx-write: use cleanup when incremental midx failsDerrick Stolee via GitGitGadget, Aug 30, 2025
  28. Patrick SteinhardtSep 3, 2025
  29. 4/6 midx-write: use uint32_t for preferred_pack_idxDerrick Stolee via GitGitGadget, Aug 30, 2025
  30. Patrick SteinhardtSep 3, 2025
  31. Derrick StoleeSep 5, 2025
  32. 5/6 midx-write: reenable signed comparison errorsDerrick Stolee via GitGitGadget, Aug 30, 2025
  33. Patrick SteinhardtSep 3, 2025
  34. 6/6 midx-write: simplify error casesDerrick Stolee via GitGitGadget, Aug 30, 2025
  35. Patrick SteinhardtSep 3, 2025
  36. Junio C HamanoSep 3, 2025
  37. 0/6 midx-write: fix segfault and do several cleanupsDerrick Stolee via GitGitGadget, Sep 5, 2025
  38. 1/6 midx-write: only load initialized packsDerrick Stolee via GitGitGadget, Sep 5, 2025
  39. 2/6 midx-write: put failing response value backDerrick Stolee via GitGitGadget, Sep 5, 2025
  40. 3/6 midx-write: use cleanup when incremental midx failsDerrick Stolee via GitGitGadget, Sep 5, 2025
  41. 4/6 midx-write: use uint32_t for preferred_pack_idxDerrick Stolee via GitGitGadget, Sep 5, 2025
  42. 5/6 midx-write: reenable signed comparison errorsDerrick Stolee via GitGitGadget, Sep 5, 2025
  43. 6/6 midx-write: simplify error casesDerrick Stolee via GitGitGadget, Sep 5, 2025
  44. Junio C HamanoSep 5, 2025
  45. Derrick StoleeSep 5, 2025
  46. Taylor BlauSep 11, 2025
  47. Junio C HamanoSep 11, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.