[PATCH] Introduce an exponential-backoff-retry in unlink on mingw.

[PATCH] Introduce an exponential-backoff-retry in unlink on mingw.

[Ludvig Strigeus]

This is to work around that you can't delete a file while open on Windows.
Visual Studio and other IDEs that have a change-notification-handler
set up on the source code tree, will stat all files inside of the
directory each time it changes, for example when doing git checkout.
If git checkout is unlucky, the file handle will be open exactly
the same time as git checkout is trying to delete the file, causing the
checkout to fail.

A backoff is introduced that retries the unlink a few times with a
small delay in between.

The code has logic to stop the exponential backoff if it detects
it's of no use, to prevent git from taking massively long time if
files are read only for a valid reason.
---
 compat/mingw.c |   60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 compat/mingw.h |    7 +-----
 2 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 10d6796..677f3a6 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -375,6 +375,66 @@ int gettimeofday(struct timeval *tv, void *tz)
 	return 0;
 }
 
+/*
+ * It's not possible to delete a file while in use on Windows
+ * This frequently happens when having files open in Microsoft
+ * Visual Studio. When switching branch it seems to rapidly
+ * open/stat files at the same time as git is checking files out,
+ * causing the unlink to fail with EACCES.
+ * This code introduces an exponential delay in case
+ * of permission denied errors.
+ * With these parameters we wait approximately
+ * max half a second extra in total with the follow retry times:
+ * 16ms, 32ms, 64ms, 128ms, 256ms
+ */
+
+#define NUM_UNLINK_RETRIES 5
+#define UNLINK_SLEEP_TIME 32
+
+/* Made this volatile in case unlink is called from many threads */
+static volatile int unlink_retries = NUM_UNLINK_RETRIES;
+
+int mingw_unlink(const char *pathname)
+{
+	int r;
+	int tries_left = unlink_retries;
+	int sleepms = UNLINK_SLEEP_TIME; 
+	DWORD attrs;
+	
+	/* read-only files cannot be removed */
+	chmod(pathname, 0666);
+	
+	for(;;) {
+		r = unlink(pathname);
+		if (r == 0 || errno != EACCES) {
+			/* If the sleep helped, then reset the retry counter */
+			if (sleepms != UNLINK_SLEEP_TIME)
+				unlink_retries = NUM_UNLINK_RETRIES;
+			return r;
+		}
+
+		/* No point in sleeping if unlink is called on a directory or invalid file */
+		attrs = GetFileAttributes(pathname);
+		if (attrs == INVALID_FILE_ATTRIBUTES || (attrs & FILE_ATTRIBUTE_DIRECTORY))
+			return r;
+
+		if (--tries_left < 0) {
+			/* 
+			 * Reduce the retry count to avoid having to wait so long
+			 * next time in case we get these errors for a reason
+			 * other than windows being stupid.
+			 */
+			int t = unlink_retries;
+			unlink_retries = (t > 0) ? (t - 1) : 0;
+			return r;
+		}
+		
+		Sleep(sleepms);
+		sleepms *= 2;
+	}
+}
+
+
 int pipe(int filedes[2])
 {
 	int fd;
diff --git a/compat/mingw.h b/compat/mingw.h
index 1b528da..9163edd 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -98,12 +98,7 @@ static inline int fcntl(int fd, int cmd, long arg)
  * simple adaptors
  */
 
-static inline int mingw_unlink(const char *pathname)
-{
-	/* read-only files cannot be removed */
-	chmod(pathname, 0666);
-	return unlink(pathname);
-}
+int mingw_unlink(const char *pathname);
 #define unlink mingw_unlink
 
 static inline int waitpid(pid_t pid, int *status, unsigned options)
-- 
1.6.5.1.1367.gcd48.dirty

Re: [PATCH] Introduce an exponential-backoff-retry in unlink on mingw.

[Johannes Sixt]

Ludvig Strigeus schrieb:
> A backoff is introduced that retries the unlink a few times with a
> small delay in between.

Thanks. Did you notice that there is already a similar strategy 
implemented in mingw_rename? It starts with Sleep(0).

A similar patch was floated recently on the msysgit mailing list. Please 
research the archives.

-- Hannes