Re: [remote rejected] master -> master (n/a (unpacker error))

From: Jonathan Nieder <jrnieder@gmail.com>
Date: May 13, 2010, 13:56 UTC
Message-ID: <20100513135619.GA16848@progeny.tock>
In-Reply-To: <m2r5lgaqdb.fsf@igel.home>

Andreas Schwab wrote:

> Jonathan Nieder <jrnieder@gmail.com> writes:

Show 5 quoted lines

>> In your case, all 16384 trials yielded the same result: file already
>> existed.
>
> IMHO it is much more likely that a race happened between two git
> processes each wanting to create the .git/objects/e6 directory.

Good catch.  But wasn’t the problem reproducible?

In any event, that such a race is possible is not so nice. Here’s a naïve fix; it does not address other races, such as hash-object versus prune. Maybe git ought to acquire some sort of lock before writing to the object dir in a shared clone.

diff --git a/sha1_file.c b/sha1_file.c
index bbb819f..d305e53 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2244,6 +2244,13 @@ static inline int directory_size(const char *filename)
 	return s - filename + 1;
 }
 
+static int ensure_directory_exists(const char *dir)
+{
+	if (mkdir(dir, 0777) && errno != EEXIST)
+		return -1;
+	return adjust_shared_perm(dir);
+}
+
 /*
  * This creates a temporary file in the same directory as the final
  * 'filename'
@@ -2266,7 +2273,7 @@ static int create_tmpfile(char *buffer, size_t bufsiz, const char *filename)
 		/* Make sure the directory exists */
 		memcpy(buffer, filename, dirlen);
 		buffer[dirlen-1] = 0;
-		if (mkdir(buffer, 0777) || adjust_shared_perm(buffer))
+		if (ensure_directory_exists(buffer))
 			return -1;
 
 		/* Try again */