From: Jeff King <peff@peff.net>
Date: Mon, 11 May 2026 20:10:49 GMT
Subject: Re: unexpected auto-maintenance, was Re: git hogs the CPU, RAM and storage despite its config
Message-ID: <20260511201049.GB22912@coredump.intra.peff.net>
In-Reply-To: <9ddfd37d-7d71-4359-b9be-d993fbfd138c@gmail.com>

On Sun, May 10, 2026 at 12:08:14PM -0400, Derrick Stolee wrote:

> > So in that sense I would prefer to "fix forward" here rather than to
> > mask over the bug. But even the relatively short diff above is not so
> > straightforward to reason through, review, or test, so I'm open to other
> > ideas on how to proceed here.
> 
> I initially worried about cross-platform support, thinking that we
> needed to pass file descriptors / handles and Windows always has
> issues with file handles. But we aren't actually keeping a handle
> open but instead a record that we created the lock and should delete
> it when everything resolves.

The daemonize() function is a noop on Windows anyway, because we don't
have fork(). It's controlled by the NO_POSIX_GOODIES knob.

> For me to be convinced that this forward fix is the right direction,
> I'd need to see a test that proves the detached process will clean up
> the locks on a normal process end and an early exit.

I think it's hard to trigger an early exit, since the only thing we do
while holding the lock is run the maintenance tasks, and those are
almost entirely just using run-command to run subprocesses. So I don't
see a single path that can lead to a die().

Which leaves signals. It is easy-ish to test manually with a
long-running maintenance (say, the "gc" task on linux.git), verifying
that maintenance.lock is there, killing the detached git-maintenance
process with SIGTERM, and then confirming that the lock was cleaned up.

It looks like Taylor found a way to make an arbitrarily slow task using
the prefetch process. So I think his test could be modified to issue a
kill at the right moment and verify cleanup (rather than waiting for a
normal exit, which calls rollback_lock_file() and never checks the owner
field at all).

-Peff