Re: Bug: fatal: Unable to create '.../.git/index.lock': File exists.

From: Jeff King <peff@peff.net>
Date: May 2, 2019, 16:58 UTC
Message-ID: <20190502165802.GA19341@sigill.intra.peff.net>
In-Reply-To: <CACsJy8Dimn9+ogDNEgy3xmLunyX_pStBq=g-1jrf74LsOW1xrA@mail.gmail.com>

On Thu, May 02, 2019 at 11:38:51PM +0700, Duy Nguyen wrote:

Show 10 quoted lines

> > Since the decision of whether to use the locks is dependent on the
> > operation being performed, it's an environment variable and not a config
> > option.
> 
> And there's also tradeoff for doing it. If git-status will not take
> locks, it cannot update the index to save refresh information and
> reuse the next time. git-status may become more and more expensive
> over time (*). Setting a config variable for this does not sound like
> a good idea at all. The same for setting GIT_OPTIONAL_LOCKS=0 in
> ~/.bashrc to "fix" the problem once and for all.

Right. I suspect in the long run it might not be _too_ bad to run with such a setting, because any non-read operations would eventually refresh the index (and as you note even many read-only operations like porcelain git-diff unconditionally refresh for now).

But I agree it's not really the direction we want to go.

> I might take a stab at the "wait and try to hold the lock again, doing
> necessary verification after if needed" idea. It sounds like the right
> way to go and we haven't had problems with refs doing the same thing
> (have we?).

No, but it's a bit easier with refs because the locking is just atomically checking the lease. I.e., after taking the lock we still say "we expected the ref to be at oid XYZ, is it still there?". What's the equivalent for an index operation?

I think it is more common with the index to take the lock, then while holding it read it in fresh (possibly dumping old results), manipulate the result, and then write it out. For callers which make sure to get a fresh view _after_ taking the lock, they should be OK if taking the lock is delayed.

I guess arguably any callers that aren't that careful are already broken, since it is a race; any delay-and-retry _could_ have happened as "we were too slow to see the initial lock".

-Peff