{"thread":{"id":"35687","subject":"[PATCH v3 00/17] Fix some mkdir/rmdir races","startedAt":"2014-01-18T22:48:44Z","lastAt":"2014-01-18T22:49:01Z","messageCount":18,"participants":["Michael Haggerty"],"isPatch":true,"patchVersion":3,"patchTotal":17},"messages":[{"id":"233341","messageId":"1390085341-2553-1-git-send-email-mhagger@alum.mit.edu","threadId":"35687","inReplyTo":null,"subject":"[PATCH v3 00/17] Fix some mkdir/rmdir races","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-01-18T22:48:44Z","receivedAt":"2014-01-18T22:48:44Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Sorry, I forgot to send this re-roll to the mailing list.\n\nThis version differs only very slightly from v2:\n\n* Rebased to the current master (there were no conflicts).\n\n* Rename a couple of local variables from \"attempts\" to\n  \"attempts_remaining\".\n\n* Patches 13, 16, and 17 have slightly improved commit messages.\n\nMichael Haggerty (17):\n  safe_create_leading_directories(): fix format of \"if\" chaining\n  safe_create_leading_directories(): reduce scope of local variable\n  safe_create_leading_directories(): add explicit \"slash\" pointer\n  safe_create_leading_directories(): rename local variable\n  safe_create_leading_directories(): split on first of multiple slashes\n  safe_create_leading_directories(): always restore slash at end of loop\n  safe_create_leading_directories(): introduce enum for return values\n  cmd_init_db(): when creating directories, handle errors conservatively\n  safe_create_leading_directories(): add new error value SCLD_VANISHED\n  lock_ref_sha1_basic(): on SCLD_VANISHED, retry\n  lock_ref_sha1_basic(): if locking fails with ENOENT, retry\n  remove_dir_recurse(): tighten condition for removing unreadable dir\n  remove_dir_recurse(): handle disappearing files and directories\n  rename_ref(): extract function rename_tmp_log()\n  rename_tmp_log(): handle a possible mkdir/rmdir race\n  rename_tmp_log(): limit the number of remote_empty_directories()\n    attempts\n  rename_tmp_log(): on SCLD_VANISHED, retry\n\n builtin/init-db.c |  9 +++---\n cache.h           | 25 +++++++++++++--\n dir.c             | 27 +++++++++++-----\n merge-recursive.c |  2 +-\n refs.c            | 92 ++++++++++++++++++++++++++++++++++++++++---------------\n sha1_file.c       | 67 ++++++++++++++++++++++------------------\n 6 files changed, 155 insertions(+), 67 deletions(-)\n\n-- \n1.8.5.2\n"},{"id":"233343","messageId":"1390085341-2553-2-git-send-email-mhagger@alum.mit.edu","threadId":"35687","inReplyTo":"1390085341-2553-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 01/17] safe_create_leading_directories(): fix format of \"if\" chaining","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-01-18T22:48:45Z","receivedAt":"2014-01-18T22:48:45Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n sha1_file.c | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex e13bd2c..8031d51 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -125,8 +125,7 @@ int safe_create_leading_directories(char *path)\n \t\t\t\t*pos = '/';\n \t\t\t\treturn -3;\n \t\t\t}\n-\t\t}\n-\t\telse if (mkdir(path, 0777)) {\n+\t\t} else if (mkdir(path, 0777)) {\n \t\t\tif (errno == EEXIST &&\n \t\t\t    !stat(path, &st) && S_ISDIR(st.st_mode)) {\n \t\t\t\t; /* somebody created it since we checked */\n@@ -134,8 +133,7 @@ int safe_create_leading_directories(char *path)\n \t\t\t\t*pos = '/';\n \t\t\t\treturn -1;\n \t\t\t}\n-\t\t}\n-\t\telse if (adjust_shared_perm(path)) {\n+\t\t} else if (adjust_shared_perm(path)) {\n \t\t\t*pos = '/';\n \t\t\treturn -2;\n \t\t}\n-- \n1.8.5.2\n"},{"id":"233344","messageId":"1390085341-2553-3-git-send-email-mhagger@alum.mit.edu","threadId":"35687","inReplyTo":"1390085341-2553-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 02/17] safe_create_leading_directories(): reduce scope of local variable","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-01-18T22:48:46Z","receivedAt":"2014-01-18T22:48:46Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This makes it more obvious that values of \"st\" don't persist across\nloop iterations.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n sha1_file.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 8031d51..d8647c7 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -108,9 +108,10 @@ int mkdir_in_gitdir(const char *path)\n int safe_create_leading_directories(char *path)\n {\n \tchar *pos = path + offset_1st_component(path);\n-\tstruct stat st;\n \n \twhile (pos) {\n+\t\tstruct stat st;\n+\n \t\tpos = strchr(pos, '/');\n \t\tif (!pos)\n \t\t\tbreak;\n-- \n1.8.5.2\n"},{"id":"233350","messageId":"1390085341-2553-4-git-send-email-mhagger@alum.mit.edu","threadId":"35687","inReplyTo":"1390085341-2553-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 03/17] safe_create_leading_directories(): add explicit \"slash\" pointer","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-01-18T22:48:47Z","receivedAt":"2014-01-18T22:48:47Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Keep track of the position of the slash character independently of\n\"pos\", thereby making the purpose of each variable clearer and\nworking towards other upcoming changes.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n sha1_file.c | 20 +++++++++++---------\n 1 file changed, 11 insertions(+), 9 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex d8647c7..88b8d8c 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -111,19 +111,21 @@ int safe_create_leading_directories(char *path)\n \n \twhile (pos) {\n \t\tstruct stat st;\n+\t\tchar *slash = strchr(pos, '/');\n \n-\t\tpos = strchr(pos, '/');\n-\t\tif (!pos)\n+\t\tif (!slash)\n \t\t\tbreak;\n-\t\twhile (*++pos == '/')\n-\t\t\t;\n+\t\twhile (*(slash + 1) == '/')\n+\t\t\tslash++;\n+\t\tpos = slash + 1;\n \t\tif (!*pos)\n \t\t\tbreak;\n-\t\t*--pos = '\\0';\n+\n+\t\t*slash = '\\0';\n \t\tif (!stat(path, &st)) {\n \t\t\t/* path exists */\n \t\t\tif (!S_ISDIR(st.st_mode)) {\n-\t\t\t\t*pos = '/';\n+\t\t\t\t*slash = '/';\n \t\t\t\treturn -3;\n \t\t\t}\n \t\t} else if (mkdir(path, 0777)) {\n@@ -131,14 +133,14 @@ int safe_create_leading_directories(char *path)\n \t\t\t    !stat(path, &st) && S_ISDIR(st.st_mode)) {\n \t\t\t\t; /* somebody created it since we checked */\n \t\t\t} else {\n-\t\t\t\t*pos = '/';\n+\t\t\t\t*slash = '/';\n \t\t\t\treturn -1;\n \t\t\t}\n \t\t} else if (adjust_shared_perm(path)) {\n-\t\t\t*pos = '/';\n+\t\t\t*slash = '/';\n \t\t\treturn -2;\n \t\t}\n-\t\t*pos++ = '/';\n+\t\t*slash = '/';\n \t}\n \treturn 0;\n }\n-- \n1.8.5.2\n"},{"id":"233348","messageId":"1390085341-2553-5-git-send-email-mhagger@alum.mit.edu","threadId":"35687","inReplyTo":"1390085341-2553-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 04/17] safe_create_leading_directories(): rename local variable","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-01-18T22:48:48Z","receivedAt":"2014-01-18T22:48:48Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Rename \"pos\" to \"next_component\", because now it always points at the\nnext component of the path name that has to be processed.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n sha1_file.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 88b8d8c..1e1510d 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -107,18 +107,18 @@ int mkdir_in_gitdir(const char *path)\n \n int safe_create_leading_directories(char *path)\n {\n-\tchar *pos = path + offset_1st_component(path);\n+\tchar *next_component = path + offset_1st_component(path);\n \n-\twhile (pos) {\n+\twhile (next_component) {\n \t\tstruct stat st;\n-\t\tchar *slash = strchr(pos, '/');\n+\t\tchar *slash = strchr(next_component, '/');\n \n \t\tif (!slash)\n \t\t\tbreak;\n \t\twhile (*(slash + 1) == '/')\n \t\t\tslash++;\n-\t\tpos = slash + 1;\n-\t\tif (!*pos)\n+\t\tnext_component = slash + 1;\n+\t\tif (!*next_component)\n \t\t\tbreak;\n \n \t\t*slash = '\\0';\n-- \n1.8.5.2\n"},{"id":"233346","messageId":"1390085341-2553-6-git-send-email-mhagger@alum.mit.edu","threadId":"35687","inReplyTo":"1390085341-2553-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 05/17] safe_create_leading_directories(): split on first of multiple slashes","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-01-18T22:48:49Z","receivedAt":"2014-01-18T22:48:49Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"If the input path has multiple slashes between path components (e.g.,\n\"foo//bar\"), then the old code was breaking the path at the last\nslash, not the first one.  So in the above example, the second slash\nwas overwritten with NUL, resulting in the parent directory being\nsought as \"foo/\".\n\nWhen stat() is called on \"foo/\", it fails with ENOTDIR if \"foo\" exists\nbut is not a directory.  This caused the wrong path to be taken in the\nsubsequent logic.\n\nSo instead, split path components at the first intercomponent slash\nrather than the last one.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n sha1_file.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 1e1510d..9d4459f 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -115,9 +115,10 @@ int safe_create_leading_directories(char *path)\n \n \t\tif (!slash)\n \t\t\tbreak;\n-\t\twhile (*(slash + 1) == '/')\n-\t\t\tslash++;\n+\n \t\tnext_component = slash + 1;\n+\t\twhile (*next_component == '/')\n+\t\t\tnext_component++;\n \t\tif (!*next_component)\n \t\t\tbreak;\n \n-- \n1.8.5.2\n"},{"id":"233345","messageId":"1390085341-2553-7-git-send-email-mhagger@alum.mit.edu","threadId":"35687","inReplyTo":"1390085341-2553-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 06/17] safe_create_leading_directories(): always restore slash at end of loop","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-01-18T22:48:50Z","receivedAt":"2014-01-18T22:48:50Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Always restore the slash that we scribbled over at the end of the\nloop, rather than also fixing it up at each premature exit from the\nloop.  This makes it harder to forget to do the cleanup as new paths\nare added to the code.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n sha1_file.c | 22 +++++++++-------------\n 1 file changed, 9 insertions(+), 13 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 9d4459f..80122b2 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -108,8 +108,9 @@ int mkdir_in_gitdir(const char *path)\n int safe_create_leading_directories(char *path)\n {\n \tchar *next_component = path + offset_1st_component(path);\n+\tint ret = 0;\n \n-\twhile (next_component) {\n+\twhile (!ret && next_component) {\n \t\tstruct stat st;\n \t\tchar *slash = strchr(next_component, '/');\n \n@@ -125,25 +126,20 @@ int safe_create_leading_directories(char *path)\n \t\t*slash = '\\0';\n \t\tif (!stat(path, &st)) {\n \t\t\t/* path exists */\n-\t\t\tif (!S_ISDIR(st.st_mode)) {\n-\t\t\t\t*slash = '/';\n-\t\t\t\treturn -3;\n-\t\t\t}\n+\t\t\tif (!S_ISDIR(st.st_mode))\n+\t\t\t\tret = -3;\n \t\t} else if (mkdir(path, 0777)) {\n \t\t\tif (errno == EEXIST &&\n-\t\t\t    !stat(path, &st) && S_ISDIR(st.st_mode)) {\n+\t\t\t    !stat(path, &st) && S_ISDIR(st.st_mode))\n \t\t\t\t; /* somebody created it since we checked */\n-\t\t\t} else {\n-\t\t\t\t*slash = '/';\n-\t\t\t\treturn -1;\n-\t\t\t}\n+\t\t\telse\n+\t\t\t\tret = -1;\n \t\t} else if (adjust_shared_perm(path)) {\n-\t\t\t*slash = '/';\n-\t\t\treturn -2;\n+\t\t\tret = -2;\n \t\t}\n \t\t*slash = '/';\n \t}\n-\treturn 0;\n+\treturn ret;\n }\n \n int safe_create_leading_directories_const(const char *path)\n-- \n1.8.5.2\n"},{"id":"233342","messageId":"1390085341-2553-8-git-send-email-mhagger@alum.mit.edu","threadId":"35687","inReplyTo":"1390085341-2553-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 07/17] safe_create_leading_directories(): introduce enum for return values","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-01-18T22:48:51Z","receivedAt":"2014-01-18T22:48:51Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Instead of returning magic integer values (which a couple of callers\ngo to the trouble of distinguishing), return values from an enum.  Add\na docstring.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n builtin/init-db.c |  4 ++--\n cache.h           | 17 +++++++++++++++--\n merge-recursive.c |  2 +-\n sha1_file.c       | 16 ++++++++--------\n 4 files changed, 26 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex b3f03cf..6f69593 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -515,10 +515,10 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t\t\t\tsaved = shared_repository;\n \t\t\t\tshared_repository = 0;\n \t\t\t\tswitch (safe_create_leading_directories_const(argv[0])) {\n-\t\t\t\tcase -3:\n+\t\t\t\tcase SCLD_EXISTS:\n \t\t\t\t\terrno = EEXIST;\n \t\t\t\t\t/* fallthru */\n-\t\t\t\tcase -1:\n+\t\t\t\tcase SCLD_FAILED:\n \t\t\t\t\tdie_errno(_(\"cannot mkdir %s\"), argv[0]);\n \t\t\t\t\tbreak;\n \t\t\t\tdefault:\ndiff --git a/cache.h b/cache.h\nindex c9efe88..8030e36 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -737,8 +737,21 @@ enum sharedrepo {\n };\n int git_config_perm(const char *var, const char *value);\n int adjust_shared_perm(const char *path);\n-int safe_create_leading_directories(char *path);\n-int safe_create_leading_directories_const(const char *path);\n+\n+/*\n+ * Create the directory containing the named path, using care to be\n+ * somewhat safe against races.  Return one of the scld_error values\n+ * to indicate success/failure.\n+ */\n+enum scld_error {\n+\tSCLD_OK = 0,\n+\tSCLD_FAILED = -1,\n+\tSCLD_PERMS = -2,\n+\tSCLD_EXISTS = -3\n+};\n+enum scld_error safe_create_leading_directories(char *path);\n+enum scld_error safe_create_leading_directories_const(const char *path);\n+\n int mkdir_in_gitdir(const char *path);\n extern void home_config_paths(char **global, char **xdg, char *file);\n extern char *expand_user_path(const char *path);\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex a18bd15..8400a8e 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -693,7 +693,7 @@ static int make_room_for_path(struct merge_options *o, const char *path)\n \t/* Make sure leading directories are created */\n \tstatus = safe_create_leading_directories_const(path);\n \tif (status) {\n-\t\tif (status == -3) {\n+\t\tif (status == SCLD_EXISTS) {\n \t\t\t/* something else exists */\n \t\t\terror(msg, path, _(\": perhaps a D/F conflict?\"));\n \t\t\treturn -1;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 80122b2..a065308 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -105,12 +105,12 @@ int mkdir_in_gitdir(const char *path)\n \treturn adjust_shared_perm(path);\n }\n \n-int safe_create_leading_directories(char *path)\n+enum scld_error safe_create_leading_directories(char *path)\n {\n \tchar *next_component = path + offset_1st_component(path);\n-\tint ret = 0;\n+\tenum scld_error ret = SCLD_OK;\n \n-\twhile (!ret && next_component) {\n+\twhile (ret == SCLD_OK && next_component) {\n \t\tstruct stat st;\n \t\tchar *slash = strchr(next_component, '/');\n \n@@ -127,26 +127,26 @@ int safe_create_leading_directories(char *path)\n \t\tif (!stat(path, &st)) {\n \t\t\t/* path exists */\n \t\t\tif (!S_ISDIR(st.st_mode))\n-\t\t\t\tret = -3;\n+\t\t\t\tret = SCLD_EXISTS;\n \t\t} else if (mkdir(path, 0777)) {\n \t\t\tif (errno == EEXIST &&\n \t\t\t    !stat(path, &st) && S_ISDIR(st.st_mode))\n \t\t\t\t; /* somebody created it since we checked */\n \t\t\telse\n-\t\t\t\tret = -1;\n+\t\t\t\tret = SCLD_FAILED;\n \t\t} else if (adjust_shared_perm(path)) {\n-\t\t\tret = -2;\n+\t\t\tret = SCLD_PERMS;\n \t\t}\n \t\t*slash = '/';\n \t}\n \treturn ret;\n }\n \n-int safe_create_leading_directories_const(const char *path)\n+enum scld_error safe_create_leading_directories_const(const char *path)\n {\n \t/* path points to cache entries, so xstrdup before messing with it */\n \tchar *buf = xstrdup(path);\n-\tint result = safe_create_leading_directories(buf);\n+\tenum scld_error result = safe_create_leading_directories(buf);\n \tfree(buf);\n \treturn result;\n }\n-- \n1.8.5.2\n"},{"id":"233357","messageId":"1390085341-2553-9-git-send-email-mhagger@alum.mit.edu","threadId":"35687","inReplyTo":"1390085341-2553-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 08/17] cmd_init_db(): when creating directories, handle errors conservatively","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-01-18T22:48:52Z","receivedAt":"2014-01-18T22:48:52Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"safe_create_leading_directories_const() returns a non-zero value on\nerror.  The old code at this calling site recognized a couple of\nparticular error values, and treated all other return values as\nsuccess.  Instead, be more conservative: recognize the errors we are\ninterested in, but treat any other nonzero values as failures.  This\nis more robust in case somebody adds another possible return value\nwithout telling us.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n builtin/init-db.c | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 6f69593..c7c76bb 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -515,13 +515,14 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t\t\t\tsaved = shared_repository;\n \t\t\t\tshared_repository = 0;\n \t\t\t\tswitch (safe_create_leading_directories_const(argv[0])) {\n+\t\t\t\tcase SCLD_OK:\n+\t\t\t\tcase SCLD_PERMS:\n+\t\t\t\t\tbreak;\n \t\t\t\tcase SCLD_EXISTS:\n \t\t\t\t\terrno = EEXIST;\n \t\t\t\t\t/* fallthru */\n-\t\t\t\tcase SCLD_FAILED:\n-\t\t\t\t\tdie_errno(_(\"cannot mkdir %s\"), argv[0]);\n-\t\t\t\t\tbreak;\n \t\t\t\tdefault:\n+\t\t\t\t\tdie_errno(_(\"cannot mkdir %s\"), argv[0]);\n \t\t\t\t\tbreak;\n \t\t\t\t}\n \t\t\t\tshared_repository = saved;\n-- \n1.8.5.2\n"},{"id":"233358","messageId":"1390085341-2553-10-git-send-email-mhagger@alum.mit.edu","threadId":"35687","inReplyTo":"1390085341-2553-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 09/17] safe_create_leading_directories(): add new error value SCLD_VANISHED","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-01-18T22:48:53Z","receivedAt":"2014-01-18T22:48:53Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Add a new possible error result that can be returned by\nsafe_create_leading_directories() and\nsafe_create_leading_directories_const(): SCLD_VANISHED.  This value\nindicates that a file or directory on the path existed at one point\n(either it already existed or the function created it), but then it\ndisappeared.  This probably indicates that another process deleted the\ndirectory while we were working.  If SCLD_VANISHED is returned, the\ncaller might want to retry the function call, as there is a chance\nthat a new attempt will succeed.\n\nWhy doesn't safe_create_leading_directories() do the retrying\ninternally?  Because an empty directory isn't really ever safe until\nit holds a file.  So even if safe_create_leading_directories() were\nabsolutely sure that the directory existed before it returned, there\nwould be no guarantee that the directory still existed when the caller\ntried to write something in it.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n cache.h     | 10 +++++++++-\n sha1_file.c | 11 +++++++++++\n 2 files changed, 20 insertions(+), 1 deletion(-)\n\ndiff --git a/cache.h b/cache.h\nindex 8030e36..c0a7a8a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -742,12 +742,20 @@ int adjust_shared_perm(const char *path);\n  * Create the directory containing the named path, using care to be\n  * somewhat safe against races.  Return one of the scld_error values\n  * to indicate success/failure.\n+ *\n+ * SCLD_VANISHED indicates that one of the ancestor directories of the\n+ * path existed at one point during the function call and then\n+ * suddenly vanished, probably because another process pruned the\n+ * directory while we were working.  To be robust against this kind of\n+ * race, callers might want to try invoking the function again when it\n+ * returns SCLD_VANISHED.\n  */\n enum scld_error {\n \tSCLD_OK = 0,\n \tSCLD_FAILED = -1,\n \tSCLD_PERMS = -2,\n-\tSCLD_EXISTS = -3\n+\tSCLD_EXISTS = -3,\n+\tSCLD_VANISHED = -4\n };\n enum scld_error safe_create_leading_directories(char *path);\n enum scld_error safe_create_leading_directories_const(const char *path);\ndiff --git a/sha1_file.c b/sha1_file.c\nindex a065308..8b0849f 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -132,6 +132,17 @@ enum scld_error safe_create_leading_directories(char *path)\n \t\t\tif (errno == EEXIST &&\n \t\t\t    !stat(path, &st) && S_ISDIR(st.st_mode))\n \t\t\t\t; /* somebody created it since we checked */\n+\t\t\telse if (errno == ENOENT)\n+\t\t\t\t/*\n+\t\t\t\t * Either mkdir() failed because\n+\t\t\t\t * somebody just pruned the containing\n+\t\t\t\t * directory, or stat() failed because\n+\t\t\t\t * the file that was in our way was\n+\t\t\t\t * just removed.  Either way, inform\n+\t\t\t\t * the caller that it might be worth\n+\t\t\t\t * trying again:\n+\t\t\t\t */\n+\t\t\t\tret = SCLD_VANISHED;\n \t\t\telse\n \t\t\t\tret = SCLD_FAILED;\n \t\t} else if (adjust_shared_perm(path)) {\n-- \n1.8.5.2\n"},{"id":"233351","messageId":"1390085341-2553-11-git-send-email-mhagger@alum.mit.edu","threadId":"35687","inReplyTo":"1390085341-2553-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 10/17] lock_ref_sha1_basic(): on SCLD_VANISHED, retry","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-01-18T22:48:54Z","receivedAt":"2014-01-18T22:48:54Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"If safe_create_leading_directories() fails because a file along the\npath unexpectedly vanished, try again (up to 3 times).\n\nThis can occur if another process is deleting directories at the same\ntime as we are trying to make them.  For example, \"git pack-refs\n--all\" tries to delete the loose refs and any empty directories that\nare left behind.  If a pack-refs process is running, then it might\ndelete a directory that we need to put a new loose reference in.\n\nIf safe_create_leading_directories() thinks this might have happened,\nthen take its advice and try again (maximum three attempts).\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c | 11 ++++++++++-\n 1 file changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex 490b57b..9ae184d 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2039,6 +2039,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \tint type, lflags;\n \tint mustexist = (old_sha1 && !is_null_sha1(old_sha1));\n \tint missing = 0;\n+\tint attempts_remaining = 3;\n \n \tlock = xcalloc(1, sizeof(struct ref_lock));\n \tlock->lock_fd = -1;\n@@ -2093,7 +2094,15 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \tif ((flags & REF_NODEREF) && (type & REF_ISSYMREF))\n \t\tlock->force_write = 1;\n \n-\tif (safe_create_leading_directories(ref_file)) {\n+ retry:\n+\tswitch (safe_create_leading_directories(ref_file)) {\n+\tcase SCLD_OK:\n+\t\tbreak; /* success */\n+\tcase SCLD_VANISHED:\n+\t\tif (--attempts_remaining > 0)\n+\t\t\tgoto retry;\n+\t\t/* fall through */\n+\tdefault:\n \t\tlast_errno = errno;\n \t\terror(\"unable to create directory for %s\", ref_file);\n \t\tgoto error_return;\n-- \n1.8.5.2\n"},{"id":"233352","messageId":"1390085341-2553-12-git-send-email-mhagger@alum.mit.edu","threadId":"35687","inReplyTo":"1390085341-2553-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 11/17] lock_ref_sha1_basic(): if locking fails with ENOENT, retry","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-01-18T22:48:55Z","receivedAt":"2014-01-18T22:48:55Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"If hold_lock_file_for_update() fails with errno==ENOENT, it might be\nbecause somebody else (for example, a pack-refs process) has just\ndeleted one of the lockfile's ancestor directories.  So if this\ncondition is detected, try again (up to 3 times).\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c | 13 ++++++++++++-\n 1 file changed, 12 insertions(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex 9ae184d..4060ed1 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2081,7 +2081,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \n \tlock->lk = xcalloc(1, sizeof(struct lock_file));\n \n-\tlflags = LOCK_DIE_ON_ERROR;\n+\tlflags = 0;\n \tif (flags & REF_NODEREF) {\n \t\trefname = orig_refname;\n \t\tlflags |= LOCK_NODEREF;\n@@ -2109,6 +2109,17 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \t}\n \n \tlock->lock_fd = hold_lock_file_for_update(lock->lk, ref_file, lflags);\n+\tif (lock->lock_fd < 0) {\n+\t\tif (errno == ENOENT && --attempts_remaining > 0)\n+\t\t\t/*\n+\t\t\t * Maybe somebody just deleted one of the\n+\t\t\t * directories leading to ref_file.  Try\n+\t\t\t * again:\n+\t\t\t */\n+\t\t\tgoto retry;\n+\t\telse\n+\t\t\tunable_to_lock_index_die(ref_file, errno);\n+\t}\n \treturn old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock;\n \n  error_return:\n-- \n1.8.5.2\n"},{"id":"233356","messageId":"1390085341-2553-13-git-send-email-mhagger@alum.mit.edu","threadId":"35687","inReplyTo":"1390085341-2553-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 12/17] remove_dir_recurse(): tighten condition for removing unreadable dir","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-01-18T22:48:56Z","receivedAt":"2014-01-18T22:48:56Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"If opendir() fails on the top-level directory, it makes sense to try\nto delete it anyway--but only if the failure was due to EACCES.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n dir.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex d10a63f..785a83e 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1511,8 +1511,11 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)\n \tflag &= ~REMOVE_DIR_KEEP_TOPLEVEL;\n \tdir = opendir(path->buf);\n \tif (!dir) {\n-\t\t/* an empty dir could be removed even if it is unreadble */\n-\t\tif (!keep_toplevel)\n+\t\tif (errno == EACCES && !keep_toplevel)\n+\t\t\t/*\n+\t\t\t * An empty dir could be removable even if it\n+\t\t\t * is unreadable:\n+\t\t\t */\n \t\t\treturn rmdir(path->buf);\n \t\telse\n \t\t\treturn -1;\n-- \n1.8.5.2\n"},{"id":"233354","messageId":"1390085341-2553-14-git-send-email-mhagger@alum.mit.edu","threadId":"35687","inReplyTo":"1390085341-2553-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 13/17] remove_dir_recurse(): handle disappearing files and directories","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-01-18T22:48:57Z","receivedAt":"2014-01-18T22:48:57Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"If a file or directory that we are trying to remove disappears (e.g.,\nbecause another process has pruned it), do not consider it an error.\n\nHowever, if REMOVE_DIR_KEEP_TOPLEVEL is set, and the toplevel\ndirectory is missing, then consider it an error (like before).\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n dir.c | 22 ++++++++++++++++------\n 1 file changed, 16 insertions(+), 6 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 785a83e..b35b633 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1511,7 +1511,9 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)\n \tflag &= ~REMOVE_DIR_KEEP_TOPLEVEL;\n \tdir = opendir(path->buf);\n \tif (!dir) {\n-\t\tif (errno == EACCES && !keep_toplevel)\n+\t\tif (errno == ENOENT)\n+\t\t\treturn keep_toplevel ? -1 : 0;\n+\t\telse if (errno == EACCES && !keep_toplevel)\n \t\t\t/*\n \t\t\t * An empty dir could be removable even if it\n \t\t\t * is unreadable:\n@@ -1531,13 +1533,21 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)\n \n \t\tstrbuf_setlen(path, len);\n \t\tstrbuf_addstr(path, e->d_name);\n-\t\tif (lstat(path->buf, &st))\n-\t\t\t; /* fall thru */\n-\t\telse if (S_ISDIR(st.st_mode)) {\n+\t\tif (lstat(path->buf, &st)) {\n+\t\t\tif (errno == ENOENT)\n+\t\t\t\t/*\n+\t\t\t\t * file disappeared, which is what we\n+\t\t\t\t * wanted anyway\n+\t\t\t\t */\n+\t\t\t\tcontinue;\n+\t\t\t/* fall thru */\n+\t\t} else if (S_ISDIR(st.st_mode)) {\n \t\t\tif (!remove_dir_recurse(path, flag, &kept_down))\n \t\t\t\tcontinue; /* happy */\n-\t\t} else if (!only_empty && !unlink(path->buf))\n+\t\t} else if (!only_empty &&\n+\t\t\t   (!unlink(path->buf) || errno == ENOENT)) {\n \t\t\tcontinue; /* happy, too */\n+\t\t}\n \n \t\t/* path too long, stat fails, or non-directory still exists */\n \t\tret = -1;\n@@ -1547,7 +1557,7 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)\n \n \tstrbuf_setlen(path, original_len);\n \tif (!ret && !keep_toplevel && !kept_down)\n-\t\tret = rmdir(path->buf);\n+\t\tret = (!rmdir(path->buf) || errno == ENOENT) ? 0 : -1;\n \telse if (kept_up)\n \t\t/*\n \t\t * report the uplevel that it is not an error that we\n-- \n1.8.5.2\n"},{"id":"233347","messageId":"1390085341-2553-15-git-send-email-mhagger@alum.mit.edu","threadId":"35687","inReplyTo":"1390085341-2553-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 14/17] rename_ref(): extract function rename_tmp_log()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-01-18T22:48:58Z","receivedAt":"2014-01-18T22:48:58Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"It's about to become a bit more complex.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c | 52 ++++++++++++++++++++++++++++++----------------------\n 1 file changed, 30 insertions(+), 22 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 4060ed1..d0fab39 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2528,6 +2528,35 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)\n  */\n #define TMP_RENAMED_LOG  \"logs/refs/.tmp-renamed-log\"\n \n+static int rename_tmp_log(const char *newrefname)\n+{\n+\tif (safe_create_leading_directories(git_path(\"logs/%s\", newrefname))) {\n+\t\terror(\"unable to create directory for %s\", newrefname);\n+\t\treturn -1;\n+\t}\n+\n+ retry:\n+\tif (rename(git_path(TMP_RENAMED_LOG), git_path(\"logs/%s\", newrefname))) {\n+\t\tif (errno==EISDIR || errno==ENOTDIR) {\n+\t\t\t/*\n+\t\t\t * rename(a, b) when b is an existing\n+\t\t\t * directory ought to result in ISDIR, but\n+\t\t\t * Solaris 5.8 gives ENOTDIR.  Sheesh.\n+\t\t\t */\n+\t\t\tif (remove_empty_directories(git_path(\"logs/%s\", newrefname))) {\n+\t\t\t\terror(\"Directory not empty: logs/%s\", newrefname);\n+\t\t\t\treturn -1;\n+\t\t\t}\n+\t\t\tgoto retry;\n+\t\t} else {\n+\t\t\terror(\"unable to move logfile \"TMP_RENAMED_LOG\" to logs/%s: %s\",\n+\t\t\t\tnewrefname, strerror(errno));\n+\t\t\treturn -1;\n+\t\t}\n+\t}\n+\treturn 0;\n+}\n+\n int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg)\n {\n \tunsigned char sha1[20], orig_sha1[20];\n@@ -2575,30 +2604,9 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\n \t\t}\n \t}\n \n-\tif (log && safe_create_leading_directories(git_path(\"logs/%s\", newrefname))) {\n-\t\terror(\"unable to create directory for %s\", newrefname);\n+\tif (log && rename_tmp_log(newrefname))\n \t\tgoto rollback;\n-\t}\n \n- retry:\n-\tif (log && rename(git_path(TMP_RENAMED_LOG), git_path(\"logs/%s\", newrefname))) {\n-\t\tif (errno==EISDIR || errno==ENOTDIR) {\n-\t\t\t/*\n-\t\t\t * rename(a, b) when b is an existing\n-\t\t\t * directory ought to result in ISDIR, but\n-\t\t\t * Solaris 5.8 gives ENOTDIR.  Sheesh.\n-\t\t\t */\n-\t\t\tif (remove_empty_directories(git_path(\"logs/%s\", newrefname))) {\n-\t\t\t\terror(\"Directory not empty: logs/%s\", newrefname);\n-\t\t\t\tgoto rollback;\n-\t\t\t}\n-\t\t\tgoto retry;\n-\t\t} else {\n-\t\t\terror(\"unable to move logfile \"TMP_RENAMED_LOG\" to logs/%s: %s\",\n-\t\t\t\tnewrefname, strerror(errno));\n-\t\t\tgoto rollback;\n-\t\t}\n-\t}\n \tlogmoved = log;\n \n \tlock = lock_ref_sha1_basic(newrefname, NULL, 0, NULL);\n-- \n1.8.5.2\n"},{"id":"233349","messageId":"1390085341-2553-16-git-send-email-mhagger@alum.mit.edu","threadId":"35687","inReplyTo":"1390085341-2553-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 15/17] rename_tmp_log(): handle a possible mkdir/rmdir race","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-01-18T22:48:59Z","receivedAt":"2014-01-18T22:48:59Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"If a directory vanishes while renaming the temporary reflog file,\nretry (up to 3 times).  This could happen if another process deletes\nthe directory created by safe_create_leading_directories() just before\nwe rename the file into the directory.\n\nAs far as I can tell, this race could not occur internal to git.  The\nonly time that a directory under $GIT_DIR/logs is deleted is if room\nhas to be made for a log file for a reference with the same name;\nfor example, in the following sequence:\n\n    git branch foo/bar    # Creates file .git/logs/refs/heads/foo/bar\n    git branch -d foo/bar # Deletes file but leaves .git/logs/refs/heads/foo/\n    git branch foo        # Deletes .git/logs/refs/heads/foo/\n\nBut the only reason the last command deletes the directory is because\nit wants to create a file with the same name.  So if another process\n(e.g.,\n\n    git branch foo/baz\n\n) wants to create that directory, one of the two is doomed to failure\nanyway because of a D/F conflict.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c | 11 ++++++++++-\n 1 file changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex d0fab39..a90904b 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2530,12 +2530,14 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)\n \n static int rename_tmp_log(const char *newrefname)\n {\n+\tint attempts_remaining = 3;\n+\n+ retry:\n \tif (safe_create_leading_directories(git_path(\"logs/%s\", newrefname))) {\n \t\terror(\"unable to create directory for %s\", newrefname);\n \t\treturn -1;\n \t}\n \n- retry:\n \tif (rename(git_path(TMP_RENAMED_LOG), git_path(\"logs/%s\", newrefname))) {\n \t\tif (errno==EISDIR || errno==ENOTDIR) {\n \t\t\t/*\n@@ -2548,6 +2550,13 @@ static int rename_tmp_log(const char *newrefname)\n \t\t\t\treturn -1;\n \t\t\t}\n \t\t\tgoto retry;\n+\t\t} else if (errno == ENOENT && --attempts_remaining > 0) {\n+\t\t\t/*\n+\t\t\t * Maybe another process just deleted one of\n+\t\t\t * the directories in the path to newrefname.\n+\t\t\t * Try again from the beginning.\n+\t\t\t */\n+\t\t\tgoto retry;\n \t\t} else {\n \t\t\terror(\"unable to move logfile \"TMP_RENAMED_LOG\" to logs/%s: %s\",\n \t\t\t\tnewrefname, strerror(errno));\n-- \n1.8.5.2\n"},{"id":"233355","messageId":"1390085341-2553-17-git-send-email-mhagger@alum.mit.edu","threadId":"35687","inReplyTo":"1390085341-2553-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 16/17] rename_tmp_log(): limit the number of remote_empty_directories() attempts","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-01-18T22:49:00Z","receivedAt":"2014-01-18T22:49:00Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This doesn't seem to be a likely error, but we've got the counter\nanyway, so we might as well use it for an added bit of safety.\n\nPlease note that the first call to rename() is optimistic, and it is\nnormal for it to fail if there is a directory in the way.  So bump the\ntotal number of allowed attempts to 4, to be sure that we can still\nhave at least 3 retries in the case of a race.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex a90904b..134d5aa 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2530,7 +2530,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)\n \n static int rename_tmp_log(const char *newrefname)\n {\n-\tint attempts_remaining = 3;\n+\tint attempts_remaining = 4;\n \n  retry:\n \tif (safe_create_leading_directories(git_path(\"logs/%s\", newrefname))) {\n@@ -2539,7 +2539,7 @@ static int rename_tmp_log(const char *newrefname)\n \t}\n \n \tif (rename(git_path(TMP_RENAMED_LOG), git_path(\"logs/%s\", newrefname))) {\n-\t\tif (errno==EISDIR || errno==ENOTDIR) {\n+\t\tif ((errno==EISDIR || errno==ENOTDIR) && --attempts_remaining > 0) {\n \t\t\t/*\n \t\t\t * rename(a, b) when b is an existing\n \t\t\t * directory ought to result in ISDIR, but\n-- \n1.8.5.2\n"},{"id":"233353","messageId":"1390085341-2553-18-git-send-email-mhagger@alum.mit.edu","threadId":"35687","inReplyTo":"1390085341-2553-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 17/17] rename_tmp_log(): on SCLD_VANISHED, retry","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-01-18T22:49:01Z","receivedAt":"2014-01-18T22:49:01Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"If safe_create_leading_directories() fails because a file along the\npath unexpectedly vanished, try again from the beginning.  Try at most\n4 times.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs.c | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex 134d5aa..0148bbd 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2533,7 +2533,14 @@ static int rename_tmp_log(const char *newrefname)\n \tint attempts_remaining = 4;\n \n  retry:\n-\tif (safe_create_leading_directories(git_path(\"logs/%s\", newrefname))) {\n+\tswitch (safe_create_leading_directories(git_path(\"logs/%s\", newrefname))) {\n+\tcase SCLD_OK:\n+\t\tbreak; /* success */\n+\tcase SCLD_VANISHED:\n+\t\tif (--attempts_remaining > 0)\n+\t\t\tgoto retry;\n+\t\t/* fall through */\n+\tdefault:\n \t\terror(\"unable to create directory for %s\", newrefname);\n \t\treturn -1;\n \t}\n-- \n1.8.5.2\n"}]}