Commit 833338144a6fd8b639b22a2d25b9cba9c767163c
2019-02-15T10:56:50
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 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54
diff --git a/src/refdb_fs.c b/src/refdb_fs.c
index c7df6f2..cf4231d 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;