Re: [PATCH 1/2] repository: add core.indexLockTimeout
flat view
From: Junio C Hamano <hidden>
Date: 2026-10-13 22:05:55
Jun Wu [off-list ref] writes:
From: Jun Wu <redacted>
When an IDE runs `git status` periodically in the background, `status`
can take the index lock, and this prevents other commands like `add`,
`commit` from running.
Commit 27344d6a6c (git: add --no-optional-locks option, 2017-09-27)
avoids the conflict by letting background `git --no-optional-locks status`
skip the index lock, at the cost of losing refreshed stat info, so
the next `status` has to re-hash files.
Approach the conflict from the other side: let non-status commands wait
for the index lock. Mercurial and Sapling have a similar "waiting for wlock"
behavior, and Git already has `core.{config,filesRef}LockTimeout`.
Ideally all non-status commands can wait for the index lock. However,
some code paths (e.g. `commit`) load the index without locking, then lock and
write changes. It is already racy and the wait behavior would make it much
worse. As a defense, waiting is disabled if index was already read.
Default timeout is 1s, effective but not too long to avoid breaking potential
user expectation (e.g. "an index.lock error means I should check whether a
commit message editor is open somewhere"). Tests that are slowed down by the
default timeout now set the timeout to 0.Need to change the existing tests is a sign that end-user workflows that are emulated by them will be impacted, and we'd need to be careful. I am not sure if the change is making the right trade-off between background tasks and end-user action that expects immediate response.
quoted hunk ↗ jump to hunk
@@ -473,7 +473,22 @@ int repo_hold_locked_index(struct repository *repo, struct lock_file *lf, int flags) { + int timeout_ms; + if (!repo->index_file) BUG("the repo hasn't been setup"); - return repo_hold_lock_file_for_update(repo, lf, repo->index_file, flags); + + if (repo->index->initialized) { + /* + * Index was initialized (read) without lock. To minimize the + * chance of lost update, do not wait. + */ + timeout_ms = 0; + } else { + timeout_ms = 1000; + repo_config_get_int(repo, "core.indexlocktimeout", &timeout_ms); + } + + return repo_hold_lock_file_for_update_timeout(repo, lf, repo->index_file, + flags, timeout_ms); }
A few comments and/or questions..
(1) In the unhappy case where the index was read already and then
we try to take the lock, what should happen when we notice that
the index is locked and better yet if we notice that the index
has been updated before we write the in-core index we wanted to
write out? The question is what "should" happen, not "what
happens with the current code".
(2) Are there cases where we take lock only to make sure nobody
will update the index without any intention to write out the
contents of our in-core index? IOW, we are holding the lock
only to have a consistent world view while performing a
read-only operation? How would this change affect it?
(3) When 'repo->index->initialized' is true, does it always mean we
have read the on-disk index already, or are there cases where
we created the in-core index from scratch and called this
function with the intention to write over it? For such a use
case, what, if any, are the implications of this change?