Skip to content

Commit ca91993

Browse files
peffgitster
authored andcommitted
get_packed_ref_cache: reload packed-refs file when it changes
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 <[email protected]> Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 3861253 commit ca91993

File tree

1 file changed

+16
-5
lines changed

1 file changed

+16
-5
lines changed

refs.c

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -825,6 +825,9 @@ struct packed_ref_cache {
825825
* when it is unlocked.
826826
*/
827827
struct lock_file *lock;
828+
829+
/* The metadata from when this packed-refs cache was read */
830+
struct stat_validity validity;
828831
};
829832

830833
/*
@@ -862,6 +865,7 @@ static int release_packed_ref_cache(struct packed_ref_cache *packed_refs)
862865
{
863866
if (!--packed_refs->referrers) {
864867
free_ref_entry(packed_refs->root);
868+
stat_validity_clear(&packed_refs->validity);
865869
free(packed_refs);
866870
return 1;
867871
} else {
@@ -1053,19 +1057,26 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
10531057
*/
10541058
static struct packed_ref_cache *get_packed_ref_cache(struct ref_cache *refs)
10551059
{
1060+
const char *packed_refs_file;
1061+
1062+
if (*refs->name)
1063+
packed_refs_file = git_path_submodule(refs->name, "packed-refs");
1064+
else
1065+
packed_refs_file = git_path("packed-refs");
1066+
1067+
if (refs->packed &&
1068+
!stat_validity_check(&refs->packed->validity, packed_refs_file))
1069+
clear_packed_ref_cache(refs);
1070+
10561071
if (!refs->packed) {
1057-
const char *packed_refs_file;
10581072
FILE *f;
10591073

10601074
refs->packed = xcalloc(1, sizeof(*refs->packed));
10611075
acquire_packed_ref_cache(refs->packed);
10621076
refs->packed->root = create_dir_entry(refs, "", 0, 0);
1063-
if (*refs->name)
1064-
packed_refs_file = git_path_submodule(refs->name, "packed-refs");
1065-
else
1066-
packed_refs_file = git_path("packed-refs");
10671077
f = fopen(packed_refs_file, "r");
10681078
if (f) {
1079+
stat_validity_update(&refs->packed->validity, fileno(f));
10691080
read_packed_refs(f, get_ref_dir(refs->packed->root));
10701081
fclose(f);
10711082
}

0 commit comments

Comments
 (0)