Commit 3ff0e3b5c310bdd3e8aa7c474852ed0b2c72770d
2019-02-15T11:16:05
refdb_fs: remove ordering dependency on loose/packed refs loading Right now, loading loose refs has the side-effect of setting the `PACKREF_SHADOWED` flag for references that exist both in the loose and the packed refs. Because of this, we are force do first look up packed refs and only afterwards loading the packed refs. This is susceptible to a race, though, when refs are being repacked: when first loading the packed cache, then it may not yet have the migrated loose ref. But when now trying to look up the loose reference afterwards, then it may already have been migrated. Thus, we would fail to find this reference in this scenario. Remove this ordering dependency to allow fixing the above race. Instead of setting the flag when loading loose refs, we will now instead set it lazily when iterating over the loose refs. This even has the added benefit of not requiring us to lock the packed refs cache, as we already have an owned copy of it.
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 cf4231d..b805093 100644
--- a/src/refdb_fs.c
+++ b/src/refdb_fs.c
@@ -564,7 +564,6 @@ static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter)
while (!error && !git_iterator_advance(&entry, fsit)) {
const char *ref_name;
- struct packref *ref;
char *ref_dup;
git_buf_truncate(&path, ref_prefix_len);
@@ -575,12 +574,6 @@ static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter)
(iter->glob && p_fnmatch(iter->glob, ref_name, 0) != 0))
continue;
- git_sortedcache_rlock(backend->refcache);
- ref = git_sortedcache_lookup(backend->refcache, ref_name);
- if (ref)
- ref->flags |= PACKREF_SHADOWED;
- git_sortedcache_runlock(backend->refcache);
-
ref_dup = git_pool_strdup(&iter->pool, ref_name);
if (!ref_dup)
error = -1;
@@ -605,8 +598,13 @@ static int refdb_fs_backend__iterator_next(
while (iter->loose_pos < iter->loose.length) {
const char *path = git_vector_get(&iter->loose, iter->loose_pos++);
- if (loose_lookup(out, backend, path) == 0)
+ if (loose_lookup(out, backend, path) == 0) {
+ ref = git_sortedcache_lookup(iter->cache, path);
+ if (ref)
+ ref->flags |= PACKREF_SHADOWED;
+
return 0;
+ }
git_error_clear();
}
@@ -640,8 +638,13 @@ static int refdb_fs_backend__iterator_next_name(
while (iter->loose_pos < iter->loose.length) {
const char *path = git_vector_get(&iter->loose, iter->loose_pos++);
+ struct packref *ref;
if (loose_lookup(NULL, backend, path) == 0) {
+ ref = git_sortedcache_lookup(iter->cache, path);
+ if (ref)
+ ref->flags |= PACKREF_SHADOWED;
+
*out = path;
return 0;
}