From: Jeff King <peff@xxxxxxxx>
Once we read the packed-refs file into memory, we cache it
to save work on future ref lookups. However, our cache may
be out of date with respect to what is on disk if another
process is simultaneously packing the refs. Normally it
is acceptable for us to be a little out of date, since there
is no guarantee whether we read the file before or after the
simultaneous update. However, there is an important special
case: our packed-refs file must be up to date with respect
to any loose refs we read. Otherwise, we risk the following
race condition:
0. There exists a loose ref refs/heads/master.
1. Process A starts and looks up the ref "master". It
first checks $GIT_DIR/master, which does not exist. It
then loads (and caches) the packed-refs file to see if
"master" exists in it, which it does not.
2. Meanwhile, process B runs "pack-refs --all --prune". It
creates a new packed-refs file which contains
refs/heads/master, and removes the loose copy at
$GIT_DIR/refs/heads/master.
3. Process A continues its lookup, and eventually tries
$GIT_DIR/refs/heads/master. It sees that the loose ref
is missing, and falls back to the packed-refs file. But
it examines its cached version, which does not have
refs/heads/master. After trying a few other prefixes,
it reports master as a non-existent ref.
There are many variants (e.g., step 1 may involve process A
looking up another ref entirely, so even a fully qualified
refname can fail). One of the most interesting ones is if
"refs/heads/master" is already packed. In that case process
A will not see it as missing, but rather will report
whatever value happened to be in the packed-refs file before
process B repacked (which might be an arbitrarily old
value).
We can fix this by making sure we reload the packed-refs
file from disk after looking at any loose refs. That's
unacceptably slow, so we can check its stat()-validity as a
proxy, and read it only when it appears to have changed.
Reading the packed-refs file after performing any loose-ref
system calls is sufficient because we know the ordering of
the pack-refs process: it always makes sure the newly
written packed-refs file is installed into place before
pruning any loose refs. As long as those operations by B
appear in their executed order to process A, by the time A
sees the missing loose ref, the new packed-refs file must be
in place.
Signed-off-by: Michael Haggerty <mhagger@xxxxxxxxxxxx>
---
refs.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index 6d48b42..d53a172 100644
--- a/refs.c
+++ b/refs.c
@@ -825,6 +825,9 @@ struct packed_ref_cache {
* when it is unlocked.
*/
struct lock_file *lock;
+
+ /* The metadata from when this packed-refs cache was read */
+ struct stat_validity validity;
};
/*
@@ -859,6 +862,7 @@ static int release_packed_ref_cache(struct packed_ref_cache *packed_refs)
{
if (!--packed_refs->referrers) {
free_ref_entry(packed_refs->root);
+ stat_validity_clear(&packed_refs->validity);
free(packed_refs);
return 1;
} else {
@@ -1050,19 +1054,26 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
*/
static struct packed_ref_cache *get_packed_ref_cache(struct ref_cache *refs)
{
+ const char *packed_refs_file;
+
+ if (*refs->name)
+ packed_refs_file = git_path_submodule(refs->name, "packed-refs");
+ else
+ packed_refs_file = git_path("packed-refs");
+
+ if (refs->packed &&
+ !stat_validity_check(&refs->packed->validity, packed_refs_file))
+ clear_packed_ref_cache(refs);
+
if (!refs->packed) {
- const char *packed_refs_file;
FILE *f;
refs->packed = xcalloc(1, sizeof(*refs->packed));
acquire_packed_ref_cache(refs->packed);
refs->packed->root = create_dir_entry(refs, "", 0, 0);
- if (*refs->name)
- packed_refs_file = git_path_submodule(refs->name, "packed-refs");
- else
- packed_refs_file = git_path("packed-refs");
f = fopen(packed_refs_file, "r");
if (f) {
+ stat_validity_update(&refs->packed->validity, fileno(f));
read_packed_refs(f, get_ref_dir(refs->packed->root));
fclose(f);
}
--
1.8.3.1
--
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