git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Johan Herland <johan@herland.net>
Date
Oct 18, 2013, 13:17 UTC
Message-ID
<1382102267-18151-1-git-send-email-johan@herland.net>

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:

Two concurrent fetches - A and B. As part of its fetch, A needs to store 12aaaaa as a loose object. B, on the other hand, needs to store 12bbbbb as a loose object. The objects/12 directory does not already exist. Concurrently, both A and B determine that they need to create the objects/12 directory (because their first call to git_mkstemp_mode() within create_tmpfile() fails witn ENOENT). One of them - let's say A - executes the following mkdir() call before the other. This first call returns success, and A moves on. When B gets around to calling mkdir(), it fails with EEXIST, because A won the race. The mkdir() error causes B to return -1 from create_tmpfile(), which propagates all the way, resulting in the fetch failing with:

  error: unable to create temporary file: File exists
  fatal: failed to write object
  fatal: unpack-objects failed

Although it's hard to add a testcase reproducing this issue, it's easy to reproduce if we insert a sleep after the

  if (mkdir(buffer, 0777) || adjust_shared_perm(buffer))
      return -1;
block, and then run two concurrent "git fetch"es against the same repo.
The fix is to simply handle mkdir() setting errno = EEXIST as success.
Signed-off-by: Johan Herland <johan@herland.net>
---
 sha1_file.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sha1_file.c b/sha1_file.c
index f80bbe4..00ffffe 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 sure the directory exists */
 		memcpy(buffer, filename, dirlen);
 		buffer[dirlen-1] = 0;
-		if (mkdir(buffer, 0777) || adjust_shared_perm(buffer))
+		if (mkdir(buffer, 0777) && errno != EEXIST)
+			return -1;
+		if (adjust_shared_perm(buffer))
 			return -1;
 
 		/* Try again */
-- 
1.8.4.653.g2df02b3
Next: Eric Sunshine
Message 1 of 12 in “sha1_file.c:create_tmpfile(): Fix race when creating loose object dirs”
  1. sha1_file.c:create_tmpfile(): Fix race when creating loose object dirsJohan Herland, Oct 18, 2013
  2. Eric SunshineOct 18, 2013
  3. Johan HerlandOct 18, 2013
  4. Eric SunshineOct 18, 2013
  5. Duy NguyenOct 18, 2013
  6. Duy NguyenOct 18, 2013
  7. Johan HerlandOct 18, 2013
  8. Jeff KingOct 18, 2013
  9. Junio C HamanoOct 18, 2013
  10. Johan HerlandOct 18, 2013
  11. Jeff KingOct 19, 2013
  12. Junio C HamanoOct 22, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.