Re: [PATCH 1/8] t: fix races caused by background maintenance

On 10.08.26 10:35, Patrick Steinhardt wrote:

I had my friendly AI agent dig into this more, and below is what it came up with. Disclaimer: I only skimmed it, and didn't check it for accuracy; I'm way too unfamiliar with any of this stuff. I hope this is of some use.

----------

Subject: [BUG] geometric-repack --auto fires on tiny repos, and the
 resulting background repacks race concurrent commands

Since v2.54.0, `git commit` in an ordinary small repository can leave a detached `git repack` running behind it, and it does so far more often than intended: the auto condition that is documented as "at least 100 loose objects" is in practice satisfied by a repository containing four objects. The background repacks that result break concurrent git commands at a rate of roughly one in a thousand commits in my testing.

There are two separate issues here. The first is a plain bug; the second is its fallout.

1. The geometric-repack auto condition triggers ~256x too eagerly
----------------------------------------------------------------

Documentation/config/maintenance.adoc says:

    maintenance.geometric-repack.auto::
            [...] a positive value implies the command should run either
            when there are packfiles that need to be merged together to
            retain the geometric progression, or when there are at least
            this many loose objects that would be written into a new
            packfile. The default value is 100.

Reproducer, on a repository with four objects (v2.54.0 and v2.55.0):

    git init repro && cd repro
    git config maintenance.auto false   # so that the setup itself does
                                        # not fork any maintenance
    echo 263 >a.txt                     # blob 175b6c5d...
    echo 410 >b.txt                     # blob 17e344e7...
    git add . && git commit -m initial

    $ find .git/objects -type f
    .git/objects/17/5b6c5dfd7f9bf6e2b2c4e2dcf3e2341298575d
    .git/objects/17/e344e7c08441fa81d5b56c21008dc0feeeaa20
    .git/objects/70/a9a72c46fc85c654e626f54e77c4946950da52
    .git/objects/7f/174478a536fc641c735e6b6e1a1944c868ba0c

    $ git maintenance run --auto --no-detach
    $ ls .git/objects/pack/
    multi-pack-index
    pack-f826a2dd....idx
    pack-f826a2dd....pack
    pack-f826a2dd....rev

(only the two blob IDs are fixed; the tree and commit depend on the commit timestamp.) Committing only a.txt -- three objects, one of them in objects/17 -- does not trigger the repack. Adding the second blob whose object ID starts with "17" does. That is the whole condition.

