Add temp_failure_retry helper function

[labath]

Summary

It is intended to wrap functions which can fail with EINTR (which we
have a surprising number of). It is inspired by the TEMP_FAILURE_RETRY
macro in glibc, but I've c++-ified it and made it more generic (by
specifying an explicit fail value), so it can also be used for functions
like fopen(3).

[krytarowski]

source/Plugins/Process/FreeBSD/ProcessMonitor.cpp

788

-1 sem_wait -> -1, ::sem_wait

source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp

852

It might be prettier to use consistently ::waitpid across the whole codebase, here and in other places.

[labath]

• fix freebsd typo
• use ::waitpid consistently

[krytarowski]

include/lldb/Host/Host.h

252

Will this build on Windows for their native API? If so, will it be useful there?

[labath]

include/lldb/Host/Host.h

252

It will build, although it will probably be a no-op. Some windows apis (i guess just the posix-compatibility ones) use errno, although I don't know if they can actually fail with EINTR. It's still useful in generic code, as some of those apis are used in shared code.

include/lldb/Host/Host.h

 //===-- Host.h --------------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef liblldb_Host_h_
-#define liblldb_Host_h_
-#if defined(__cplusplus)
-
-#include <stdarg.h>
-
-#include <map>
-#include <string>
+#ifndef LLDB_HOST_HOST_H
+#define LLDB_HOST_HOST_H
 
 #include "lldb/Host/File.h"
 #include "lldb/Host/HostThread.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/StringList.h"
 #include "lldb/lldb-private-forward.h"
 #include "lldb/lldb-private.h"
+#include <cerrno>
+#include <map>
+#include <stdarg.h>
+#include <string>
+#include <type_traits>
 
 namespace lldb_private {
 
 class FileAction;
 class ProcessLaunchInfo;
 
 //----------------------------------------------------------------------
 /// @class Host Host.h "lldb/Host/Host.h"
#endif
   static bool OpenFileInExternalEditor(const FileSpec &file_spec,
                                        uint32_t line_no);
 
   static size_t GetEnvironment(StringList &env);
 };
 
 } // namespace lldb_private
 
-#endif // #if defined(__cplusplus)
-#endif // liblldb_Host_h_
+#endif // LLDB_HOST_HOST_H

include/lldb/Utility/Status.h

protected:
   //------------------------------------------------------------------
   /// Member variables
   //------------------------------------------------------------------
   ValueType m_code;             ///< Status code as an integer value.
   lldb::ErrorType m_type;       ///< The type of the above error code.
   mutable std::string m_string; ///< A string representation of the error code.
 };
 
+template <typename R, typename F, typename... Args>
+inline auto temp_failure_retry(const R &fail_value, const F &f,
+                               const Args &... args) -> decltype(f(args...)) {
+  decltype(f(args...)) result;
+  do
+    result = f(args...);
+  while (result == fail_value && errno == EINTR);
+  return result;
+}
+
 } // namespace lldb_private
 
 namespace llvm {
 template <> struct format_provider<lldb_private::Status> {
   static void format(const lldb_private::Status &error, llvm::raw_ostream &OS,
                      llvm::StringRef Options);
 };
 }
 
 #endif // #ifndef LLDB_UTILITY_STATUS_H

source/Host/common/File.cpp

 #include <termios.h>
 #endif
 
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Process.h" // for llvm::sys::Process::FileDescriptorHasColors()
 
 #include "lldb/Host/Config.h"
+#include "lldb/Host/Host.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Log.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 static const char *GetStreamOpenModeFromOptions(uint32_t options) {

 #ifdef _WIN32
           m_descriptor = ::_dup(GetDescriptor());
 #else
           m_descriptor = dup(GetDescriptor());
 #endif
           m_should_close_fd = true;
         }
 
-        do {
-          m_stream = ::fdopen(m_descriptor, mode);
-        } while (m_stream == NULL && errno == EINTR);
+        m_stream = temp_failure_retry(nullptr, ::fdopen, m_descriptor, mode);
 
         // If we got a stream, then we own the stream and should no
         // longer own the descriptor because fclose() will close it for us
 
         if (m_stream) {
           m_own_stream = true;
           m_should_close_fd = false;
         }
       }
     }
   }
   return m_stream;
 }
 
 void File::SetStream(FILE *fh, bool transfer_ownership) {
   if (IsValid())
     Close();
   m_stream = fh;
   m_own_stream = transfer_ownership;
 }
 
