Skip to content

Commit

Permalink
Win32: factor out retry logic
Browse files Browse the repository at this point in the history
The retry pattern is duplicated in three places. It also seems to be too
hard to use: 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.

mingw_unlink: include _wchmod in the retry loop (which may fail if the
file is locked exclusively).

mingw_rmdir: include special error handling in the retry loop.

Signed-off-by: Karsten Blees <blees@dcon.de>
  • Loading branch information
kblees authored and dscho committed Sep 24, 2024
1 parent d78d40a commit 07ad87a
Showing 1 changed file with 45 additions and 57 deletions.
102 changes: 45 additions & 57 deletions compat/mingw.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,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;
Expand Down Expand Up @@ -202,15 +200,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) {
Expand All @@ -235,6 +230,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,
Expand Down Expand Up @@ -336,34 +356,24 @@ static wchar_t *normalize_ntpath(wchar_t *wbuf)

int mingw_unlink(const char *pathname)
{
int ret, tries = 0;
int tries = 0;
wchar_t wpathname[MAX_LONG_PATH];
if (xutftowcs_long_path(wpathname, pathname) < 0)
return -1;

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;
/*
* 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)
Expand All @@ -390,7 +400,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_LONG_PATH];
struct stat st;

Expand All @@ -416,7 +426,11 @@ int mingw_rmdir(const char *pathname)
if (xutftowcs_long_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)
Expand All @@ -425,23 +439,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)
Expand Down Expand Up @@ -2445,20 +2445,8 @@ int mingw_rename(const char *pold, const char *pnew)
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;

Expand Down

0 comments on commit 07ad87a

Please sign in to comment.