Re: [PATCH 2/3] Support for partial HTTP transfers

From: Junio C Hamano <junkio@cox.net>
Date: Sep 27, 2005, 05:46 UTC
Message-ID: <7vfyrr3vwp.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <20050927000931.GA15615@reactrix.com>

Nick Hengeveld <nickh@reactrix.com> writes:

> How about using mkstemp on the prev file to keep multiple instances
> from stepping on each other?  Since O_CREAT | O_EXCL is used to
> open the tmpfile, only one instance will be able to succeed and
> continue.

I think (O_CREAT|O_EXCL) in your code can be easily defeated by this sequence:

    one                 	two

    unlink prevfile
    rename tmpfile, prevfile
    unlink tmpfile
    local = open tmpfile
                                unlink prevfile
                                rename tmpfile, prevfile
                                unlink tmpfile
                                local = open tmpfile
                                prevlocal = open prevfile
    write local
                                copyfile prevlocal, local
    write local
				???

Not that I think the multiple instances should be prevented at this low level --- if they stomp on each other at this level, it is very likely the they are doing duplicated work on the network side as well, and should be prevented from doing so at much higher level than this, I think. That's why I said I do not mind BKL upfront in git-fetch.sh.

> That's an annoying case, all right...  Would it be worth including a 
> full retry if a partial failed the SHA1 check?

Probably not. Just failing loudly and asking the upper layer to retry would be fine, I think.