Re: [PATCH 3/4] get_packed_refs: reload packed-refs file when it changes

[Eric Sunshine <sunshine@sunshineco.com>]

On Mon, May 6, 2013 at 10:43 PM, Jeff King <peff@peff.net> wrote:
> 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 it's stat()-validity as a

s/it's/its/

> 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: Jeff King <peff@peff.net>