Re: [PATCH 2/2] commit: lock index before reading

[Jun Wu <quark.zju@gmail.com>]

Hi Peff,

On Wed, 14 Oct 2026 06:04:32 -0400, Jeff King wrote:
> I do think your patch fixes the problem. But we probably want to discuss
> that in the commit message as a motivation (and likewise, discuss the
> fact that we may be holding the lock for longer).

Will do in v2: describe the race as motivation, and mention the longer hold
trade-off.

I'd like to note 'commit' is just one command with the read/lock order issue.
There are many others, like: reset, rebase, stash, am, merge, etc.
Some are easy to fix, some seem tricky (e.g. merge).

Do you think that I should fix all order issues before adding the timeout?

Note Patch 1 alone does not hurt - if a command has the read/lock order issue
and is therefore racy, then the timeout feature is automatically disabled
(by the 'initialized' check) so the racy command behaves exactly the same as
before, not worse.

My plan is to add the timeout support first, then incrementally fix order
issues, starting from the easy/more common ones (e.g. commit).

> There's probably some kind of hybrid world where we optimistically read
> without the lock, then take the lock, and then check (cheaply-ish) that
> the index didn't change since what we read. But I don't think we do that
> in this code path.

For reference, 'status' is such an example, although its write is optional.
'add -i' could be another example - it avoids locking when waiting for user
input, although it lacks the "didn't change" check right now.

-Jun