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

[PATCH v3 15/17] rename_tmp_log(): handle a possible mkdir/rmdir race

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Jan 18, 2014, 22:48 UTC
Message-ID
<1390085341-2553-16-git-send-email-mhagger@alum.mit.edu>
In-Reply-To
<1390085341-2553-1-git-send-email-mhagger@alum.mit.edu>

If a directory vanishes while renaming the temporary reflog file, retry (up to 3 times). This could happen if another process deletes the directory created by safe_create_leading_directories() just before we rename the file into the directory.

As far as I can tell, this race could not occur internal to git. The only time that a directory under $GIT_DIR/logs is deleted is if room has to be made for a log file for a reference with the same name; for example, in the following sequence:

    git branch foo/bar    # Creates file .git/logs/refs/heads/foo/bar
    git branch -d foo/bar # Deletes file but leaves .git/logs/refs/heads/foo/
    git branch foo        # Deletes .git/logs/refs/heads/foo/

But the only reason the last command deletes the directory is because it wants to create a file with the same name. So if another process (e.g.,

    git branch foo/baz

) wants to create that directory, one of the two is doomed to failure anyway because of a D/F conflict.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index d0fab39..a90904b 100644
--- a/refs.c
+++ b/refs.c
@@ -2530,12 +2530,14 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
 
 static int rename_tmp_log(const char *newrefname)
 {
+	int attempts_remaining = 3;
+
+ retry:
 	if (safe_create_leading_directories(git_path("logs/%s", newrefname))) {
 		error("unable to create directory for %s", newrefname);
 		return -1;
 	}
 
- retry:
 	if (rename(git_path(TMP_RENAMED_LOG), git_path("logs/%s", newrefname))) {
 		if (errno==EISDIR || errno==ENOTDIR) {
 			/*
@@ -2548,6 +2550,13 @@ static int rename_tmp_log(const char *newrefname)
 				return -1;
 			}
 			goto retry;
+		} else if (errno == ENOENT && --attempts_remaining > 0) {
+			/*
+			 * Maybe another process just deleted one of
+			 * the directories in the path to newrefname.
+			 * Try again from the beginning.
+			 */
+			goto retry;
 		} else {
 			error("unable to move logfile "TMP_RENAMED_LOG" to logs/%s: %s",
 				newrefname, strerror(errno));
-- 
1.8.5.2
Previous: Michael HaggertyNext: Michael Haggerty
Message 16 of 18 in “Fix some mkdir/rmdir races”
  1. 00/17 Fix some mkdir/rmdir racesMichael Haggerty, Jan 18, 2014
  2. 01/17 safe_create_leading_directories(): fix format of "if" chainingMichael Haggerty, Jan 18, 2014
  3. 02/17 safe_create_leading_directories(): reduce scope of local variableMichael Haggerty, Jan 18, 2014
  4. 03/17 safe_create_leading_directories(): add explicit "slash" pointerMichael Haggerty, Jan 18, 2014
  5. 04/17 safe_create_leading_directories(): rename local variableMichael Haggerty, Jan 18, 2014
  6. 05/17 safe_create_leading_directories(): split on first of multiple slashesMichael Haggerty, Jan 18, 2014
  7. 06/17 safe_create_leading_directories(): always restore slash at end of loopMichael Haggerty, Jan 18, 2014
  8. 07/17 safe_create_leading_directories(): introduce enum for return valuesMichael Haggerty, Jan 18, 2014
  9. 08/17 cmd_init_db(): when creating directories, handle errors conservativelyMichael Haggerty, Jan 18, 2014
  10. 09/17 safe_create_leading_directories(): add new error value SCLD_VANISHEDMichael Haggerty, Jan 18, 2014
  11. 10/17 lock_ref_sha1_basic(): on SCLD_VANISHED, retryMichael Haggerty, Jan 18, 2014
  12. 11/17 lock_ref_sha1_basic(): if locking fails with ENOENT, retryMichael Haggerty, Jan 18, 2014
  13. 12/17 remove_dir_recurse(): tighten condition for removing unreadable dirMichael Haggerty, Jan 18, 2014
  14. 13/17 remove_dir_recurse(): handle disappearing files and directoriesMichael Haggerty, Jan 18, 2014
  15. 14/17 rename_ref(): extract function rename_tmp_log()Michael Haggerty, Jan 18, 2014
  16. 15/17 rename_tmp_log(): handle a possible mkdir/rmdir raceMichael Haggerty, Jan 18, 2014
  17. 16/17 rename_tmp_log(): limit the number of remote_empty_directories() attemptsMichael Haggerty, Jan 18, 2014
  18. 17/17 rename_tmp_log(): on SCLD_VANISHED, retryMichael Haggerty, Jan 18, 2014

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.