+static int DoOpen(const char *path, int flags, int mode) {
+#ifdef _MSC_VER
+  std::wstring wpath;
+  if (!llvm::ConvertUTF8toWide(path, wpath))
+    return -1;
+  int result;
+  ::_wsopen_s(&result, wpath.c_str(), oflag, _SH_DENYNO, mode);
+  return result;
+#else
+  return ::open(path, flags, mode);
+#endif
+}
+
 Status File::Open(const char *path, uint32_t options, uint32_t permissions) {
   Status error;
   if (IsValid())
     Close();
 
   int oflag = 0;
   const bool read = options & eOpenOptionRead;
   const bool write = options & eOpenOptionWrite;
if (oflag & O_CREAT) {
     if (permissions & lldb::eFilePermissionsWorldRead)
       mode |= S_IROTH;
     if (permissions & lldb::eFilePermissionsWorldWrite)
       mode |= S_IWOTH;
     if (permissions & lldb::eFilePermissionsWorldExecute)
       mode |= S_IXOTH;
   }
 
-  do {
-#ifdef _MSC_VER
-    std::wstring wpath;
-    if (!llvm::ConvertUTF8toWide(path, wpath)) {
-      m_descriptor = -1;
-      error.SetErrorString("Error converting path to UTF-16");
-      return error;
-    }
-    ::_wsopen_s(&m_descriptor, wpath.c_str(), oflag, _SH_DENYNO, mode);
-#else
-    m_descriptor = ::open(path, oflag, mode);
-#endif
-  } while (m_descriptor < 0 && errno == EINTR);
-
+  m_descriptor = temp_failure_retry(-1, DoOpen, path, oflag, mode);
   if (!DescriptorIsValid())
     error.SetErrorToErrno();
   else {
     m_should_close_fd = true;
     m_options = options;
   }
 
   return error;
if (DescriptorIsValid()) {
     error_ptr->SetErrorString("invalid file handle");
   }
   return result;
 }
 
 Status File::Flush() {
   Status error;
   if (StreamIsValid()) {
-    int err = 0;
-    do {
-      err = ::fflush(m_stream);
-    } while (err == EOF && errno == EINTR);
-
-    if (err == EOF)
+    if (temp_failure_retry(EOF, ::fflush, m_stream) == EOF)
       error.SetErrorToErrno();
   } else if (!DescriptorIsValid()) {
     error.SetErrorString("invalid file handle");
   }
   return error;
 }
 
 Status File::Sync() {
   Status error;
   if (DescriptorIsValid()) {
 #ifdef _WIN32
     int err = FlushFileBuffers((HANDLE)_get_osfhandle(m_descriptor));
     if (err == 0)
       error.SetErrorToGenericError();
 #else
-    int err = 0;
-    do {
-      err = ::fsync(m_descriptor);
-    } while (err == -1 && errno == EINTR);
-
-    if (err == -1)
+    if (temp_failure_retry(-1, ::fsync, m_descriptor) == -1)
       error.SetErrorToErrno();
 #endif
   } else {
     error.SetErrorString("invalid file handle");
   }
   return error;
 }
 
