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

[Jeff King <peff@peff.net>]

On Sat, May 09, 2026 at 05:52:29PM -0400, Taylor Blau wrote:

> Dropping and re-acquiring the lock is possible, but it's racy since
> there is a gap during the critical window while we fork(). I believe
> that the only airtight way to do this is to update the owner field of
> the tempfiles we want to pass down during daemonization.

It's racy in the sense that we might not get the lock back, but I don't
think that's _wrong_ exactly. We just care that some instance runs the
second half eventually.

It is technically vulnerable to starvation, if processes keep taking the
lock, doing the foreground jobs, and then losing the race to take it a
second time. But besides the fact that you'd eventually win by chance,
unless you have an infinite supply of processes coming along to run the
foreground half, the final invocation will run the background tasks.

I agree it feels pretty hacky. It is, however, how "git gc" seems to
work (notice that it does a manual delete_tempfile() on the lock right
before daemonizing).

> (As an aside, do we want to do that for all tempfiles? It might be nice
> to have a "->reassign_on_fork" flag or something on the tempfile struct
> in case there are instances where the parent wants to retain ownership
> of the tempfile after fork()-ing, but I can't think of any off the top
> of my head. If we do introduce such a field, it should probably default
> to "true" to avoid any foot-guns.)

I think the answer is yes, we want it for all tempfiles. Because it is
not a property of the individual tempfiles, but rather of the caller who
is forking. The point of that owner field is so that when we spawn
children, we don't end up with duplicate or unexpected cleanups.

But daemonize() is not about spawning a child or otherwise splitting the
process into two. There is still a single contiguous flow of execution,
and the fact that we switch pids and call exit() is an implementation
detail. So we would still want to remain the "owner" of any cleanup.

And that is true for any other atexit() handlers besides the tempfile
ones, too. If we called run_processes_parallel(), and then while that
child was running we called daemonize(), we'd not want to kill the child
then, but our atexit handler would do so! But we don't tend to leave
processes running while doing stuff like daemonize(), which is why we
never bothered to implement an owner field there in the first place.

> > 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).
> 
> I think something like the following (untested) would do the trick:

Yeah, that's what I was envisioning. Note that there's a small race
here:

> +	switch ((pid = fork())) {
>  		case 0:
> +			reassign_tempfile_ownership(ppid, getpid());
>  			break;
>  		case -1:
>  			die_errno(_("fork failed"));
>  		default:
> +			reassign_tempfile_ownership(ppid, pid);
>  			exit(0);
>  	}

The parent is giving up ownership and the child is taking it. So there
may be a moment where both claim ownership, or a moment where the parent
has relinquished but the child has not yet taken. We're not going to
call exit() in the middle there, but a signal could kill us.

And then either:

  - If both claim ownership, it's mostly OK, because they'll both try to
    unlink() which is idempotent-ish. Though there is a bad sequence
    where we delete somebody _else's_ lock, like:

       1. Parent forks, but has not yet reassigned.

       2. Child calls reassign to take ownership. Now both have
	  ownership.

       3. Signal kills both parent and child, so they enter cleanup
	  code.

       4. One of them (let's say the parent) deletes the lockfile.

       5. Some other unrelated process (let's call it "git other") takes
	  the lock.

       6. The child deletes the lockfile.

    And at that point "git other" thinks it holds the lock, but it
    doesn't. It's quite an unlikely sequence in practice, though, I'd
    think.

  - If neither claims ownership and a signal kills both, then nobody
    cleans up the lock and it is left in place. This is annoying, but
    also something that can happen occasionally anyway (kill -9, etc).

I don't have an easy suggestion for making it more atomic, though. You
could choose one or the other direction using some synchronization
between the two (e.g., child reassigns only after parent signals over a
pipe that it has relinquished), but it's all kind of ugly.

So it may be reasonable to just shrug and assume it's unlikely enough
not to worry about.

-Peff