Re: [PATCH] lockfile: add PID file for debugging stale locks

From: Jeff King <peff@peff.net>
Date: Dec 5, 2025, 18:46 UTC
Message-ID: <20251205184648.GC33447@coredump.intra.peff.net>
In-Reply-To: <aTDFks3RW57Ytwvq@nand.local>

On Wed, Dec 03, 2025 at 06:19:46PM -0500, Taylor Blau wrote:

Show 5 quoted lines

> Changing the naming scheme as above would cause us to hold
> "foo.pid.lock" in addition to "foo.lock". That would allow process B
> here to write branch "foo.lock.pid" (as is the case today). But if the
> scenario were instead "process B wants to write branch foo.pid.lock", it
> would fail immediately since the ".lock" suffix is reserved.

I agree that this gets rid of any corruption or race issues, since all versions understand how to handle ".lock" specially (and assuming we create foo.pid.lock with O_EXCL). But there is still a namespace collision; I cannot write refs "foo" and "foo.pid" at the same time.

Show 7 quoted lines

> > So I dunno what that means for your patch. I notice that the user has to
> > enable the feature manually. But it feels more like it should be
> > selective based on which subsystem is using the lockfile (so refs would
> > never want it, but other lockfiles/tempfiles might).
> 
> Yeah, I think that something similar to the "which files do we fsync()
> and how?" configuration we have today would be a nice complement here.

I'm not sure it makes sense for the user to configure this. I more meant that the ref files-backend code would set a flag for "no, do not create a pid file for me ever" (or inversely, other bits of the code would add a flag for "yes, it's OK to do so").

-Peff