[PATCH] D81803: [Support] PR42623: Avoid setting the delete-on-close bit for TempFile

[Ronald Wampler via Phabricator via llvm-commits]

rdwampler created this revision.
rdwampler added reviewers: zturner, pcc, JDevlieghere, espindola.
rdwampler added a project: LLVM.
Herald added subscribers: llvm-commits, hiraditya.
rdwampler added a comment.

I noticed this first when using llvm-ar (LLVM 10), which worked on mapped network drive with LLVM 6.0 and bisected it to the commit in the message. I'm not familiar with Windows api so maybe there is a better way?


On Windows, after commit 881ba104656c40098d4bc90c52613c08136f0fe1, tools
using TempFile would error with "bad file descriptor" when writing the file
on a network drive. It appears that setting the delete-on-close bit via
`SetFileInformationByHandle/FileDispositionInfo` prevented it from
 accessing the file on network drive, and although using `FILE_DISPOSITION_INFO`
 seems to work, it causes other troubles. The situation where a temp file
 won't be delete without the delete-on-close bit set seems rare so just avoid
 setting it entirely.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81803

Files:
  llvm/lib/Support/Path.cpp
  llvm/lib/Support/Windows/Path.inc


Index: llvm/lib/Support/Windows/Path.inc
===================================================================
--- llvm/lib/Support/Windows/Path.inc
+++ llvm/lib/Support/Windows/Path.inc
@@ -250,7 +250,7 @@
     return ec;
   if (std::error_code ec = widenPath(to, wide_to))
     return ec;
-
+	
   if (!::CreateHardLinkW(wide_from.begin(), wide_to.begin(), NULL))
     return mapWindowsError(::GetLastError());
 
@@ -402,15 +402,6 @@
   return is_local_internal(FinalPath, Result);
 }
 
-static std::error_code setDeleteDisposition(HANDLE Handle, bool Delete) {
-  FILE_DISPOSITION_INFO Disposition;
-  Disposition.DeleteFile = Delete;
-  if (!SetFileInformationByHandle(Handle, FileDispositionInfo, &Disposition,
-                                  sizeof(Disposition)))
-    return mapWindowsError(::GetLastError());
-  return std::error_code();
-}
-
 static std::error_code rename_internal(HANDLE FromHandle, const Twine &To,
                                        bool ReplaceIfExists) {
   SmallVector<wchar_t, 0> ToWide;
@@ -1166,12 +1157,6 @@
     }
   }
 
-  if (Flags & OF_Delete) {
-    if ((EC = setDeleteDisposition(Result, true))) {
-      ::CloseHandle(Result);
-      return errorCodeToError(EC);
-    }
-  }
   return Result;
 }
 
Index: llvm/lib/Support/Path.cpp
===================================================================
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -1199,24 +1199,6 @@
   assert(!Done);
   Done = true;
   // Always try to close and rename.
-#ifdef _WIN32
-  // If we can't cancel the delete don't rename.
-  auto H = reinterpret_cast<HANDLE>(_get_osfhandle(FD));
-  std::error_code RenameEC = setDeleteDisposition(H, false);
-  if (!RenameEC) {
-    RenameEC = rename_fd(FD, Name);
-    // If rename failed because it's cross-device, copy instead
-    if (RenameEC ==
-      std::error_code(ERROR_NOT_SAME_DEVICE, std::system_category())) {
-      RenameEC = copy_file(TmpName, Name);
-      setDeleteDisposition(H, true);
-    }
-  }
-
-  // If we can't rename, discard the temporary file.
-  if (RenameEC)
-    setDeleteDisposition(H, true);
-#else
   std::error_code RenameEC = fs::rename(TmpName, Name);
   if (RenameEC) {
     // If we can't rename, try to copy to work around cross-device link issues.
@@ -1226,7 +1208,6 @@
       remove(TmpName);
   }
   sys::DontRemoveFileOnSignal(TmpName);
-#endif
 
   if (!RenameEC)
     TmpName = "";
@@ -1244,13 +1225,7 @@
   assert(!Done);
   Done = true;
 
-#ifdef _WIN32
-  auto H = reinterpret_cast<HANDLE>(_get_osfhandle(FD));
-  if (std::error_code EC = setDeleteDisposition(H, false))
-    return errorCodeToError(EC);
-#else
   sys::DontRemoveFileOnSignal(TmpName);
-#endif
 
   TmpName = "";
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D81803.270613.patch
Type: text/x-patch
Size: 2712 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200614/92a23855/attachment.bin>