On Fri, Jul 16, 2021 at 04:13:04PM +0200, Ævar Arnfjörð Bjarmason wrote:
> Add a comment about why it is that we need to check for the the
s/the the//
> existence of a reflog we're deleting after we've successfully acquired
> the lock in files_reflog_expire(). As noted in [1] the lock protocol
> for reflogs is somewhat intuitive.
Did you mean unintuitive here?
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index ec9c70d79cc..f73637fa087 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -3068,6 +3068,17 @@ static int files_reflog_expire(struct ref_store *ref_store,
> strbuf_release(&err);
> return -1;
> }
> +
> + /*
> + * When refs are deleted their reflog is deleted before the
> + * ref itself deleted. This race happens because there's no
> + * such thing as a lock on the reflog, instead we always lock
> + * the "loose ref" (even if packet) above with
> + * lock_ref_oid_basic().
> + *
> + * If race happens we've got nothing more to do, we were asked
> + * to delete the reflog, and it's not there anymore. Great!
> + */
> if (!refs_reflog_exists(ref_store, refname)) {
> unlock_ref(lock);
> return 0;
I think everything is accurate here, though I wouldn't have made the
distinction with "locking the loose ref". There is no such thing as
locking a packed ref; we always take the loose lock.
s/packet/packed/, and maybe s/If race/If a race/.
-Peff
