[PATCH v2 07/18] mingw: factor out the retry logic
From: Karsten Blees <karsten.blees@gmail.com>
In several places, Git's Windows-specific code follows the pattern where it tries to perform an operation, and retries several times when that operation fails, sleeping an increasing amount of time, before finally giving up and asking the user whether to rety (after, say, closing an editor that held a handle to a file, preventing the operation from succeeding).
This logic is a bit hard to use, and inconsistent: `mingw_unlink()` and `mingw_rmdir()` duplicate the code to retry, and both of them do so incompletely. They also do not restore `errno` if the user answers 'no'.
Introduce a `retry_ask_yes_no()` helper function that handles retry with small delay, asking the user, and restoring `errno`.
Note that in `mingw_unlink()`, we include the `_wchmod()` call in the retry loop (which may fail if the file is locked exclusively).
In `mingw_rmdir()`, we include special error handling in the retry loop.
Signed-off-by: Karsten Blees <karsten.blees@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
compat/mingw.c | 104 ++++++++++++++++++++++---------------------------
1 file changed, 46 insertions(+), 58 deletions(-)
Show changes to compat/mingw.c +46 −58
diff --git a/compat/mingw.c b/compat/mingw.c
index c7571951dc..26e64c6a5a 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -28,8 +28,6 @@
#define HCAST(type, handle) ((type)(intptr_t)handle)
-static const int delay[] = { 0, 1, 10, 20, 40 };
-
void open_in_gdb(void)
{
static struct child_process cp = CHILD_PROCESS_INIT;
@@ -205,15 +203,12 @@ static int read_yes_no_answer(void)
return -1;
}
-static int ask_yes_no_if_possible(const char *format, ...)
+static int ask_yes_no_if_possible(const char *format, va_list args)
{
char question[4096];
const char *retry_hook;
- va_list args;
- va_start(args, format);
vsnprintf(question, sizeof(question), format, args);
- va_end(args);
retry_hook = mingw_getenv("GIT_ASK_YESNO");
if (retry_hook) {
@@ -238,6 +233,31 @@ static int ask_yes_no_if_possible(const char *format, ...)
}
}
+static int retry_ask_yes_no(int *tries, const char *format, ...)
+{
+ static const int delay[] = { 0, 1, 10, 20, 40 };
+ va_list args;
+ int result, saved_errno = errno;
+
+ if ((*tries) < ARRAY_SIZE(delay)) {
+ /*
+ * We assume that some other process had the file open at the wrong
+ * moment and retry. In order to give the other process a higher
+ * chance to complete its operation, we give up our time slice now.
+ * If we have to retry again, we do sleep a bit.
+ */
+ Sleep(delay[*tries]);
+ (*tries)++;
+ return 1;
+ }
+
+ va_start(args, format);
+ result = ask_yes_no_if_possible(format, args);
+ va_end(args);
+ errno = saved_errno;
+ return result;
+}
+
/* Windows only */
enum hide_dotfiles_type {
HIDE_DOTFILES_FALSE = 0,
@@ -298,7 +318,7 @@ static wchar_t *normalize_ntpath(wchar_t *wbuf)
int mingw_unlink(const char *pathname, int handle_in_use_error)
{
- int ret, tries = 0;
+ int tries = 0;
wchar_t wpathname[MAX_PATH];
if (xutftowcs_path(wpathname, pathname) < 0)
return -1;
@@ -306,29 +326,19 @@ int mingw_unlink(const char *pathname, int handle_in_use_error)
if (DeleteFileW(wpathname))
return 0;
- /* read-only files cannot be removed */
- _wchmod(wpathname, 0666);
- while ((ret = _wunlink(wpathname)) == -1 && tries < ARRAY_SIZE(delay)) {
+ do {
+ /* read-only files cannot be removed */
+ _wchmod(wpathname, 0666);
+ if (!_wunlink(wpathname))
+ return 0;
if (!is_file_in_use_error(GetLastError()))
break;
if (!handle_in_use_error)
- return ret;
+ return -1;
- /*
- * We assume that some other process had the source or
- * destination file open at the wrong moment and retry.
- * In order to give the other process a higher chance to
- * complete its operation, we give up our time slice now.
- * If we have to retry again, we do sleep a bit.
- */
- Sleep(delay[tries]);
- tries++;
- }
- while (ret == -1 && is_file_in_use_error(GetLastError()) &&
- ask_yes_no_if_possible("Unlink of file '%s' failed. "
- "Should I try again?", pathname))
- ret = _wunlink(wpathname);
- return ret;
+ } while (retry_ask_yes_no(&tries, "Unlink of file '%s' failed. "
+ "Should I try again?", pathname));
+ return -1;
}
static int is_dir_empty(const wchar_t *wpath)
@@ -355,7 +365,7 @@ static int is_dir_empty(const wchar_t *wpath)
int mingw_rmdir(const char *pathname)
{
- int ret, tries = 0;
+ int tries = 0;
wchar_t wpathname[MAX_PATH];
struct stat st;
@@ -381,7 +391,11 @@ int mingw_rmdir(const char *pathname)
if (xutftowcs_path(wpathname, pathname) < 0)
return -1;
- while ((ret = _wrmdir(wpathname)) == -1 && tries < ARRAY_SIZE(delay)) {
+ do {
+ if (!_wrmdir(wpathname)) {
+ invalidate_lstat_cache();
+ return 0;
+ }
if (!is_file_in_use_error(GetLastError()))
errno = err_win_to_posix(GetLastError());
if (errno != EACCES)
@@ -390,23 +404,9 @@ int mingw_rmdir(const char *pathname)
errno = ENOTEMPTY;
break;
}
- /*
- * We assume that some other process had the source or
- * destination file open at the wrong moment and retry.
- * In order to give the other process a higher chance to
- * complete its operation, we give up our time slice now.
- * If we have to retry again, we do sleep a bit.
- */
- Sleep(delay[tries]);
- tries++;
- }
- while (ret == -1 && errno == EACCES && is_file_in_use_error(GetLastError()) &&
- ask_yes_no_if_possible("Deletion of directory '%s' failed. "
- "Should I try again?", pathname))
- ret = _wrmdir(wpathname);
- if (!ret)
- invalidate_lstat_cache();
- return ret;
+ } while (retry_ask_yes_no(&tries, "Deletion of directory '%s' failed. "
+ "Should I try again?", pathname));
+ return -1;
}
static inline int needs_hiding(const char *path)
@@ -2384,20 +2384,8 @@ repeat:
SetFileAttributesW(wpnew, attrs);
}
}
- if (tries < ARRAY_SIZE(delay) && gle == ERROR_ACCESS_DENIED) {
- /*
- * We assume that some other process had the source or
- * destination file open at the wrong moment and retry.
- * In order to give the other process a higher chance to
- * complete its operation, we give up our time slice now.
- * If we have to retry again, we do sleep a bit.
- */
- Sleep(delay[tries]);
- tries++;
- goto repeat;
- }
if (gle == ERROR_ACCESS_DENIED &&
- ask_yes_no_if_possible("Rename from '%s' to '%s' failed. "
+ retry_ask_yes_no(&tries, "Rename from '%s' to '%s' failed. "
"Should I try again?", pold, pnew))
goto repeat;
--
gitgitgadget
Johannes Schindelin via GitGitGadget
