On Mon, May 06, 2013 at 02:41:22PM -0400, Jeff King wrote:
> That is a weaker guarantee, and I think we can provide it with:
>
> 1. Load all loose refs into cache for a particular enumeration.
>
> 2. Make sure the packed-refs cache is up-to-date (by checking its
> stat() information and reloading if necessary).
>
> 3. Run the usual iteration over the loose/packed ref caches.
This does seem to work in my experiments. With stock git, I can trigger
the race reliably with:
# base load, in one terminal, as before
git init -q repo &&
cd repo &&
git commit -q --allow-empty -m one &&
one=`git rev-parse HEAD` &&
git commit -q --allow-empty -m two &&
two=`git rev-parse HEAD` &&
sha1=$one &&
while true; do
# this re-creates the loose ref in .git/refs/heads/master
if test "$sha1" = "$one"; then
sha1=$two
else
sha1=$one
fi &&
git update-ref refs/heads/master $sha1 &&
# we can remove packed-refs safely, as we know that
# its only value is now stale. Real git would not do
# this, but we are simulating the case that "master"
# simply wasn't included in the last packed-refs file.
rm -f .git/packed-refs &&
# and now we repack, which will create an up-to-date
# packed-refs file, and then delete the loose ref
git pack-refs --all --prune
done
# in another terminal, enumerate and make sure we never miss the ref
cd repo &&
while true; do
refs=`git.compile for-each-ref --format='%(refname)'`
echo "==> $refs"
test -z "$refs" && break
done
It usually takes about 30 seconds to hit a problem, though I measured
failures at up to 90 seconds. With the patch below (on top of the one I
posted the other day, which refreshes the packed-refs cache in
get_packed_refs), it has been running fine for 15 minutes.
The "noop_each_fn" is a little gross. I could also just reimplement the
recursion from do_for_each_ref_in_dir (except we don't care about flags,
trim, base, etc, and would just be calling get_ref_dir recursively).
It's a slight repetition of code, but it would be less subtle than what
I have written below (which uses a no-op callback for the side effect
that it primes the loose ref cache). Which poison do you prefer?
diff --git a/refs.c b/refs.c
index 45a7ee6..59ae7e4 100644
--- a/refs.c
+++ b/refs.c
@@ -1363,19 +1363,38 @@ static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn
for_each_rawref(warn_if_dangling_symref, &data);
}
+static int noop_each_fn(const char *ref, const unsigned char *sha1, int flags,
+ void *data)
+{
+ return 0;
+}
+
static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn,
int trim, int flags, void *cb_data)
{
struct ref_cache *refs = get_ref_cache(submodule);
- struct ref_dir *packed_dir = get_packed_refs(refs);
- struct ref_dir *loose_dir = get_loose_refs(refs);
+ struct ref_dir *packed_dir;
+ struct ref_dir *loose_dir;
int retval = 0;
- if (base && *base) {
- packed_dir = find_containing_dir(packed_dir, base, 0);
+ /*
+ * Prime the loose ref cache; we must make sure the packed ref cache is
+ * uptodate after we read the loose refs in order to avoid race
+ * conditions with a simultaneous "pack-refs --prune".
+ */
+ loose_dir = get_loose_refs(refs);
+ if (base && *base)
loose_dir = find_containing_dir(loose_dir, base, 0);
+ if (loose_dir) {
+ sort_ref_dir(loose_dir);
+ do_for_each_ref_in_dir(loose_dir, 0, base, noop_each_fn, 0, 0,
+ NULL);
}
+ packed_dir = get_packed_refs(refs);
+ if (base && *base)
+ packed_dir = find_containing_dir(packed_dir, base, 0);
+
if (packed_dir && loose_dir) {
sort_ref_dir(packed_dir);
sort_ref_dir(loose_dir);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html