Re: [PATCH] receive-pack: fix stale packfile locks when dying

From: Jeff King <peff@peff.net>
Date: Mar 10, 2023, 08:37 UTC
Message-ID: <ZArsRLHksmJ9Tgoy@coredump.intra.peff.net>
In-Reply-To: <ZArNJHMmPd8QCLwQ@ncase>

On Fri, Mar 10, 2023 at 07:24:36AM +0100, Patrick Steinhardt wrote:

Show 14 quoted lines

> > I think one could also make an argument that index_pack_lockfile()
> > should return a tempfile struct itself, but I didn't look too closely at
> > the other caller on the fetch side (but it should be conceptually the
> > same).
> 
> I had a look at it, but git-fetch-pack(1) works quite differently in
> that regard as it also supports the case where the packfile lock should
> stay locked after it exits via the `--keep` switch. So the logic is more
> intricate here.
> 
> Furthermore, git-fetch-pack(1) only does the locking, but never unlocks
> the packfiles. That is instead handled by git-fetch(1). So converting
> the interface to use tempfiles directly wouldn't work as we are crossing
> process boundaries here.

I think the calls into fetch-pack.c that handle the pack lockfiles can happen in-process from git-fetch itself. But I also think there are probably cases where they don't (v0 git-over-http should use a separate "fetch-pack --stateless-rpc", I believe).

So yeah, it's probably too complicated to worry about lumping in here, especially since you noted that it handles cleanup correctly already.

Thanks for looking into it.

-Peff