geometric_repack_auto_condition() (builtin/gc.c) passes its threshold to too_many_loose_objects(), which does not count loose objects: it counts the entries of .git/objects/17 and scales by 256. In v2.54.0:

    int auto_threshold = DIV_ROUND_UP(limit, 256);
    [...]
            if (++num_loose > auto_threshold) {

and equivalently after the rewrite in v2.55.0:

    /*
     * This is weird, but stems from legacy behaviour: [...]
     */
    int auto_threshold = DIV_ROUND_UP(limit, 256) * 256;
    [...]
    return loose_count > auto_threshold;

with loose_count coming from ODB_COUNT_OBJECTS_APPROXIMATE, i.e. the same one-directory estimate. Either way, any limit <= 256 collapses to "two or more objects share the objects/17 directory".

That estimator is fine for gc.auto, whose default of 6700 needs 27 entries in that directory -- a number you only reach with thousands of objects, and whose documentation says "approximately". It falls apart for a threshold below 256, where the smallest representable estimate step exceeds the threshold itself and a single fanout collision decides the outcome. For a repository with n objects the condition is satisfied with probability ~1-(1-p)^n-np(1-p)^(n-1), p=1/256: about 5% at 90 objects, and much higher for repositories that accumulate objects over time.

By contrast maintenance.loose-objects.auto, documented in the same terms, is implemented as an exact count in loose_object_auto_condition().

Since 452b12c2e0 (builtin/maintenance: use "geometric" strategy by default, 2026-02-24) this condition governs what `git maintenance run --auto` does, and run_auto_maintenance() invokes that from commit, am, fetch, merge and rebase, with --detach unless gc.autoDetach says otherwise. So the practical effect is: once any two objects in a repository collide in objects/17 -- and the condition only becomes easier to satisfy as the repository grows -- every subsequent commit forks a detached repack.

Measured on a fixture repository built by 40 commits of "write a file, git add, git commit": a background repack fired in 4 of 25 runs (the prediction from the 40 nondeterministic commit object IDs alone is 1-(255/256)^40 = 14%).

2. The resulting background repacks break concurrent commands
-------------------------------------------------------------

`git repack -d` installs the new pack, removes the redundant ones and then calls prune_packed_objects(), which unlinks the loose copies of objects that are now packed and rmdir()s the fanout directories it empties. Doing that concurrently with unrelated git processes in the same repository is new exposure: before v2.54.0 the same repositories never reached the gc.auto threshold and no such repack ever ran.

Stress test: 24 concurrent workers, each repeatedly creating a fresh repository and running 40 iterations of "write a file, git add, git commit", with maintenance.geometric-repack.auto=-1 so that the condition fires on every commit rather than in ~15% of repositories. Nothing else touches the repositories; the only concurrency is git's own detached auto-maintenance.

109280 commits produced 97 failing commands, i.e. one per ~1100 commits. Message lines across those 97 failures (one failure often emits two of them, e.g. a temporary-file error followed by a fatal):

      71  error: packfile .../pack-....pack index unavailable
      50  fatal: failed to write commit object
      37  error: unable to create temporary file: Invalid argument
      23  error: unable to create temporary file: No such file or directory
      10  error: unable to index file '...'
      10  fatal: adding files failed

At least the ENOENT variant has a straightforward explanation:

  - The writer, in create_tmpfile() (object-file.c), calls
    mkstemp(".git/objects/XX/tmp_obj_XXXXXX").  On ENOENT it mkdir()s
    the fanout directory and retries -- exactly once.

  - The repack, in prune_subdir() (prune-packed.c), rmdir()s each fanout
    directory after pruning its contents.

An rmdir() landing between the writer's mkdir() and its retried mkstemp() defeats the single retry, and the command aborts. The writer retries a bounded number of times against interference that is not bounded.

(For anyone hitting this meanwhile: setting maintenance.auto=false makes it go away, since it stops the fork at the source. We have done that in lazygit's test configuration.)

3. Where this came from
-----------------------

lazygit's integration tests started failing intermittently on CI, always and only in the job running the newest git (2.54.0 on GitHub Actions ubuntu-latest), never in the jobs pinned to 2.32.0/2.38.2/2.44.0, and always during test fixture setup, which is a plain loop of "write a file, git add, git commit":

    error: invalid object 100644 50d561270fcfcdc9afc85f6136f937c529accaaa for 'file09.txt'
    error: invalid object 100644 50d561270fcfcdc9afc85f6136f937c529accaaa for 'file09.txt'
    error: Error building trees

(the first message really is printed twice), i.e. the cache-tree existence check in update_one() (cache-tree.c) failing to find a blob that a previous iteration had successfully committed. The hash is the correct blob for the file's content, so `git add` had staged it fine.

Those fixture repositories are exactly the case in section 1: they contain one object in objects/17 by construction, so a single commit object hashing into the same directory tips them over, and every following commit forks a repack. The affected tests were, without exception, the ones whose fixtures build six or more commits.

I could not reproduce this specific failure outside Linux, and I have not isolated the window that produces it. A fresh process is robust (7200 lookups against a repack loop, no misses), and so is a `git commit` whose repack completes mid-flight -- the flag the cache-tree check passes, ODB_HAS_OBJECT_RECHECK_PACKED, makes a missing lookup reprepare the pack list and retry, and that recovery does work in everything I could construct deliberately. So I am reporting this as the symptom that led to the above rather than as a diagnosed bug, in case it is familiar to someone.

One observation from reading that retry path, which may or may not be related: packfile_store_reprepare() refreshes the pack list but never reloads the multi-pack-index, because prepare_multi_pack_index_one() (midx.c) returns early when one is already loaded. A long-lived process therefore keeps a midx that may reference packs a concurrent repack has since deleted, for its entire lifetime.

Environment -----------

Original failures: git 2.54.0, GitHub Actions ubuntu-latest. Reproducers and measurements above: git 2.55.0 (built from source), macOS 26, APFS. Code references are to v2.54.0 unless stated; I checked that the behaviour is unchanged in v2.55.0.

Thanks, Stefan