Commit 8c77343871c1d8c6a2cc1149143d3577c0ccfc43

Patrick Steinhardt 2019-02-15T10:15:39

refdb_fs: fix potential race with ref repacking in `exists` callback When repacking references, git.git will first update the packed refs and only afterwards delete any existing loose references that have now been moved to the new packed refs file. Due to this, there is a potential for racing if one first reads the packfile (which has not been updated yet) and only then trying to read the loose reference (which has just been deleted). In this case, one will incorrectly fail to lookup the reference and it will be reported as missing. Naturally, this is exactly what we've been doing in `refdb_fs_backend__exists`. Fix the race by reversing the lookup: we will now first check if the loose reference exists and only afterwards refresh the packed file.

diff --git a/src/refdb_fs.c b/src/refdb_fs.c
index d582cb8..7d987b4 100644
--- a/src/refdb_fs.c
+++ b/src/refdb_fs.c
@@ -333,15 +333,27 @@ static int refdb_fs_backend__exists(
 
 	assert(backend);
 
-	if ((error = packed_reload(backend)) < 0 ||
-		(error = git_buf_joinpath(&ref_path, backend->gitpath, ref_name)) < 0)
-		return error;
+	*exists = 0;
+
+	if ((error = git_buf_joinpath(&ref_path, backend->gitpath, ref_name)) < 0)
+		goto out;
+
+	if (git_path_isfile(ref_path.ptr)) {
+		*exists = 1;
+		goto out;
+	}
+
+	if ((error = packed_reload(backend)) < 0)
+		goto out;
 
-	*exists = git_path_isfile(ref_path.ptr) ||
-		(git_sortedcache_lookup(backend->refcache, ref_name) != NULL);
+	if (git_sortedcache_lookup(backend->refcache, ref_name) != NULL) {
+		*exists = 1;
+		goto out;
+	}
 
+out:
 	git_buf_dispose(&ref_path);
-	return 0;
+	return error;
 }
 
 static const char *loose_parse_symbolic(git_buf *file_content)