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

2013-10-22 Thread Junio C Hamano
Johan Herland writes: >> Agreed. We may notice the failure to correct the permissions in the >> new code, where the old code left existing directories incorrect >> permissions as they were. > > I'm trying to mentally construct a scenario where two writers with > different configuration contexts

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

2013-10-18 Thread Jeff King
On Sat, Oct 19, 2013 at 01:45:03AM +0200, Johan Herland wrote: > I'm trying to mentally construct a scenario where two writers with > different configuration contexts - one with shared_repository and one > without - compete in this race, and we somehow end up with one of them > not being able to w

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

2013-10-18 Thread Johan Herland
On Fri, Oct 18, 2013 at 9:24 PM, Junio C Hamano wrote: > Jeff King writes: >> Agreed. We already leave a wrong-permission directory in place if it >> existed before we started create_tmpfile. The code before your patch, >> when racing with such a wrong-directory creator, would abort the >> tmpfil

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

2013-10-18 Thread Junio C Hamano
Jeff King writes: > I was almost tempted to say "we do not even need to run > adjust_shared_perm twice", but we do, or we risk another race: one side > loses the mkdir race, but wins the open() race, and writes to a > wrong-permission directory. Of course, that should not matter unless the > race

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

2013-10-18 Thread Jeff King
On Fri, Oct 18, 2013 at 05:41:54PM +0200, Johan Herland wrote: > (c) mkdir() fails with EEXIST, i.e. we lost the race. We do the > adjust_shared_perms() which might fail (-> abort) or succeed, and we > then _retry_ the git_mkstemp_mode(). There is no case where the > "whatever" that happened betw

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

2013-10-18 Thread Eric Sunshine
On Fri, Oct 18, 2013 at 10:52 AM, Johan Herland wrote: > On Fri, Oct 18, 2013 at 3:53 PM, Eric Sunshine > wrote: >> On Fri, Oct 18, 2013 at 9:17 AM, Johan Herland wrote: >>> There are cases (e.g. when running concurrent fetches in a repo) where >>> multiple Git processes concurrently attempt to

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

2013-10-18 Thread Johan Herland
On Fri, Oct 18, 2013 at 4:00 PM, Duy Nguyen wrote: > On Fri, Oct 18, 2013 at 8:55 PM, Duy Nguyen wrote: >> On Fri, Oct 18, 2013 at 8:17 PM, Johan Herland wrote: >>> diff --git a/sha1_file.c b/sha1_file.c >>> index f80bbe4..00e 100644 >>> --- a/sha1_file.c >>> +++ b/sha1_file.c >>> @@ -2857,7

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

2013-10-18 Thread Johan Herland
On Fri, Oct 18, 2013 at 3:53 PM, Eric Sunshine wrote: > On Fri, Oct 18, 2013 at 9:17 AM, Johan Herland 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 cr

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

2013-10-18 Thread Duy Nguyen
On Fri, Oct 18, 2013 at 8:55 PM, Duy Nguyen wrote: > On Fri, Oct 18, 2013 at 8:17 PM, Johan Herland wrote: >> diff --git a/sha1_file.c b/sha1_file.c >> index f80bbe4..00e 100644 >> --- a/sha1_file.c >> +++ b/sha1_file.c >> @@ -2857,7 +2857,9 @@ static int create_tmpfile(char *buffer, size_t b

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

2013-10-18 Thread Duy Nguyen
On Fri, Oct 18, 2013 at 8:17 PM, Johan Herland wrote: > diff --git a/sha1_file.c b/sha1_file.c > index f80bbe4..00e 100644 > --- a/sha1_file.c > +++ b/sha1_file.c > @@ -2857,7 +2857,9 @@ static int create_tmpfile(char *buffer, size_t bufsiz, > const char *filename) > /* Make s

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

2013-10-18 Thread Eric Sunshine
On Fri, Oct 18, 2013 at 9:17 AM, Johan Herland 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 rac

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

2013-10-18 Thread Johan Herland
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