Support: Don't call close again if we get EINTR

Most Unix-like operating systems guarantee that the file descriptor is
closed after a call to close(2), even if close comes back with EINTR.
For these systems, calling close _again_ will either do nothing or close
some other file descriptor open(2)'d by another thread. (Linux)

However, some operating systems do not have this behavior.  They require
at least another call to close(2) before guaranteeing that the
descriptor is closed. (HP-UX)

And some operating systems have an unpredictable blend of the two
behaviors! (xnu)

Avoid this disaster by blocking all signals before we call close(2).
This ensures that a signal will not be delivered to the thread and
close(2) will not give us back EINTR.  We restore the signal mask once
the operation is done.

N.B. This isn't a problem on Windows, it doesn't have a notion of EINTR
because signals always get delivered to dedicated signal handling
threads.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@219189 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/Support/Process.h b/include/llvm/Support/Process.h
index bb7467bfc83ee13ed543e9cf178a72be0b0611ae..861667927d8a9bb8dc9587525c9f1b2625b8ad98 100644 (file)
--- a/include/llvm/Support/Process.h
+++ b/include/llvm/Support/Process.h
@@ -192,6 +192,15 @@ public:
   // components should not call this.
   static std::error_code FixupStandardFileDescriptors();
 
+  // This function safely closes a file descriptor.  It is not safe to retry
+  // close(2) when it returns with errno equivalent to EINTR; this is because
+  // *nixen cannot agree if the file descriptor is, in fact, closed when this
+  // occurs.
+  //
+  // N.B. Some operating systems, due to thread cancellation, cannot properly
+  // guarantee that it will or will not be closed one way or the other!
+  static std::error_code SafelyCloseFileDescriptor(int FD);
+
   /// This function determines if the standard input is connected directly
   /// to a user's input (keyboard probably), rather than coming from a file
   /// or pipe.

diff --git a/lib/Support/Unix/Process.inc b/lib/Support/Unix/Process.inc
index 01c78cbc0da184d3b82e65354ed973c29369b38b..d39443b1124ff30ee7fa619729da8a777968d0c3 100644 (file)
--- a/lib/Support/Unix/Process.inc
+++ b/lib/Support/Unix/Process.inc
@@ -258,6 +258,30 @@ std::error_code Process::FixupStandardFileDescriptors() {
   return std::error_code();
 }
 
+std::error_code Process::SafelyCloseFileDescriptor(int FD) {
+  // Create a signal set filled with *all* signals.
+  sigset_t FullSet;
+  if (sigfillset(&FullSet) < 0)
+    return std::error_code(errno, std::generic_category());
+  // Atomically swap our current signal mask with a full mask.
+  sigset_t SavedSet;
+  if (int EC = pthread_sigmask(SIG_SETMASK, &FullSet, &SavedSet))
+    return std::error_code(EC, std::generic_category());
+  // Attempt to close the file descriptor.
+  // We need to save the error, if one occurs, because our subsequent call to
+  // pthread_sigmask might tamper with errno.
+  int ErrnoFromClose = 0;
+  if (::close(FD) < 0)
+    ErrnoFromClose = errno;
+  // Restore the signal mask back to what we saved earlier.
+  int EC = pthread_sigmask(SIG_SETMASK, &SavedSet, nullptr);
+  // The error code from close takes precedence over the one from
+  // pthread_sigmask.
+  if (ErrnoFromClose)
+    return std::error_code(ErrnoFromClose, std::generic_category());
+  return std::error_code(EC, std::generic_category());
+}
+
 bool Process::StandardInIsUserInput() {
   return FileDescriptorIsDisplayed(STDIN_FILENO);
 }

diff --git a/lib/Support/Windows/Process.inc b/lib/Support/Windows/Process.inc
index db87d8ed60ed847cad342dac61cc50a4eca59dc9..3819e638c72eed6c5c6857305e4cc9ccef71d5fc 100644 (file)
--- a/lib/Support/Windows/Process.inc
+++ b/lib/Support/Windows/Process.inc
@@ -277,6 +277,12 @@ std::error_code Process::FixupStandardFileDescriptors() {
   return std::error_code();
 }
 
+std::error_code Process::SafelyCloseFileDescriptor(int FD) {
+  if (::close(FD) < 0)
+    return std::error_code(errno, std::generic_category());
+  return std::error_code();
+}
+
 bool Process::StandardInIsUserInput() {
   return FileDescriptorIsDisplayed(0);
 }

diff --git a/lib/Support/raw_ostream.cpp b/lib/Support/raw_ostream.cpp
index c2c55cff7ed9ee600cf3723807c08ba589187124..bbbbe4ae8c655bbcd8b1f0edce13dbf7236d89a3 100644 (file)
--- a/lib/Support/raw_ostream.cpp
+++ b/lib/Support/raw_ostream.cpp
@@ -536,12 +536,8 @@ raw_fd_ostream::raw_fd_ostream(int fd, bool shouldClose, bool unbuffered)
 raw_fd_ostream::~raw_fd_ostream() {
   if (FD >= 0) {
     flush();
-    if (ShouldClose)
-      while (::close(FD) != 0)
-        if (errno != EINTR) {
-          error_detected();
-          break;
-        }
+    if (ShouldClose && sys::Process::SafelyCloseFileDescriptor(FD))
+      error_detected();
   }
 
 #ifdef __MINGW32__
@@ -615,11 +611,8 @@ void raw_fd_ostream::close() {
   assert(ShouldClose);
   ShouldClose = false;
   flush();
-  while (::close(FD) != 0)
-    if (errno != EINTR) {
-      error_detected();
-      break;
-    }
+  if (sys::Process::SafelyCloseFileDescriptor(FD))
+    error_detected();
   FD = -1;
 }