lock_packed_refs(): fix cache validity check
commitfed6ebebf132eb32478a5cbbebba3760c943970a
authorMichael Haggerty <mhagger@alum.mit.edu>
Mon, 12 Jun 2017 08:06:13 +0000 (12 10:06 +0200)
committerJunio C Hamano <gitster@pobox.com>
Mon, 12 Jun 2017 17:11:36 +0000 (12 10:11 -0700)
tree65dbc52e5e4da2e5b8fa9858e77ee416b62837e0
parentf23092f19e73f5d8c3480ef02104af627a90361f
lock_packed_refs(): fix cache validity check

Commit 28ed9830b1 (get_packed_ref_cache(): assume "packed-refs" won't
change while locked, 2017-05-22) assumes that the "packed-refs" file
cannot change while we hold the lock. That assumption is
justified *if* the lock has been held the whole time since the
"packed-refs" file was last read.

But in `lock_packed_refs()`, we ourselves lock the "packed-refs" file
and then call `get_packed_ref_cache()` to ensure that the cache agrees
with the file. The intent is to guard against the possibility that
another process changed the "packed-refs" file the moment before we
locked it.

This check was defeated because `get_packed_ref_cache()` saw that the
file was locked, and therefore didn't do the `stat_validity_check()`
that we want.

The mistake was compounded with a misleading comment in
`lock_packed_refs()` claiming that it was doing the right thing. That
comment came from an earlier draft of the mh/packed-ref-store-prep
patch series when the commits were in a different order.

So instead:

* Extract a function `validate_packed_ref_cache()` that does the
  validity check independent of whether the lock is held.

* Change `get_packed_ref_cache()` to call the new function, but only
  if the lock *isn't* held.

* Change `lock_packed_refs()` to call the new function in any case
  before calling `get_packed_ref_cache()`.

* Fix the comment in `lock_packed_refs()`.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
refs/files-backend.c