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

From: Jeff King <peff@peff.net>
Date: Oct 30, 2013, 09:19 UTC
Message-ID: <20131030091927.GQ11317@sigill.intra.peff.net>
In-Reply-To: <1382873743-4648-1-git-send-email-johan@herland.net>

On Sun, Oct 27, 2013 at 12:35:43PM +0100, Johan Herland wrote:

> I didn't see this in the latest "What's cooking", so here's a resend, with
> an expanded commit message to reflect our discussion. The patch itself is
> unchanged.

Thanks, your expanded description looks correct to me.

Show 18 quoted lines

> In order to fix the remaining race, I assume we have to ensure the dir
> creation obeys the same rules as the object creation, i.e. that there are
> only two possible states at any time:
> 
>  - The directory does not exist
> 
>  - The directory exists with the correct permissons
> 
> To achieve this, I guess we have to follow the same procedure we do for
> loose object creation:
> 
>  1. Create a temporary directory with a unique name (mkdtemp?)
> 
>  2. Adjust permissions
> 
>  3. Rename into place
> 
> Can this be done sufficiently atomically across all platforms?

Yeah, I think that is the only way to do it. I do not know offhand of any platforms that have problems with atomic directory renames, though I would not be surprised if some network filesystems don't handle it well.

I'd also be fine if you want to simply leave it at your patch for now and let somebody who cares more about the other race worry about it later.

-Peff