Re: [PATCH] sha1_file.c:create_tmpfile(): Fix race when creating loose object dirs

From: Johan Herland <johan@herland.net>
Date: Oct 18, 2013, 14:52 UTC
Message-ID: <CALKQrgdMagUQMTGWX7RwBfBkA6+Q65aPfpk1W3m1BKMUNmjKFg@mail.gmail.com>
In-Reply-To: <CAPig+cRg0ovyutXmTggr5P++dWuVP3izf5JfcJEeEVZhHuVJiA@mail.gmail.com>

On Fri, Oct 18, 2013 at 3:53 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 6 quoted lines

> On Fri, Oct 18, 2013 at 9:17 AM, Johan Herland <johan@herland.net> wrote:
>> There are cases (e.g. when running concurrent fetches in a repo) where
>> multiple Git processes concurrently attempt to create loose objects
>> within the same objects/XX/ dir. The creation of the loose object files
>> is (AFAICS) safe from races, but the creation of the objects/XX/ dir in
>> which the loose objects reside is unsafe, for example:

[...]

Show 6 quoted lines

>> The fix is to simply handle mkdir() setting errno = EEXIST as success.
>
> Would it make sense for the commit message to explain what happens if
> EEXIST is returned but the pathname is not a directory? (For instance,
> in the same race window, another process had created a plain file or
> pipe there.)

I'm pretty sure in that case, the following adjust_shared_perm() or git_mkstemp_mode() would fail, and we'd end up returning error from create_tmpfile() in any case.

I can add the above to the commit message if you insist. :)

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net