while (bytes_left > 0) {
         break;
     }
     return error;
   }
 #endif
 
   ssize_t bytes_read = -1;
   if (DescriptorIsValid()) {
-    do {
-      bytes_read = ::read(m_descriptor, buf, num_bytes);
-    } while (bytes_read < 0 && errno == EINTR);
-
+    bytes_read = temp_failure_retry(-1, ::read, m_descriptor, buf, num_bytes);
     if (bytes_read == -1) {
       error.SetErrorToErrno();
       num_bytes = 0;
     } else
       num_bytes = bytes_read;
   } else if (StreamIsValid()) {
     bytes_read = ::fread(buf, 1, num_bytes, m_stream);
 
while (bytes_left > 0) {
         break;
     }
     return error;
   }
 #endif
 
   ssize_t bytes_written = -1;
   if (DescriptorIsValid()) {
-    do {
-      bytes_written = ::write(m_descriptor, buf, num_bytes);
-    } while (bytes_written < 0 && errno == EINTR);
-
+    bytes_written =
+        temp_failure_retry(-1, ::write, m_descriptor, buf, num_bytes);
     if (bytes_written == -1) {
       error.SetErrorToErrno();
       num_bytes = 0;
     } else
       num_bytes = bytes_written;
   } else if (StreamIsValid()) {
     bytes_written = ::fwrite(buf, 1, num_bytes, m_stream);
 
if (num_bytes > MAX_READ_SIZE) {
     }
     return error;
   }
 #endif
 
 #ifndef _WIN32
   int fd = GetDescriptor();
   if (fd != kInvalidDescriptor) {
-    ssize_t bytes_read = -1;
-    do {
-      bytes_read = ::pread(fd, buf, num_bytes, offset);
-    } while (bytes_read < 0 && errno == EINTR);
-
+    ssize_t bytes_read =
+        temp_failure_retry(-1, ::pread, fd, buf, num_bytes, offset);
     if (bytes_read < 0) {
       num_bytes = 0;
       error.SetErrorToErrno();
     } else {
       offset += bytes_read;
       num_bytes = bytes_read;
     }
   } else {
if (num_bytes > MAX_WRITE_SIZE) {
     }
     return error;
   }
 #endif
 
   int fd = GetDescriptor();
   if (fd != kInvalidDescriptor) {
 #ifndef _WIN32
-    ssize_t bytes_written = -1;
-    do {
-      bytes_written = ::pwrite(m_descriptor, buf, num_bytes, offset);
-    } while (bytes_written < 0 && errno == EINTR);
-
+    ssize_t bytes_written =
+        temp_failure_retry(-1, ::pwrite, m_descriptor, buf, num_bytes, offset);
     if (bytes_written < 0) {
       num_bytes = 0;
       error.SetErrorToErrno();
     } else {
       offset += bytes_written;
       num_bytes = bytes_written;
     }
 #else

source/Host/macosx/Host.mm

::dispatch_source_set_cancel_handler(source, ^{
       dispatch_release(source);
     });
     ::dispatch_source_set_event_handler(source, ^{
 
       int status = 0;
       int wait_pid = 0;
       bool cancel = false;
       bool exited = false;
-      do {
-        wait_pid = ::waitpid(pid, &status, 0);
-      } while (wait_pid < 0 && errno == EINTR);
-
+      wait_pid = temp_failure_retry(-1, ::waitpid, pid, &status, 0);
       if (wait_pid >= 0) {
         int signal = 0;
         int exit_status = 0;
         const char *status_cstr = NULL;
         if (WIFSTOPPED(status)) {
           signal = WSTOPSIG(status);
           status_cstr = "STOPPED";
         } else if (WIFEXITED(status)) {

source/Host/posix/ConnectionFileDescriptorPosix.cpp

else if ((addr = GetURLAddress(path, FD_SCHEME))) {
         error_ptr->SetErrorStringWithFormat("invalid file descriptor: \"%s\"",
                                             path.str().c_str());
       m_read_sp.reset();
       m_write_sp.reset();
       return eConnectionStatusError;
     } else if ((addr = GetURLAddress(path, FILE_SCHEME))) {
       std::string addr_str = addr->str();
       // file:///PATH
-      int fd = -1;
-      do {
-        fd = ::open(addr_str.c_str(), O_RDWR);
-      } while (fd == -1 && errno == EINTR);
-
+      int fd = temp_failure_retry(-1, ::open, addr_str.c_str(), O_RDWR);
       if (fd == -1) {
         if (error_ptr)
           error_ptr->SetErrorToErrno();
         return eConnectionStatusError;
       }
 
       if (::isatty(fd)) {
         // Set up serial terminal emulation
while (handle == m_read_sp->GetWaitableHandle()) {
         }
       } else {
         if (select_helper.FDIsSetRead(handle))
           return eConnectionStatusSuccess;
 
         if (select_helper.FDIsSetRead(pipe_fd)) {
           // There is an interrupt or exit command in the command pipe
           // Read the data from that pipe:
-          char buffer[1];
-
-          ssize_t bytes_read;
-
-          do {
-            bytes_read = ::read(pipe_fd, buffer, sizeof(buffer));
-          } while (bytes_read < 0 && errno == EINTR);
+          char c;
 
-          switch (buffer[0]) {
+          ssize_t bytes_read = temp_failure_retry(-1, ::read, pipe_fd, &c, 1);
+          assert(bytes_read == 1);
+          (void)bytes_read;
+          switch (c) {
           case 'q':
             if (log)
               log->Printf("%p ConnectionFileDescriptor::BytesAvailable() "
                           "got data: %c from the command channel.",
-                          static_cast<void *>(this), buffer[0]);
+                          static_cast<void *>(this), c);
             return eConnectionStatusEndOfFile;
           case 'i':
             // Interrupt the current read
             return eConnectionStatusInterrupted;
           }
         }
       }
     }

source/Plugins/Process/FreeBSD/ProcessMonitor.cpp

ProcessMonitor::ProcessMonitor(
 
   sem_init(&m_operation_pending, 0, 0);
   sem_init(&m_operation_done, 0, 0);
 
   StartLaunchOpThread(args.get(), error);
   if (!error.Success())
     return;
 
-WAIT_AGAIN:
-  // Wait for the operation thread to initialize.
-  if (sem_wait(&args->m_semaphore)) {
-    if (errno == EINTR)
-      goto WAIT_AGAIN;
-    else {
-      error.SetErrorToErrno();
-      return;
-    }
-  }
+  if (temp_failure_retry(-1, sem_wait, &args->m_semaphore) == -1) {
 
   // Check that the launch was a success.
   if (!args->m_error.Success()) {
     StopOpThread();
     error = args->m_error;
     return;
   }
 
ProcessMonitor::ProcessMonitor(ProcessFreeBSD *process, lldb::pid_t pid,
   sem_init(&m_operation_done, 0, 0);
 
   std::unique_ptr<AttachArgs> args(new AttachArgs(this, pid));
 
   StartAttachOpThread(args.get(), error);
   if (!error.Success())
     return;
 
-WAIT_AGAIN:
-  // Wait for the operation thread to initialize.
-  if (sem_wait(&args->m_semaphore)) {
-    if (errno == EINTR)
-      goto WAIT_AGAIN;
-    else {
-      error.SetErrorToErrno();
-      return;
-    }
-  }
+  if (temp_failure_retry(-1 sem_wait, &args->m_semaphore) == -1) {
 
   // Check that the attach was a success.
   if (!args->m_error.Success()) {
     StopOpThread();
     error = args->m_error;
     return;
   }
 

source/Plugins/Process/Linux/NativeProcessLinux.cpp

if (new_thread_sp) {
     // We are already tracking the thread - we got the event on the new thread
     // (see
     // MonitorSignal) before this one. We are done.
     return;
   }
 
   // The thread is not tracked yet, let's wait for it to appear.
   int status = -1;
-  ::pid_t wait_pid;
-  do {
-    LLDB_LOG(log,
-             "received thread creation event for tid {0}. tid not tracked "
-             "yet, waiting for thread to appear...",
-             tid);
-    wait_pid = waitpid(tid, &status, __WALL);
-  } while (wait_pid == -1 && errno == EINTR);
+  ::pid_t wait_pid = temp_failure_retry(-1, waitpid, tid, &status, __WALL);
   // Since we are waiting on a specific tid, this must be the creation event.
   // But let's do some checks just in case.
   if (wait_pid != tid) {
     LLDB_LOG(log,
              "waiting for tid {0} failed. Assuming the thread has "
              "disappeared in the meantime",
              tid);
     // The only way I know of this could happen is if the whole process was
void NativeProcessLinux::ThreadWasCreated(NativeThreadLinux &thread) {
   }
 }
 
 void NativeProcessLinux::SigchldHandler() {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
   // Process all pending waitpid notifications.
   while (true) {
     int status = -1;
-    ::pid_t wait_pid = waitpid(-1, &status, __WALL | __WNOTHREAD | WNOHANG);
+    ::pid_t wait_pid = temp_failure_retry(-1, waitpid, -1, &status,
+                                          __WALL | __WNOTHREAD | WNOHANG);
 
     if (wait_pid == 0)
       break; // We are done.
 
     if (wait_pid == -1) {
-      if (errno == EINTR)
-        continue;
-
       Status error(errno, eErrorTypePOSIX);
       LLDB_LOG(log, "waitpid (-1, &status, _) failed: {0}", error);
       break;
     }
 
     bool exited = false;
     int signal = 0;
     int exit_status = 0;

source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp

void NativeProcessNetBSD::AttachToInferior(MainLoop &mainloop, lldb::pid_t pid,
 
   Attach(pid, error);
 }
 
 void NativeProcessNetBSD::SigchldHandler() {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
   // Process all pending waitpid notifications.
   int status;
-  ::pid_t wait_pid = waitpid(GetID(), &status, WALLSIG | WNOHANG);
+  ::pid_t wait_pid =
+      temp_failure_retry(-1, waitpid, GetID(), &status, WALLSIG | WNOHANG);
 
   if (wait_pid == 0)
     return; // We are done.
 
   if (wait_pid == -1) {
-    if (errno == EINTR)
-      return;
-
     Status error(errno, eErrorTypePOSIX);
     LLDB_LOG(log, "waitpid ({0}, &status, _) failed: {1}", GetID(), error);
   }
 
   bool exited = false;
   int signal = 0;
   int exit_status = 0;
   const char *status_cstr = nullptr;

unittests/Host/CMakeLists.txt

 set (FILES
   FileSpecTest.cpp
   FileSystemTest.cpp
+  HostTest.cpp
   MainLoopTest.cpp
   SocketAddressTest.cpp
   SocketTest.cpp
   SymbolsTest.cpp
 )
 
 if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
   list(APPEND FILES

unittests/Host/HostTest.cpp

+//===-- HostTest.cpp --------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Host/Host.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+TEST(Host, temp_failure_retry) {
+  EXPECT_EQ(1, temp_failure_retry(-1, [] { return 1; }));
+
+  EXPECT_EQ(-1, temp_failure_retry(-1, [] {
+    errno = EAGAIN;
+    return -1;
+  }));
+  EXPECT_EQ(EAGAIN, errno);
+
+  unsigned calls = 0;
+  EXPECT_EQ(1, temp_failure_retry(-1, [&calls] {
+              errno = EINTR;
+              ++calls;
+              return calls == 1 ? -1 : 1;
+            }));
+  EXPECT_EQ(2u, calls);
+
+  EXPECT_EQ(1, temp_failure_retry(-1, [](int x) { return x; }, 1));
+}