From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 02 Apr 2014 17:01:59 GMT
Subject: Re: [PATCH 07/22] lock_file(): always add lock_file object to lock_file_list
Message-ID: <xmqqppkzwtq0.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140401201659.GE21715@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> So for a moment, lk->filename contains the name of the valuable file we
> are locking.  If we get a signal at that moment, do we accidentally
> delete it in remove_lock_file?
>
> I think the answer is "no", because we check lk->owner before deleting,
> which will not match our pid (it should generally be zero due to xcalloc
> or static initialization, though perhaps we should clear it here).

That "generally be zero" no longer holds since 2/22, no?

> But that makes me wonder about the case of a reused lock. It will have
> lk->owner set from a previous invocation, and would potentially suffer
> from this problem. In other words, I think the change you are
> introducing does not have the problem, but the existing code does. :-/

Yes, I tend to agree.

> I didn't reproduce it experimentally, though.  We should be able to just
>
>     lk->owner = 0;
>
> before the initial strcpy to fix it, I would think.
>
> -Peff