config: retry acquiring config.lock for 100ms

When multiple processes write to a config file concurrently, they
contend on its ".lock" file, which is acquired via open(O_EXCL) with
no retry. The losers fail immediately with "could not lock config
file". Two processes writing unrelated keys (say, "branch.a.remote"
and "branch.b.remote") have no semantic conflict, yet one of them
fails for a purely mechanical reason.

This bites in practice when running `git worktree add -b` concurrently
against the same repository. Each invocation makes several writes to
".git/config" to set up branch tracking, and tooling that creates
worktrees in parallel sees intermittent failures. Worse, `git worktree
add` does not propagate the failed config write to its exit code: the
worktree is created and the command exits 0, but tracking
configuration is silently dropped.

The lock is held only for the duration of rewriting a small file, so
retrying for 100 ms papers over any realistic contention while still
failing fast if a stale lock has been left behind by a crashed
process. This mirrors what we already do for individual reference
locks (4ff0f01cb7 (refs: retry acquiring reference locks for 100ms,
2017-08-21)).

Signed-off-by: Jörg Thalheim <joerg@thalheim.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/config.c b/config.c
index 156f2a24fa00271c6d6adafac16527e674f5ccd4..f7aff8725de4a3cafa49267562ea620017bc3d31 100644 (file)
--- a/config.c
+++ b/config.c
@@ -2903,6 +2903,14 @@ char *git_config_prepare_comment_string(const char *comment)
        return prepared;
 }
 
+/*
+ * How long to retry acquiring config.lock when another process holds it.
+ * The lock is held only for the duration of rewriting a small file, so
+ * 100 ms covers any realistic contention while still failing fast if
+ * a stale lock has been left behind by a crashed process.
+ */
+#define CONFIG_LOCK_TIMEOUT_MS 100
+
 static void validate_comment_string(const char *comment)
 {
        size_t leading_blanks;
@@ -2986,7 +2994,8 @@ int repo_config_set_multivar_in_file_gently(struct repository *r,
         * The lock serves a purpose in addition to locking: the new
         * contents of .git/config will be written into it.
         */
-       fd = hold_lock_file_for_update(&lock, config_filename, 0);
+       fd = hold_lock_file_for_update_timeout(&lock, config_filename, 0,
+                                              CONFIG_LOCK_TIMEOUT_MS);
        if (fd < 0) {
                error_errno(_("could not lock config file %s"), config_filename);
                ret = CONFIG_NO_LOCK;
@@ -3331,7 +3340,8 @@ static int repo_config_copy_or_rename_section_in_file(
        if (!config_filename)
                config_filename = filename_buf = repo_git_path(r, "config");
 
-       out_fd = hold_lock_file_for_update(&lock, config_filename, 0);
+       out_fd = hold_lock_file_for_update_timeout(&lock, config_filename, 0,
+                                                  CONFIG_LOCK_TIMEOUT_MS);
        if (out_fd < 0) {
                ret = error(_("could not lock config file %s"), config_filename);
                goto out;