Skip to content

Commit 3ff0e3b

Browse files
committed
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 parent 8333381 commit 3ff0e3b

File tree

1 file changed

+11
-8
lines changed

1 file changed

+11
-8
lines changed

src/refdb_fs.c

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,6 @@ static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter)
564564

565565
while (!error && !git_iterator_advance(&entry, fsit)) {
566566
const char *ref_name;
567-
struct packref *ref;
568567
char *ref_dup;
569568

570569
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)
575574
(iter->glob && p_fnmatch(iter->glob, ref_name, 0) != 0))
576575
continue;
577576

578-
git_sortedcache_rlock(backend->refcache);
579-
ref = git_sortedcache_lookup(backend->refcache, ref_name);
580-
if (ref)
581-
ref->flags |= PACKREF_SHADOWED;
582-
git_sortedcache_runlock(backend->refcache);
583-
584577
ref_dup = git_pool_strdup(&iter->pool, ref_name);
585578
if (!ref_dup)
586579
error = -1;
@@ -605,8 +598,13 @@ static int refdb_fs_backend__iterator_next(
605598
while (iter->loose_pos < iter->loose.length) {
606599
const char *path = git_vector_get(&iter->loose, iter->loose_pos++);
607600

608-
if (loose_lookup(out, backend, path) == 0)
601+
if (loose_lookup(out, backend, path) == 0) {
602+
ref = git_sortedcache_lookup(iter->cache, path);
603+
if (ref)
604+
ref->flags |= PACKREF_SHADOWED;
605+
609606
return 0;
607+
}
610608

611609
git_error_clear();
612610
}
@@ -640,8 +638,13 @@ static int refdb_fs_backend__iterator_next_name(
640638

641639
while (iter->loose_pos < iter->loose.length) {
642640
const char *path = git_vector_get(&iter->loose, iter->loose_pos++);
641+
struct packref *ref;
643642

644643
if (loose_lookup(NULL, backend, path) == 0) {
644+
ref = git_sortedcache_lookup(iter->cache, path);
645+
if (ref)
646+
ref->flags |= PACKREF_SHADOWED;
647+
645648
*out = path;
646649
return 0;
647650
}

0 commit comments

Comments
 (0)