Merge branch 'mh/ref-lock-entry'

The code to acquire a lock on a reference (e.g. while accepting a
push from a client) used to immediately fail when the reference is
already locked---now it waits for a very short while and retries,
which can make it succeed if the lock holder was holding it during
a read-only operation.

* mh/ref-lock-entry:
  refs: retry acquiring reference locks for 100ms

4 files changed, 39 insertions, 5 deletions

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 9b42e0ca73..dc4e3f58a2 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -776,6 +776,12 @@ core.commentChar::
 If set to "auto", `git-commit` would select a character that is not
 the beginning character of any line in existing commit messages.
 
+core.filesRefLockTimeout::
+	The length of time, in milliseconds, to retry when trying to
+	lock an individual reference. Value 0 means not to retry at
+	all; -1 means to try indefinitely. Default is 100 (i.e.,
+	retry for 100ms).
+
 core.packedRefsTimeout::
 	The length of time, in milliseconds, to retry when trying to
 	lock the `packed-refs` file. Value 0 means not to retry at
diff --git a/refs.c b/refs.c
index 3d549a8970..b0106b8162 100644
--- a/refs.c
+++ b/refs.c
@@ -579,6 +579,21 @@ enum ref_type ref_type(const char *refname)
        return REF_TYPE_NORMAL;
 }
 
+long get_files_ref_lock_timeout_ms(void)
+{
+	static int configured = 0;
+
+	/* The default timeout is 100 ms: */
+	static int timeout_ms = 100;
+
+	if (!configured) {
+		git_config_get_int("core.filesreflocktimeout", &timeout_ms);
+		configured = 1;
+	}
+
+	return timeout_ms;
+}
+
 static int write_pseudoref(const char *pseudoref, const unsigned char *sha1,
 			   const unsigned char *old_sha1, struct strbuf *err)
 {
@@ -591,7 +606,9 @@ static int write_pseudoref(const char *pseudoref, const unsigned char *sha1,
 	strbuf_addf(&buf, "%s\n", sha1_to_hex(sha1));
 
 	filename = git_path("%s", pseudoref);
-	fd = hold_lock_file_for_update(&lock, filename, LOCK_DIE_ON_ERROR);
+	fd = hold_lock_file_for_update_timeout(&lock, filename,
+					       LOCK_DIE_ON_ERROR,
+					       get_files_ref_lock_timeout_ms());
 	if (fd < 0) {
 		strbuf_addf(err, "could not open '%s' for writing: %s",
 			    filename, strerror(errno));
@@ -634,8 +651,9 @@ static int delete_pseudoref(const char *pseudoref, const unsigned char *old_sha1
 		int fd;
 		unsigned char actual_old_sha1[20];
 
-		fd = hold_lock_file_for_update(&lock, filename,
-					       LOCK_DIE_ON_ERROR);
+		fd = hold_lock_file_for_update_timeout(
+				&lock, filename, LOCK_DIE_ON_ERROR,
+				get_files_ref_lock_timeout_ms());
 		if (fd < 0)
 			die_errno(_("Could not open '%s' for writing"), filename);
 		if (read_ref(pseudoref, actual_old_sha1))
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 5cca55510b..fccbc24ac4 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -537,7 +537,9 @@ retry:
 	if (!lock->lk)
 		lock->lk = xcalloc(1, sizeof(struct lock_file));
 
-	if (hold_lock_file_for_update(lock->lk, ref_file.buf, LOCK_NO_DEREF) < 0) {
+	if (hold_lock_file_for_update_timeout(
+			    lock->lk, ref_file.buf, LOCK_NO_DEREF,
+			    get_files_ref_lock_timeout_ms()) < 0) {
 		if (errno == ENOENT && --attempts_remaining > 0) {
 			/*
 			 * Maybe somebody just deleted one of the
@@ -865,7 +867,9 @@ static int create_reflock(const char *path, void *cb)
 {
 	struct lock_file *lk = cb;
 
-	return hold_lock_file_for_update(lk, path, LOCK_NO_DEREF) < 0 ? -1 : 0;
+	return hold_lock_file_for_update_timeout(
+			lk, path, LOCK_NO_DEREF,
+			get_files_ref_lock_timeout_ms()) < 0 ? -1 : 0;
 }
 
 /*
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 4789106fc0..b02dc5a7e3 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -62,6 +62,12 @@
 #define REF_DELETED_LOOSE 0x200
 
 /*
+ * Return the length of time to retry acquiring a loose reference lock
+ * before giving up, in milliseconds:
+ */
+long get_files_ref_lock_timeout_ms(void);
+
+/*
  * Return true iff refname is minimally safe. "Safe" here means that
  * deleting a loose reference by this name will not do any damage, for
  * example by causing a file that is not a reference to be deleted.