* syscalls.cc (NT_TRANSACTIONAL_ERROR): Define.

	(stop_transaction): Take "trans" HANDLE by reference and set it to
	NULL after closing it.
	(unlink_nt): If NtOpenFile fails due to a transactional error, stop
	transaction and retry NtOpenFile.  Simplify check for having to call
	stop_transaction.
	(rename): If NtOpenFile fails due to a transactional error, stop
	transaction and retry NtOpenFile in both affected cases.  Simplify check
	for having to call stop_transaction and add comment from unlink_nt.

3 files changed, 57 insertions, 9 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 087550d..0b0b9fd 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,5 +1,17 @@
 2013-12-11  Corinna Vinschen  <corinna@vinschen.de>
 
+	* syscalls.cc (NT_TRANSACTIONAL_ERROR): Define.
+	(stop_transaction): Take "trans" HANDLE by reference and set it to
+	NULL after closing it.
+	(unlink_nt): If NtOpenFile fails due to a transactional error, stop
+	transaction and retry NtOpenFile.  Simplify check for having to call
+	stop_transaction.
+	(rename): If NtOpenFile fails due to a transactional error, stop
+	transaction and retry NtOpenFile in both affected cases.  Simplify check
+	for having to call stop_transaction and add comment from unlink_nt.
+
+2013-12-11  Corinna Vinschen  <corinna@vinschen.de>
+
 	* mount.cc (fs_info::update): Fix formatting.
 
 2013-12-10  Corinna Vinschen  <corinna@vinschen.de>
diff --git a/winsup/cygwin/release/1.7.28 b/winsup/cygwin/release/1.7.28
index 7769171..1bc1c01 100644
--- a/winsup/cygwin/release/1.7.28
+++ b/winsup/cygwin/release/1.7.28
@@ -8,3 +8,8 @@ Bug Fixes
 - Signals should no longer hang when they occur within a low-level
   Windows DLL.
   Fixes: http://cygwin.com/ml/cygwin/2013-12/threads.html#00151
+
+- If it turns out that transactions don't work during unlink(2) or rename(2),
+  despite the fact that the filesystem claims to handle them, stop transaction
+  and try again without.
+  Fixes: http://cygwin.com/ml/cygwin/2013-12/msg00119.html
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index ba43078..c482159 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -185,6 +185,11 @@ dup3 (int oldfd, int newfd, int flags)
   return res;
 }
 
+/* Define macro to simplify checking for a transactional error code. */
+#define NT_TRANSACTIONAL_ERROR(s)	\
+		(((ULONG)(s) >= (ULONG)STATUS_TRANSACTIONAL_CONFLICT) \
+		 && ((ULONG)(s) <= (ULONG)STATUS_LOG_GROWTH_FAILED))
+
 static inline void
 start_transaction (HANDLE &old_trans, HANDLE &trans)
 {
@@ -204,7 +209,7 @@ start_transaction (HANDLE &old_trans, HANDLE &trans)
 }
 
 static inline NTSTATUS
-stop_transaction (NTSTATUS status, HANDLE old_trans, HANDLE trans)
+stop_transaction (NTSTATUS status, HANDLE old_trans, HANDLE &trans)
 {
   RtlSetCurrentTransaction (old_trans);
   if (NT_SUCCESS (status))
@@ -212,6 +217,7 @@ stop_transaction (NTSTATUS status, HANDLE old_trans, HANDLE trans)
   else
     status = NtRollbackTransaction (trans, TRUE);
   NtClose (trans);
+  trans = NULL;
   return status;
 }
 
@@ -702,7 +708,7 @@ unlink_nt (path_conv &pc)
       if (wincap.has_transactions ()
 	  && (pc.fs_flags () & FILE_SUPPORTS_TRANSACTIONS))
 	start_transaction (old_trans, trans);
-
+retry_open:
       status = NtOpenFile (&fh_ro, FILE_WRITE_ATTRIBUTES, &attr, &io,
 			   FILE_SHARE_VALID_FLAGS, flags);
       if (NT_SUCCESS (status))
@@ -718,8 +724,18 @@ unlink_nt (path_conv &pc)
 	  pc.init_reopen_attr (&attr, fh_ro);
 	}
       else
-	debug_printf ("Opening %S for removing R/O failed, status = %y",
-		      pc.get_nt_native_path (), status);
+	{
+	  debug_printf ("Opening %S for removing R/O failed, status = %y",
+			pc.get_nt_native_path (), status);
+	  if (NT_TRANSACTIONAL_ERROR (status) && trans)
+	    {
+	      /* If NtOpenFile fails due to transactional problems, stop
+		 transaction and go ahead without. */
+	      stop_transaction (status, old_trans, trans);
+	      debug_printf ("Transaction failure.  Retry open.");
+	      goto retry_open;
+	    }
+	}
       if (pc.is_lnk_symlink ())
 	{
 	  status = NtQueryInformationFile (fh_ro, &io, &fsi, sizeof fsi,
@@ -984,11 +1000,8 @@ try_again:
     }
 out:
   /* Stop transaction if we started one. */
-  if ((access & FILE_WRITE_ATTRIBUTES)
-      && wincap.has_transactions ()
-      && (pc.fs_flags () & FILE_SUPPORTS_TRANSACTIONS))
+  if (trans)
     stop_transaction (status, old_trans, trans);
-
   syscall_printf ("%S, return status = %y", pc.get_nt_native_path (), status);
   return status;
 }
@@ -2421,6 +2434,14 @@ retry:
 	      goto retry;
 	    }
 	}
+      else if (NT_TRANSACTIONAL_ERROR (status) && trans)
+	{
+	  /* If NtOpenFile fails due to transactional problems, stop
+	     transaction and go ahead without. */
+	  stop_transaction (status, old_trans, trans);
+	  debug_printf ("Transaction failure.  Retry open.");
+	  goto retry;
+	}
       __seterrno_from_nt_status (status);
       goto out;
     }
@@ -2542,6 +2563,7 @@ retry:
 	     Fortunately nothing has happened yet, so the atomicity of the
 	     rename functionality is not spoiled. */
 	  NtClose (fh);
+retry_reopen:
 	  status = NtOpenFile (&fh, DELETE,
 			       oldpc.get_object_attr (attr, sec_none_nih),
 			       &io, FILE_SHARE_VALID_FLAGS,
@@ -2550,6 +2572,14 @@ retry:
 				  ? FILE_OPEN_REPARSE_POINT : 0));
 	  if (!NT_SUCCESS (status))
 	    {
+	      if (NT_TRANSACTIONAL_ERROR (status) && trans)
+		{
+		  /* If NtOpenFile fails due to transactional problems, stop
+		     transaction and go ahead without. */
+		  stop_transaction (status, old_trans, trans);
+		  debug_printf ("Transaction failure.  Retry open.");
+		  goto retry_reopen;
+		}
 	      __seterrno_from_nt_status (status);
 	      goto out;
 	    }
@@ -2571,7 +2601,8 @@ retry:
 out:
   if (fh)
     NtClose (fh);
-  if (wincap.has_transactions () && trans)
+  /* Stop transaction if we started one. */
+  if (trans)
     stop_transaction (status, old_trans, trans);
   syscall_printf ("%R = rename(%s, %s)", res, oldpath, newpath);
   return res;