From c4c61c763e700d02344490590d6980ee51031a27 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 18 Jan 2014 23:48:54 +0100 Subject: lock_ref_sha1_basic(): on SCLD_VANISHED, retry If safe_create_leading_directories() fails because a file along the path unexpectedly vanished, try again (up to 3 times). This can occur if another process is deleting directories at the same time as we are trying to make them. For example, "git pack-refs --all" tries to delete the loose refs and any empty directories that are left behind. If a pack-refs process is running, then it might delete a directory that we need to put a new loose reference in. If safe_create_leading_directories() thinks this might have happened, then take its advice and try again (maximum three attempts). Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 5e5a3824b..2d0faf232 100644 --- a/refs.c +++ b/refs.c @@ -2039,6 +2039,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, int type, lflags; int mustexist = (old_sha1 && !is_null_sha1(old_sha1)); int missing = 0; + int attempts_remaining = 3; lock = xcalloc(1, sizeof(struct ref_lock)); lock->lock_fd = -1; @@ -2093,7 +2094,15 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, if ((flags & REF_NODEREF) && (type & REF_ISSYMREF)) lock->force_write = 1; - if (safe_create_leading_directories(ref_file)) { + retry: + switch (safe_create_leading_directories(ref_file)) { + case SCLD_OK: + break; /* success */ + case SCLD_VANISHED: + if (--attempts_remaining > 0) + goto retry; + /* fall through */ + default: last_errno = errno; error("unable to create directory for %s", ref_file); goto error_return; -- cgit v1.2.1 From e5c223e98b985d9faa274039aefa4391d2da25a6 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 18 Jan 2014 23:48:55 +0100 Subject: lock_ref_sha1_basic(): if locking fails with ENOENT, retry If hold_lock_file_for_update() fails with errno==ENOENT, it might be because somebody else (for example, a pack-refs process) has just deleted one of the lockfile's ancestor directories. So if this condition is detected, try again (up to 3 times). Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 2d0faf232..a09bbb768 100644 --- a/refs.c +++ b/refs.c @@ -2081,7 +2081,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, lock->lk = xcalloc(1, sizeof(struct lock_file)); - lflags = LOCK_DIE_ON_ERROR; + lflags = 0; if (flags & REF_NODEREF) { refname = orig_refname; lflags |= LOCK_NODEREF; @@ -2109,6 +2109,17 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, } lock->lock_fd = hold_lock_file_for_update(lock->lk, ref_file, lflags); + if (lock->lock_fd < 0) { + if (errno == ENOENT && --attempts_remaining > 0) + /* + * Maybe somebody just deleted one of the + * directories leading to ref_file. Try + * again: + */ + goto retry; + else + unable_to_lock_index_die(ref_file, errno); + } return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock; error_return: -- cgit v1.2.1 From fa59ae7971498157370d935adbb2bfc28012aa9f Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 18 Jan 2014 23:48:58 +0100 Subject: rename_ref(): extract function rename_tmp_log() It's about to become a bit more complex. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 52 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 22 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index a09bbb768..827e543b0 100644 --- a/refs.c +++ b/refs.c @@ -2528,6 +2528,35 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt) */ #define TMP_RENAMED_LOG "logs/refs/.tmp-renamed-log" +static int rename_tmp_log(const char *newrefname) +{ + 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) { + /* + * rename(a, b) when b is an existing + * directory ought to result in ISDIR, but + * Solaris 5.8 gives ENOTDIR. Sheesh. + */ + if (remove_empty_directories(git_path("logs/%s", newrefname))) { + error("Directory not empty: logs/%s", newrefname); + return -1; + } + goto retry; + } else { + error("unable to move logfile "TMP_RENAMED_LOG" to logs/%s: %s", + newrefname, strerror(errno)); + return -1; + } + } + return 0; +} + int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg) { unsigned char sha1[20], orig_sha1[20]; @@ -2575,30 +2604,9 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms } } - if (log && safe_create_leading_directories(git_path("logs/%s", newrefname))) { - error("unable to create directory for %s", newrefname); + if (log && rename_tmp_log(newrefname)) goto rollback; - } - retry: - if (log && rename(git_path(TMP_RENAMED_LOG), git_path("logs/%s", newrefname))) { - if (errno==EISDIR || errno==ENOTDIR) { - /* - * rename(a, b) when b is an existing - * directory ought to result in ISDIR, but - * Solaris 5.8 gives ENOTDIR. Sheesh. - */ - if (remove_empty_directories(git_path("logs/%s", newrefname))) { - error("Directory not empty: logs/%s", newrefname); - goto rollback; - } - goto retry; - } else { - error("unable to move logfile "TMP_RENAMED_LOG" to logs/%s: %s", - newrefname, strerror(errno)); - goto rollback; - } - } logmoved = log; lock = lock_ref_sha1_basic(newrefname, NULL, 0, NULL); -- cgit v1.2.1 From ae4a283e3b4cdc99a548d215739539c3a690f290 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 18 Jan 2014 23:48:59 +0100 Subject: rename_tmp_log(): handle a possible mkdir/rmdir race 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 Signed-off-by: Junio C Hamano --- refs.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 827e543b0..bbcdc88e4 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)); -- cgit v1.2.1 From f1e9e9a4dbe2cfb39dcb14ee4f34628ef46d7b15 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 18 Jan 2014 23:49:00 +0100 Subject: rename_tmp_log(): limit the number of remote_empty_directories() attempts This doesn't seem to be a likely error, but we've got the counter anyway, so we might as well use it for an added bit of safety. Please note that the first call to rename() is optimistic, and it is normal for it to fail if there is a directory in the way. So bump the total number of allowed attempts to 4, to be sure that we can still have at least 3 retries in the case of a race. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index bbcdc88e4..fd644f0f4 100644 --- a/refs.c +++ b/refs.c @@ -2530,7 +2530,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt) static int rename_tmp_log(const char *newrefname) { - int attempts_remaining = 3; + int attempts_remaining = 4; retry: if (safe_create_leading_directories(git_path("logs/%s", newrefname))) { @@ -2539,7 +2539,7 @@ static int rename_tmp_log(const char *newrefname) } if (rename(git_path(TMP_RENAMED_LOG), git_path("logs/%s", newrefname))) { - if (errno==EISDIR || errno==ENOTDIR) { + if ((errno==EISDIR || errno==ENOTDIR) && --attempts_remaining > 0) { /* * rename(a, b) when b is an existing * directory ought to result in ISDIR, but -- cgit v1.2.1 From 08f555cb82f92797ca0aa0d6ba32e6872f1331e5 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 18 Jan 2014 23:49:01 +0100 Subject: rename_tmp_log(): on SCLD_VANISHED, retry If safe_create_leading_directories() fails because a file along the path unexpectedly vanished, try again from the beginning. Try at most 4 times. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index fd644f0f4..703b5a2f4 100644 --- a/refs.c +++ b/refs.c @@ -2533,7 +2533,14 @@ static int rename_tmp_log(const char *newrefname) int attempts_remaining = 4; retry: - if (safe_create_leading_directories(git_path("logs/%s", newrefname))) { + switch (safe_create_leading_directories(git_path("logs/%s", newrefname))) { + case SCLD_OK: + break; /* success */ + case SCLD_VANISHED: + if (--attempts_remaining > 0) + goto retry; + /* fall through */ + default: error("unable to create directory for %s", newrefname); return -1; } -- cgit v1.2.1