refdb_fs: do not lazily copy packed ref cache

When creating a new iterator, we eagerly load loose refs but only lazily create
a copy of packed refs. The lazy load only happens as soon as we have iterated
over all loose refs, opening up a potentially wide window for races. This
may lead to an inconsistent view e.g. when the caller decides to reload packed
references somewhen between iterating the loose refs, which is unexpected.

Fix the issue by eagerly copying the sorted cache. Note that right now, we are
heavily dependent on ordering here: we first need to reload packed refs, then we
have to load loose refs and only as a last step are we allowed to copy the
cache. This is because loading loose refs has the side-effect of setting the
`PACKED_SHADOWED` flag in the packed refs cache, which we require to avoid
outputting packed refs that already exist as loose refs.

1 files changed, 6 insertions, 13 deletions

diff --git a/src/refdb_fs.c b/src/refdb_fs.c
index c7df6f20a..cf4231d65 100644
--- a/src/refdb_fs.c
+++ b/src/refdb_fs.c
@@ -611,11 +611,6 @@ static int refdb_fs_backend__iterator_next(
 		git_error_clear();
 	}
 
-	if (!iter->cache) {
-		if ((error = git_sortedcache_copy(&iter->cache, backend->refcache, 1, NULL, NULL)) < 0)
-			return error;
-	}
-
 	error = GIT_ITEROVER;
 	while (iter->packed_pos < git_sortedcache_entrycount(iter->cache)) {
 		ref = git_sortedcache_entry(iter->cache, iter->packed_pos++);
@@ -654,11 +649,6 @@ static int refdb_fs_backend__iterator_next_name(
 		git_error_clear();
 	}
 
-	if (!iter->cache) {
-		if ((error = git_sortedcache_copy(&iter->cache, backend->refcache, 1, NULL, NULL)) < 0)
-			return error;
-	}
-
 	error = GIT_ITEROVER;
 	while (iter->packed_pos < git_sortedcache_entrycount(iter->cache)) {
 		ref = git_sortedcache_entry(iter->cache, iter->packed_pos++);
@@ -687,9 +677,6 @@ static int refdb_fs_backend__iterator(
 
 	assert(backend);
 
-	if ((error = packed_reload(backend)) < 0)
-		goto out;
-
 	iter = git__calloc(1, sizeof(refdb_fs_iter));
 	GIT_ERROR_CHECK_ALLOC(iter);
 
@@ -704,9 +691,15 @@ static int refdb_fs_backend__iterator(
 		goto out;
 	}
 
+	if ((error = packed_reload(backend)) < 0)
+		goto out;
+
 	if ((error = iter_load_loose_paths(backend, iter)) < 0)
 		goto out;
 
+	if ((error = git_sortedcache_copy(&iter->cache, backend->refcache, 1, NULL, NULL)) < 0)
+		goto out;
+
 	iter->parent.next = refdb_fs_backend__iterator_next;
 	iter->parent.next_name = refdb_fs_backend__iterator_next_name;
 	iter->parent.free = refdb_fs_backend__iterator_free;