Re: unexpected auto-maintenance, was Re: git hogs the CPU, RAM and storage despite its config

[Jeff King <peff@peff.net>]

On Fri, May 08, 2026 at 02:03:41PM -0400, Jeff King wrote:

> Also, should background maintenance be locking to avoid multiple runs?
> It does not seem to do so, and if I run:
> 
>   git init
>   for i in $(seq 10000); do
>     echo $i >>file
>     git add file
>     git commit -m "commit $i"
>   done
> 
> I get several concurrent pack-objects processes. After a few thousand
> commits I got bored and hit ^C, and the resulting repo was corrupt!
> Which is not too surprising, as multiple simultaneous repacks are known
> to be unsafe, but means we should probably avoid them.

There is a pretty awful bug here, and it got much worse in v2.54.

First, the locking for "git maintenance run --detach" is totally broken.
We take the lock as the first step of maintenance_run_tasks(), then run
foreground tasks, then daemonize(), and then run background tasks. But
daemonize() forks and exits the parent process, leaving the child
process to do the real work. But the lock subsystem registers the
lockfile as a tempfile to be cleaned up at exit. So the moment we
daemonize, we release the lock, even though we're still going to run
sub-commands that should happen under lock.

I don't think this affected the old "git gc --detach" because it takes
the lock after daemonizing[1]. We can't do the same here, though, since we
need to hold the lock for the foreground tasks. So either we need to
release and re-take the lock between foreground and background tasks, or
we need to teach the daemonize() function to update the "owner" field on
all of the tempfiles to the new child[2].

AFAICT this has been broken since a6affd3343 (builtin/maintenance: add a
`--detach` flag, 2024-08-16). But it was mostly harmless because under
the hood we were running the "gc" task, which ran git-gc itself in
no-detach mode. So git-gc took its own lock, and we never ended up with
more than one repack running at a time.

But that changed in 452b12c2e0 (builtin/maintenance: use "geometric"
strategy by default, 2026-02-24), which is new in v2.54. Now we are
running repack directly, with no additional locking. So because of the
lock bug from a6affd3343, that means lots of repacks running at the same
time.

Ultimately fixing the lock bug will solve that. Though if doing so is
too complicated for a quick maint release, I'm tempted to say we should
consider reverting 452b12c2e0 for a potential v2.54.1 (as there were a
few other regression fixes so far, I assume we'll have one soon-ish).

-Peff

[1] git-gc does have a mode where it will take the lock and run some
    foreground tasks. That mode probably has the same bug, but I'm not
    sure if it existed before we switched to "maintenance run --auto".
    At this point it's moot.

[2] You can't just disable the atexit handler in the parent process
    here, because then the child wouldn't actually clean them up either
    (because its pid will not match the owner field). So we really need
    to transfer ownership during the daemonize() process, which probably
    needs a new tempfile.[ch] function to walk the list and update all
    of the owner fields.