kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject kudu git commit: KUDU-2427: retry more system calls on EINTR
Date Wed, 23 May 2018 23:17:05 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 0e19cf772 -> b7cf3b2e4


KUDU-2427: retry more system calls on EINTR

In order to collect its own stack traces, Kudu periodically sends itself a
SIGUSR2. The diagnostics log initiates stack collection every 60s, as do
some service queue overflow events. In theory, the collection shouldn't
affect any ongoing syscalls because the SIGUSR2 signal handler is installed
with SA_RESTART; in practice, not all syscalls are restartable, and
precisely categorizing those that are and those that aren't is difficult. As
such, it's really important that we retry every interruptible syscall rather
than surfacing the EINTR up the call stack as a failure.

For whatever reason this happens more frequently on Ubuntu 18.04, though
maybe it's because I've placed my test directory on tmpfs. For example, I
can easily repro a crash due to non-existent retry with the following
command line:

  bin/tablet_server-test --gtest_repeat=1000 --gtest_throw_on_failure \
    --diagnostics_log_stack_traces_interval_ms=100 \
    --unlock_experimental_flags --gtest_filter=*KUDU_177

This patch also fixes KUDU-2151.

Change-Id: I6cce03c4e1b2be32c1910382737526082fc99966
Reviewed-on: http://gerrit.cloudera.org:8080/10435
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <aserbin@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/b7cf3b2e
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/b7cf3b2e
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/b7cf3b2e

Branch: refs/heads/master
Commit: b7cf3b2e44f8ef560c67e901efc7c59a091ddefd
Parents: 0e19cf7
Author: Adar Dembo <adar@cloudera.com>
Authored: Mon May 14 17:19:03 2018 -0700
Committer: Adar Dembo <adar@cloudera.com>
Committed: Wed May 23 23:16:48 2018 +0000

----------------------------------------------------------------------
 src/kudu/consensus/log_index.cc      |   7 +-
 src/kudu/gutil/macros.h              |  22 ++++++
 src/kudu/gutil/sysinfo.cc            |  30 +++++---
 src/kudu/tools/tool_action_common.cc |  19 ++---
 src/kudu/tools/tool_action_test.cc   |  10 ++-
 src/kudu/util/env-test.cc            |  18 +++--
 src/kudu/util/env_posix.cc           | 111 ++++++++++++++++++++----------
 src/kudu/util/net/socket.cc          |  16 +++--
 src/kudu/util/os-util.cc             |  14 ++--
 src/kudu/util/os-util.h              |   9 ---
 src/kudu/util/pstack_watcher-test.cc |  26 ++++---
 src/kudu/util/pstack_watcher.cc      |   5 +-
 src/kudu/util/semaphore.cc           |  16 ++---
 src/kudu/util/subprocess-test.cc     |   9 ++-
 src/kudu/util/subprocess.cc          |  79 ++++++++++++---------
 15 files changed, 253 insertions(+), 138 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/b7cf3b2e/src/kudu/consensus/log_index.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/log_index.cc b/src/kudu/consensus/log_index.cc
index 319e9eb..587fb7a 100644
--- a/src/kudu/consensus/log_index.cc
+++ b/src/kudu/consensus/log_index.cc
@@ -50,7 +50,6 @@
 #include "kudu/gutil/stringprintf.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/errno.h"
-#include "kudu/util/os-util.h"
 
 using std::string;
 using std::vector;
@@ -113,7 +112,11 @@ LogIndex::IndexChunk::~IndexChunk() {
   }
 
   if (fd_ >= 0) {
-    close(fd_);
+    int ret;
+    RETRY_ON_EINTR(ret, close(fd_));
+    if (PREDICT_FALSE(ret != 0)) {
+      PLOG(WARNING) << "Failed to close fd " << fd_;
+    }
   }
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/b7cf3b2e/src/kudu/gutil/macros.h
----------------------------------------------------------------------
diff --git a/src/kudu/gutil/macros.h b/src/kudu/gutil/macros.h
index f271bd4..696323b 100644
--- a/src/kudu/gutil/macros.h
+++ b/src/kudu/gutil/macros.h
@@ -259,4 +259,26 @@ enum LinkerInitialized { LINKER_INITIALIZED };
 #define FALLTHROUGH_INTENDED do { } while (0)
 #endif
 
+// Retry on EINTR for functions like read() that return -1 on error.
+#define RETRY_ON_EINTR(err, expr) do { \
+  static_assert(std::is_signed<decltype(err)>::value, \
+                #err " must be a signed integer"); \
+  (err) = (expr); \
+} while ((err) == -1 && errno == EINTR)
+
+// Same as above but for stream API calls like fread() and fwrite().
+#define STREAM_RETRY_ON_EINTR(nread, stream, expr) do { \
+  static_assert(std::is_unsigned<decltype(nread)>::value == true, \
+                #nread " must be an unsigned integer"); \
+  (nread) = (expr); \
+} while ((nread) == 0 && ferror(stream) == EINTR)
+
+// Same as above but for functions that return pointer types (like
+// fopen() and freopen()).
+#define POINTER_RETRY_ON_EINTR(ptr, expr) do { \
+  static_assert(std::is_pointer<decltype(ptr)>::value == true, \
+                #ptr " must be a pointer"); \
+  (ptr) = (expr); \
+} while ((ptr) == nullptr && errno == EINTR)
+
 #endif  // BASE_MACROS_H_

http://git-wip-us.apache.org/repos/asf/kudu/blob/b7cf3b2e/src/kudu/gutil/sysinfo.cc
----------------------------------------------------------------------
diff --git a/src/kudu/gutil/sysinfo.cc b/src/kudu/gutil/sysinfo.cc
index 407a8c8..289a4b1 100644
--- a/src/kudu/gutil/sysinfo.cc
+++ b/src/kudu/gutil/sysinfo.cc
@@ -37,8 +37,8 @@
 #include <unistd.h>   // for read()
 
 #if defined __MACH__          // Mac OS X, almost certainly
-#include <sys/types.h>
 #include <sys/sysctl.h>       // how we figure out numcpu's on OS X
+#include <sys/types.h>
 #elif defined __FreeBSD__
 #include <sys/sysctl.h>
 #elif defined __sun__         // Solaris
@@ -49,6 +49,8 @@
 #include <tlhelp32.h>         // for Module32First()
 #endif
 
+#include "kudu/gutil/sysinfo.h"
+
 #include <cerrno>    // for errno
 #include <cstdio>    // for snprintf(), sscanf()
 #include <cstdlib>   // for getenv()
@@ -61,7 +63,7 @@
 #include "kudu/gutil/dynamic_annotations.h"   // for RunningOnValgrind
 #include "kudu/gutil/integral_types.h"
 #include "kudu/gutil/macros.h"
-#include "kudu/gutil/sysinfo.h"
+#include "kudu/gutil/port.h"
 #include "kudu/gutil/walltime.h"
 
 // This isn't in the 'base' namespace in tcmallc. But, tcmalloc
@@ -119,18 +121,25 @@ static int64 EstimateCyclesPerSecond(const int estimate_time_ms) {
 // issue a FATAL error.
 static bool SlurpSmallTextFile(const char* file, char* buf, int buflen) {
   bool ret = false;
-  int fd = open(file, O_RDONLY);
+  int fd;
+  RETRY_ON_EINTR(fd, open(file, O_RDONLY));
   if (fd == -1) return ret;
 
   memset(buf, '\0', buflen);
-  int n = read(fd, buf, buflen - 1);
+  int n;
+  RETRY_ON_EINTR(n, read(fd, buf, buflen - 1));
   CHECK_NE(n, buflen - 1) << "buffer of len " << buflen << " not large
enough to store "
                           << "contents of " << file;
   if (n > 0) {
     ret = true;
   }
 
-  close(fd);
+  int close_ret;
+  RETRY_ON_EINTR(close_ret, close(fd));
+  if (PREDICT_FALSE(close_ret != 0)) {
+    PLOG(WARNING) << "Failed to close fd " << fd;
+  }
+
   return ret;
 }
 
@@ -225,7 +234,8 @@ static void InitializeSystemInfo() {
 
   // Read /proc/cpuinfo for other values, and if there is no cpuinfo_max_freq.
   const char* pname = "/proc/cpuinfo";
-  int fd = open(pname, O_RDONLY);
+  int fd;
+  RETRY_ON_EINTR(fd, open(pname, O_RDONLY));
   if (fd == -1) {
     PLOG(FATAL) << "Unable to read CPU info from /proc. procfs must be mounted.";
   }
@@ -248,7 +258,7 @@ static void InitializeSystemInfo() {
       const int linelen = strlen(line);
       const int bytes_to_read = sizeof(line)-1 - linelen;
       CHECK(bytes_to_read > 0);  // because the memmove recovered >=1 bytes
-      chars_read = read(fd, line + linelen, bytes_to_read);
+      RETRY_ON_EINTR(chars_read, read(fd, line + linelen, bytes_to_read));
       line[linelen + chars_read] = '\0';
       newline = strchr(line, '\n');
     }
@@ -293,7 +303,11 @@ static void InitializeSystemInfo() {
       num_cpus++;  // count up every time we see an "processor :" entry
     }
   } while (chars_read > 0);
-  close(fd);
+  int ret;
+  RETRY_ON_EINTR(ret, close(fd));
+  if (PREDICT_FALSE(ret != 0)) {
+    PLOG(WARNING) << "Failed to close fd " << fd;
+  }
 
   if (!saw_mhz) {
     if (saw_bogo) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/b7cf3b2e/src/kudu/tools/tool_action_common.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/tool_action_common.cc b/src/kudu/tools/tool_action_common.cc
index 9edc548..d5604d9 100644
--- a/src/kudu/tools/tool_action_common.cc
+++ b/src/kudu/tools/tool_action_common.cc
@@ -663,8 +663,9 @@ ControlShellProtocol::ControlShellProtocol(SerializationMode serialization_mode,
 
 ControlShellProtocol::~ControlShellProtocol() {
   if (close_mode_ == CloseMode::CLOSE_ON_DESTROY) {
-    close(read_fd_);
-    close(write_fd_);
+    int ret;
+    RETRY_ON_EINTR(ret, close(read_fd_));
+    RETRY_ON_EINTR(ret, close(write_fd_));
   }
 }
 
@@ -772,12 +773,9 @@ Status ControlShellProtocol::DoRead(faststring* buf) {
   uint8_t* pos = buf->data();
   size_t rem = buf->length();
   while (rem > 0) {
-    ssize_t r = read(read_fd_, pos, rem);
+    ssize_t r;
+    RETRY_ON_EINTR(r, read(read_fd_, pos, rem));
     if (r == -1) {
-      if (errno == EINTR) {
-        // Interrupted by a signal, retry.
-        continue;
-      }
       return Status::IOError("Error reading from pipe", "", errno);
     }
     if (r == 0) {
@@ -794,12 +792,9 @@ Status ControlShellProtocol::DoWrite(const faststring& buf) {
   const uint8_t* pos = buf.data();
   size_t rem = buf.length();
   while (rem > 0) {
-    ssize_t r = write(write_fd_, pos, rem);
+    ssize_t r;
+    RETRY_ON_EINTR(r, write(write_fd_, pos, rem));
     if (r == -1) {
-      if (errno == EINTR) {
-        // Interrupted by a signal, retry.
-        continue;
-      }
       if (errno == EPIPE) {
         return Status::EndOfFile("Other end of pipe was closed");
       }

http://git-wip-us.apache.org/repos/asf/kudu/blob/b7cf3b2e/src/kudu/tools/tool_action_test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/tool_action_test.cc b/src/kudu/tools/tool_action_test.cc
index 356350b..af65dad 100644
--- a/src/kudu/tools/tool_action_test.cc
+++ b/src/kudu/tools/tool_action_test.cc
@@ -34,6 +34,7 @@
 
 #include "kudu/common/common.pb.h"
 #include "kudu/common/wire_protocol.h"
+#include "kudu/gutil/macros.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"
 #include "kudu/security/test/mini_kdc.h"
@@ -298,9 +299,12 @@ Status RunControlShell(const RunnerContext& /*context*/) {
   // Because we use stdin and stdout to communicate with the shell's parent,
   // it's critical that none of our subprocesses write to stdout. To that end,
   // the protocol will use stdout via another fd, and we'll redirect fd 1 to stderr.
-  int new_stdout = dup(STDOUT_FILENO);
-  PCHECK(new_stdout != -1);
-  PCHECK(dup2(STDERR_FILENO, STDOUT_FILENO) == STDOUT_FILENO);
+  int new_stdout;
+  RETRY_ON_EINTR(new_stdout, dup(STDOUT_FILENO));
+  CHECK_ERR(new_stdout);
+  int ret;
+  RETRY_ON_EINTR(ret, dup2(STDERR_FILENO, STDOUT_FILENO));
+  PCHECK(ret == STDOUT_FILENO);
   ControlShellProtocol::SerializationMode serde_mode;
   if (boost::iequals(FLAGS_serialization, "json")) {
     serde_mode = ControlShellProtocol::SerializationMode::JSON;

http://git-wip-us.apache.org/repos/asf/kudu/blob/b7cf3b2e/src/kudu/util/env-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/env-test.cc b/src/kudu/util/env-test.cc
index 83bf67d..1c7f899 100644
--- a/src/kudu/util/env-test.cc
+++ b/src/kudu/util/env-test.cc
@@ -27,6 +27,8 @@
 #define FALLOC_FL_PUNCH_HOLE  0x02 /* de-allocates range */
 #endif
 
+#include "kudu/util/env.h"
+
 #include <fcntl.h>
 #include <sys/stat.h>
 #include <unistd.h>
@@ -48,13 +50,13 @@
 #include <gtest/gtest.h>
 
 #include "kudu/gutil/bind.h"
+#include "kudu/gutil/macros.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/port.h"
 #include "kudu/gutil/strings/human_readable.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/gutil/strings/util.h"
 #include "kudu/util/array_view.h" // IWYU pragma: keep
-#include "kudu/util/env.h"
 #include "kudu/util/env_util.h"
 #include "kudu/util/faststring.h"
 #include "kudu/util/monotime.h"
@@ -105,16 +107,18 @@ class TestEnv : public KuduTest {
     if (checked) return;
 
 #if defined(__linux__)
-    int fd = creat(GetTestPath("check-fallocate").c_str(), S_IWUSR);
-    PCHECK(fd >= 0);
-    int err = fallocate(fd, 0, 0, 4096);
+    int fd;
+    RETRY_ON_EINTR(fd, creat(GetTestPath("check-fallocate").c_str(), S_IWUSR));
+    CHECK_ERR(fd);
+    int err;
+    RETRY_ON_EINTR(err, fallocate(fd, 0, 0, 4096));
     if (err != 0) {
       PCHECK(errno == ENOTSUP);
     } else {
       fallocate_supported_ = true;
 
-      err = fallocate(fd, FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
-                      1024, 1024);
+      RETRY_ON_EINTR(err, fallocate(fd, FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE,
+                                    1024, 1024));
       if (err != 0) {
         PCHECK(errno == ENOTSUP);
       } else {
@@ -122,7 +126,7 @@ class TestEnv : public KuduTest {
       }
     }
 
-    close(fd);
+    RETRY_ON_EINTR(err, close(fd));
 #endif
 
     checked = true;

http://git-wip-us.apache.org/repos/asf/kudu/blob/b7cf3b2e/src/kudu/util/env_posix.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/env_posix.cc b/src/kudu/util/env_posix.cc
index c51a192..fe47bfd 100644
--- a/src/kudu/util/env_posix.cc
+++ b/src/kudu/util/env_posix.cc
@@ -57,7 +57,6 @@
 #include "kudu/util/logging.h"
 #include "kudu/util/malloc.h"
 #include "kudu/util/monotime.h"
-#include "kudu/util/os-util.h"
 #include "kudu/util/path_util.h"
 #include "kudu/util/scoped_cleanup.h"
 #include "kudu/util/slice.h"
@@ -120,13 +119,6 @@ typedef struct xfs_flock64 {
 #define fread_unlocked fread
 #endif
 
-// Same as the above, but for stream API calls like fread() and fwrite().
-#define STREAM_RETRY_ON_EINTR(nread, stream, expr) do { \
-  static_assert(std::is_unsigned<decltype(nread)>::value == true, \
-                #nread " must be an unsigned integer"); \
-  (nread) = (expr); \
-} while ((nread) == 0 && ferror(stream) == EINTR)
-
 // With some probability, if 'filename_expr' matches the glob pattern specified
 // by the 'env_inject_eio_globs' flag, calls RETURN_NOT_OK on 'error_expr'.
 #define MAYBE_RETURN_EIO(filename_expr, error_expr) do { \
@@ -296,7 +288,8 @@ class ScopedFdCloser {
 
   ~ScopedFdCloser() {
     ThreadRestrictions::AssertIOAllowed();
-    int err = ::close(fd_);
+    int err;
+    RETRY_ON_EINTR(err, ::close(fd_));
     if (PREDICT_FALSE(err != 0)) {
       PLOG(WARNING) << "Failed to close fd " << fd_;
     }
@@ -363,7 +356,8 @@ Status DoOpen(const string& filename, Env::CreateMode mode, int* fd)
{
     default:
       return Status::NotSupported(Substitute("Unknown create mode $0", mode));
   }
-  const int f = open(filename.c_str(), flags, 0666);
+  int f;
+  RETRY_ON_EINTR(f, open(filename.c_str(), flags, 0666));
   if (f < 0) {
     return IOError(filename, errno);
   }
@@ -556,7 +550,13 @@ class PosixSequentialFile: public SequentialFile {
  public:
   PosixSequentialFile(std::string fname, FILE* f)
       : filename_(std::move(fname)), file_(f) {}
-  virtual ~PosixSequentialFile() { fclose(file_); }
+  virtual ~PosixSequentialFile() {
+    int err;
+    RETRY_ON_EINTR(err, fclose(file_));
+    if (PREDICT_FALSE(err != 0)) {
+      PLOG(WARNING) << "Failed to close " << filename_;
+    }
+  }
 
   virtual Status Read(Slice* result) OVERRIDE {
     MAYBE_RETURN_EIO(filename_, IOError(Env::kInjectedFailureStatusMsg, EIO));
@@ -599,7 +599,13 @@ class PosixRandomAccessFile: public RandomAccessFile {
  public:
   PosixRandomAccessFile(std::string fname, int fd)
       : filename_(std::move(fname)), fd_(fd) {}
-  virtual ~PosixRandomAccessFile() { close(fd_); }
+  virtual ~PosixRandomAccessFile() {
+    int err;
+    RETRY_ON_EINTR(err, close(fd_));
+    if (PREDICT_FALSE(err != 0)) {
+      PLOG(WARNING) << "Failed to close " << filename_;
+    }
+  }
 
   virtual Status Read(uint64_t offset, Slice result) const OVERRIDE {
     return DoReadV(fd_, filename_, offset, ArrayView<Slice>(&result, 1));
@@ -672,7 +678,9 @@ class PosixWritableFile : public WritableFile {
     TRACE_EVENT1("io", "PosixWritableFile::PreAllocate", "path", filename_);
     ThreadRestrictions::AssertIOAllowed();
     uint64_t offset = std::max(filesize_, pre_allocated_size_);
-    if (fallocate(fd_, 0, offset, size) < 0) {
+    int ret;
+    RETRY_ON_EINTR(ret, fallocate(fd_, 0, offset, size));
+    if (ret != 0) {
       if (errno == EOPNOTSUPP) {
         KLOG_FIRST_N(WARNING, 1) << "The filesystem does not support fallocate().";
       } else if (errno == ENOSYS) {
@@ -712,7 +720,9 @@ class PosixWritableFile : public WritableFile {
       }
     }
 
-    if (close(fd_) < 0) {
+    int ret;
+    RETRY_ON_EINTR(ret, close(fd_));
+    if (ret < 0) {
       if (s.ok()) {
         s = IOError(filename_, errno);
       }
@@ -797,8 +807,7 @@ class PosixRWFile : public RWFile {
   }
 
   virtual Status WriteV(uint64_t offset, ArrayView<const Slice> data) OVERRIDE {
-    Status s = DoWriteV(fd_, filename_, offset, data);
-    return s;
+    return DoWriteV(fd_, filename_, offset, data);
   }
 
   virtual Status PreAllocate(uint64_t offset,
@@ -812,7 +821,9 @@ class PosixRWFile : public RWFile {
     if (mode == DONT_CHANGE_FILE_SIZE) {
       falloc_mode = FALLOC_FL_KEEP_SIZE;
     }
-    if (fallocate(fd_, falloc_mode, offset, length) < 0) {
+    int ret;
+    RETRY_ON_EINTR(ret, fallocate(fd_, falloc_mode, offset, length));
+    if (ret != 0) {
       if (errno == EOPNOTSUPP) {
         KLOG_FIRST_N(WARNING, 1) << "The filesystem does not support fallocate().";
       } else if (errno == ENOSYS) {
@@ -863,9 +874,13 @@ class PosixRWFile : public RWFile {
       if (ioctl(fd_, XFS_IOC_UNRESVSP64, &cmd) < 0) {
         return IOError(filename_, errno);
       }
-    } else if (fallocate(fd_, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
-                         offset, length) < 0) {
-      return IOError(filename_, errno);
+    } else {
+      int ret;
+      RETRY_ON_EINTR(ret, fallocate(
+          fd_, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, offset, length));
+      if (ret != 0) {
+        return IOError(filename_, errno);
+      }
     }
     return Status::OK();
 #else
@@ -918,7 +933,9 @@ class PosixRWFile : public RWFile {
       }
     }
 
-    if (close(fd_) < 0) {
+    int ret;
+    RETRY_ON_EINTR(ret, close(fd_));
+    if (ret < 0) {
       if (s.ok()) {
         s = IOError(filename_, errno);
       }
@@ -1034,7 +1051,9 @@ int LockOrUnlock(int fd, bool lock) {
   f.l_whence = SEEK_SET;
   f.l_start = 0;
   f.l_len = 0;        // Lock/unlock entire file
-  return fcntl(fd, F_SETLK, &f);
+  int ret;
+  RETRY_ON_EINTR(ret, fcntl(fd, F_SETLK, &f));
+  return ret;
 }
 
 class PosixFileLock : public FileLock {
@@ -1055,13 +1074,13 @@ class PosixEnv : public Env {
     TRACE_EVENT1("io", "PosixEnv::NewSequentialFile", "path", fname);
     MAYBE_RETURN_EIO(fname, IOError(Env::kInjectedFailureStatusMsg, EIO));
     ThreadRestrictions::AssertIOAllowed();
-    FILE* f = fopen(fname.c_str(), "r");
+    FILE* f;
+    POINTER_RETRY_ON_EINTR(f, fopen(fname.c_str(), "r"));
     if (f == nullptr) {
       return IOError(fname, errno);
-    } else {
-      result->reset(new PosixSequentialFile(fname, f));
-      return Status::OK();
     }
+    result->reset(new PosixSequentialFile(fname, f));
+    return Status::OK();
   }
 
   virtual Status NewRandomAccessFile(const std::string& fname,
@@ -1075,7 +1094,8 @@ class PosixEnv : public Env {
     TRACE_EVENT1("io", "PosixEnv::NewRandomAccessFile", "path", fname);
     MAYBE_RETURN_EIO(fname, IOError(Env::kInjectedFailureStatusMsg, EIO));
     ThreadRestrictions::AssertIOAllowed();
-    int fd = open(fname.c_str(), O_RDONLY);
+    int fd;
+    RETRY_ON_EINTR(fd, open(fname.c_str(), O_RDONLY));
     if (fd < 0) {
       return IOError(fname, errno);
     }
@@ -1223,7 +1243,8 @@ class PosixEnv : public Env {
     ThreadRestrictions::AssertIOAllowed();
     if (FLAGS_never_fsync) return Status::OK();
     int dir_fd;
-    if ((dir_fd = open(dirname.c_str(), O_DIRECTORY|O_RDONLY)) == -1) {
+    RETRY_ON_EINTR(dir_fd, open(dirname.c_str(), O_DIRECTORY|O_RDONLY));
+    if (dir_fd < 0) {
       return IOError(dirname, errno);
     }
     ScopedFdCloser fd_closer(dir_fd);
@@ -1355,12 +1376,17 @@ class PosixEnv : public Env {
     ThreadRestrictions::AssertIOAllowed();
     *lock = nullptr;
     Status result;
-    int fd = open(fname.c_str(), O_RDWR | O_CREAT, 0666);
+    int fd;
+    RETRY_ON_EINTR(fd, open(fname.c_str(), O_RDWR | O_CREAT, 0666));
     if (fd < 0) {
       result = IOError(fname, errno);
     } else if (LockOrUnlock(fd, true) == -1) {
       result = IOError("lock " + fname, errno);
-      close(fd);
+      int err;
+      RETRY_ON_EINTR(err, close(fd));
+      if (PREDICT_FALSE(err != 0)) {
+        PLOG(WARNING) << "Failed to close fd " << fd;
+      }
     } else {
       auto my_lock = new PosixFileLock;
       my_lock->fd_ = fd;
@@ -1372,13 +1398,16 @@ class PosixEnv : public Env {
   virtual Status UnlockFile(FileLock* lock) OVERRIDE {
     TRACE_EVENT0("io", "PosixEnv::UnlockFile");
     ThreadRestrictions::AssertIOAllowed();
-    PosixFileLock* my_lock = reinterpret_cast<PosixFileLock*>(lock);
+    unique_ptr<PosixFileLock> my_lock(reinterpret_cast<PosixFileLock*>(lock));
     Status result;
     if (LockOrUnlock(my_lock->fd_, false) == -1) {
       result = IOError("unlock", errno);
     }
-    close(my_lock->fd_);
-    delete my_lock;
+    int err;
+    RETRY_ON_EINTR(err, close(my_lock->fd_));
+    if (PREDICT_FALSE(err != 0)) {
+      PLOG(WARNING) << "Failed to close fd " << my_lock->fd_;
+    }
     return result;
   }
 
@@ -1481,11 +1510,13 @@ class PosixEnv : public Env {
     char *(paths[]) = { name_dup.get(), nullptr };
 
     // FTS_NOCHDIR is important here to make this thread-safe.
-    unique_ptr<FTS, FtsCloser> tree(
-        fts_open(paths, FTS_PHYSICAL | FTS_XDEV | FTS_NOCHDIR, nullptr));
-    if (!tree.get()) {
+    FTS* ret;
+    POINTER_RETRY_ON_EINTR(ret, fts_open(
+        paths, FTS_PHYSICAL | FTS_XDEV | FTS_NOCHDIR, nullptr));
+    if (ret == nullptr) {
       return IOError(root, errno);
     }
+    unique_ptr<FTS, FtsCloser> tree(ret);
 
     FTSENT *ent = nullptr;
     bool had_errors = false;
@@ -1717,7 +1748,13 @@ class PosixEnv : public Env {
   // unique_ptr Deleter implementation for fts_close
   struct FtsCloser {
     void operator()(FTS *fts) const {
-      if (fts) { fts_close(fts); }
+      if (fts) {
+        int err;
+        RETRY_ON_EINTR(err, fts_close(fts));
+        if (PREDICT_FALSE(err != 0)) {
+          PLOG(WARNING) << "Failed to close fts";
+        }
+      }
     }
   };
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/b7cf3b2e/src/kudu/util/net/socket.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/net/socket.cc b/src/kudu/util/net/socket.cc
index f1d2466..cc14702 100644
--- a/src/kudu/util/net/socket.cc
+++ b/src/kudu/util/net/socket.cc
@@ -44,7 +44,6 @@
 #include "kudu/util/monotime.h"
 #include "kudu/util/net/net_util.h"
 #include "kudu/util/net/sockaddr.h"
-#include "kudu/util/os-util.h"
 #include "kudu/util/random.h"
 #include "kudu/util/random_util.h"
 #include "kudu/util/slice.h"
@@ -94,7 +93,9 @@ Status Socket::Close() {
     return Status::OK();
   }
   int fd = fd_;
-  if (::close(fd) < 0) {
+  int ret;
+  RETRY_ON_EINTR(ret, ::close(fd));
+  if (ret < 0) {
     int err = errno;
     return Status::NetworkError("close error", ErrnoToString(err), err);
   }
@@ -379,7 +380,10 @@ Status Socket::Connect(const Sockaddr &remote) {
   struct sockaddr_in addr;
   memcpy(&addr, &remote.addr(), sizeof(sockaddr_in));
   DCHECK_GE(fd_, 0);
-  if (::connect(fd_, reinterpret_cast<const struct sockaddr*>(&addr), sizeof(addr))
< 0) {
+  int ret;
+  RETRY_ON_EINTR(ret, ::connect(
+      fd_, reinterpret_cast<const struct sockaddr*>(&addr), sizeof(addr)));
+  if (ret < 0) {
     int err = errno;
     return Status::NetworkError("connect(2) error", ErrnoToString(err), err);
   }
@@ -408,7 +412,8 @@ Status Socket::Write(const uint8_t *buf, int32_t amt, int32_t *nwritten)
{
                            amt), Slice(), EINVAL);
   }
   DCHECK_GE(fd_, 0);
-  int res = ::send(fd_, buf, amt, MSG_NOSIGNAL);
+  int res;
+  RETRY_ON_EINTR(res, ::send(fd_, buf, amt, MSG_NOSIGNAL));
   if (res < 0) {
     int err = errno;
     return Status::NetworkError("write error", ErrnoToString(err), err);
@@ -431,7 +436,8 @@ Status Socket::Writev(const struct ::iovec *iov, int iov_len,
   memset(&msg, 0, sizeof(struct msghdr));
   msg.msg_iov = const_cast<iovec *>(iov);
   msg.msg_iovlen = iov_len;
-  ssize_t res = ::sendmsg(fd_, &msg, MSG_NOSIGNAL);
+  ssize_t res;
+  RETRY_ON_EINTR(res, ::sendmsg(fd_, &msg, MSG_NOSIGNAL));
   if (PREDICT_FALSE(res < 0)) {
     int err = errno;
     return Status::NetworkError("sendmsg error", ErrnoToString(err), err);

http://git-wip-us.apache.org/repos/asf/kudu/blob/b7cf3b2e/src/kudu/util/os-util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/os-util.cc b/src/kudu/util/os-util.cc
index d4ab2c7..df7761f 100644
--- a/src/kudu/util/os-util.cc
+++ b/src/kudu/util/os-util.cc
@@ -28,14 +28,15 @@
 #include <sys/resource.h>
 #include <unistd.h>
 
-#include <glog/logging.h>
-
 #include <cstddef>
 #include <fstream>
 #include <string>
 #include <utility>
 #include <vector>
 
+#include <glog/logging.h>
+
+#include "kudu/gutil/macros.h"
 #include "kudu/gutil/strings/numbers.h"
 #include "kudu/gutil/strings/split.h"
 #include "kudu/gutil/strings/stringpiece.h"
@@ -141,10 +142,13 @@ void DisableCoreDumps() {
   // is set to a pipe rather than a file, it's not sufficient. Setting
   // this pattern results in piping a very minimal dump into the core
   // processor (eg abrtd), thus speeding up the crash.
-  int f = open("/proc/self/coredump_filter", O_WRONLY);
+  int f;
+  RETRY_ON_EINTR(f, open("/proc/self/coredump_filter", O_WRONLY));
   if (f >= 0) {
-    write(f, "00000000", 8);
-    close(f);
+    ssize_t ret;
+    RETRY_ON_EINTR(ret, write(f, "00000000", 8));
+    int close_ret;
+    RETRY_ON_EINTR(close_ret, close(f));
   }
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/b7cf3b2e/src/kudu/util/os-util.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/os-util.h b/src/kudu/util/os-util.h
index c79d579..7e1bbb6 100644
--- a/src/kudu/util/os-util.h
+++ b/src/kudu/util/os-util.h
@@ -23,21 +23,12 @@
 #ifndef KUDU_UTIL_OS_UTIL_H
 #define KUDU_UTIL_OS_UTIL_H
 
-#include <errno.h>
-
 #include <cstdint>
 #include <string>
 #include <type_traits> // IWYU pragma: keep
 
 #include "kudu/util/status.h"
 
-// Retry on EINTR for functions like read() that return -1 on error.
-#define RETRY_ON_EINTR(err, expr) do { \
-  static_assert(std::is_signed<decltype(err)>::value, \
-                #err " must be a signed integer"); \
-  (err) = (expr); \
-} while ((err) == -1 && errno == EINTR)
-
 namespace kudu {
 
 // Utility methods to read interesting values from /proc.

http://git-wip-us.apache.org/repos/asf/kudu/blob/b7cf3b2e/src/kudu/util/pstack_watcher-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/pstack_watcher-test.cc b/src/kudu/util/pstack_watcher-test.cc
index abc39e4..a993d66 100644
--- a/src/kudu/util/pstack_watcher-test.cc
+++ b/src/kudu/util/pstack_watcher-test.cc
@@ -20,18 +20,20 @@
 #include <unistd.h>
 
 #include <cerrno>
-#include <memory>
 #include <cstdio>
+#include <memory>
 #include <string>
 
 #include <glog/logging.h>
 #include <gtest/gtest.h>
 
+#include "kudu/gutil/macros.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/env.h"
 #include "kudu/util/errno.h"
 #include "kudu/util/faststring.h"
 #include "kudu/util/monotime.h"
+#include "kudu/util/scoped_cleanup.h"
 #include "kudu/util/status.h"
 #include "kudu/util/test_macros.h"
 
@@ -55,28 +57,36 @@ TEST(TestPstackWatcher, TestDumpStacks) {
   ASSERT_OK(PstackWatcher::DumpStacks());
 }
 
-static shared_ptr<FILE> RedirectStdout(string *temp_path) {
+static FILE* RedirectStdout(string *temp_path) {
   string temp_dir;
   CHECK_OK(Env::Default()->GetTestDirectory(&temp_dir));
   *temp_path = Substitute("$0/pstack_watcher-dump.$1.txt",
                       temp_dir, getpid());
-  return shared_ptr<FILE>(
-      freopen(temp_path->c_str(), "w", stdout), fclose);
+  FILE* reopened;
+  POINTER_RETRY_ON_EINTR(reopened, freopen(temp_path->c_str(), "w", stdout));
+  return reopened;
 }
 
 TEST(TestPstackWatcher, TestPstackWatcherRunning) {
   string stdout_file;
   int old_stdout;
-  CHECK_ERR(old_stdout = dup(STDOUT_FILENO));
+  RETRY_ON_EINTR(old_stdout, dup(STDOUT_FILENO));
+  CHECK_ERR(old_stdout);
   {
-    shared_ptr<FILE> out_fp = RedirectStdout(&stdout_file);
-    PCHECK(out_fp.get());
+    FILE* out_fp = RedirectStdout(&stdout_file);
+    PCHECK(out_fp != nullptr);
+    SCOPED_CLEANUP({
+        int err;
+        RETRY_ON_EINTR(err, fclose(out_fp));
+      });
     PstackWatcher watcher(MonoDelta::FromMilliseconds(500));
     while (watcher.IsRunning()) {
       SleepFor(MonoDelta::FromMilliseconds(1));
     }
   }
-  CHECK_ERR(dup2(old_stdout, STDOUT_FILENO));
+  int dup2_ret;
+  RETRY_ON_EINTR(dup2_ret, dup2(old_stdout, STDOUT_FILENO));
+  CHECK_ERR(dup2_ret);
   PCHECK(stdout = fdopen(STDOUT_FILENO, "w"));
 
   faststring contents;

http://git-wip-us.apache.org/repos/asf/kudu/blob/b7cf3b2e/src/kudu/util/pstack_watcher.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/pstack_watcher.cc b/src/kudu/util/pstack_watcher.cc
index 5236416..2c4481a 100644
--- a/src/kudu/util/pstack_watcher.cc
+++ b/src/kudu/util/pstack_watcher.cc
@@ -27,6 +27,7 @@
 #include <boost/bind.hpp>
 #include <glog/logging.h>
 
+#include "kudu/gutil/macros.h"
 #include "kudu/gutil/strings/numbers.h"
 #include "kudu/gutil/strings/split.h"
 #include "kudu/gutil/strings/strip.h"
@@ -224,7 +225,9 @@ Status PstackWatcher::RunStackDump(const vector<string>& argv)
{
   }
   Subprocess pstack_proc(argv);
   RETURN_NOT_OK_PREPEND(pstack_proc.Start(), "RunStackDump proc.Start() failed");
-  if (::close(pstack_proc.ReleaseChildStdinFd()) == -1) {
+  int ret;
+  RETRY_ON_EINTR(ret, ::close(pstack_proc.ReleaseChildStdinFd()));
+  if (ret == -1) {
     return Status::IOError("Unable to close child stdin", ErrnoToString(errno), errno);
   }
   RETURN_NOT_OK_PREPEND(pstack_proc.Wait(), "RunStackDump proc.Wait() failed");

http://git-wip-us.apache.org/repos/asf/kudu/blob/b7cf3b2e/src/kudu/util/semaphore.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/semaphore.cc b/src/kudu/util/semaphore.cc
index d9bc5a5..72ff214 100644
--- a/src/kudu/util/semaphore.cc
+++ b/src/kudu/util/semaphore.cc
@@ -47,23 +47,23 @@ Semaphore::~Semaphore() {
 
 void Semaphore::Acquire() {
   while (true) {
-    int ret = sem_wait(&sem_);
+    int ret;
+    RETRY_ON_EINTR(ret, sem_wait(&sem_));
     if (ret == 0) {
-      // TODO: would be nice to track acquisition time, etc.
+      // TODO(todd): would be nice to track acquisition time, etc.
       return;
     }
-
-    if (errno == EINTR) continue;
     Fatal("wait");
   }
 }
 
 bool Semaphore::TryAcquire() {
-  int ret = sem_trywait(&sem_);
+  int ret;
+  RETRY_ON_EINTR(ret, sem_trywait(&sem_));
   if (ret == 0) {
     return true;
   }
-  if (errno == EAGAIN || errno == EINTR) {
+  if (errno == EAGAIN) {
     return false;
   }
   Fatal("trywait");
@@ -78,10 +78,10 @@ bool Semaphore::TimedAcquire(const MonoDelta& timeout) {
                              &abs_timeout);
 
   while (true) {
-    int ret = sem_timedwait(&sem_, &abs_timeout);
+    int ret;
+    RETRY_ON_EINTR(ret, sem_timedwait(&sem_, &abs_timeout));
     if (ret == 0) return true;
     if (errno == ETIMEDOUT) return false;
-    if (errno == EINTR) continue;
     Fatal("timedwait");
   }
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/b7cf3b2e/src/kudu/util/subprocess-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/subprocess-test.cc b/src/kudu/util/subprocess-test.cc
index c7ab804..24d7cb3 100644
--- a/src/kudu/util/subprocess-test.cc
+++ b/src/kudu/util/subprocess-test.cc
@@ -35,6 +35,7 @@
 #include <glog/logging.h>
 #include <gtest/gtest.h>
 
+#include "kudu/gutil/macros.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/env.h"
 #include "kudu/util/monotime.h"
@@ -67,7 +68,8 @@ TEST_F(SubprocessTest, TestSimplePipe) {
   fprintf(out, "hello world\n");
   // We have to close 'out' or else tr won't write any output, since
   // it enters a buffered mode if it detects that its input is a FIFO.
-  fclose(out);
+  int err;
+  RETRY_ON_EINTR(err, fclose(out));
 
   char buf[1024];
   ASSERT_EQ(buf, fgets(buf, sizeof(buf), in));
@@ -88,7 +90,10 @@ TEST_F(SubprocessTest, TestErrPipe) {
   PCHECK(out);
 
   fprintf(out, "Hello, World\n");
-  fclose(out); // same reasoning as above, flush to prevent tee buffering
+
+  // Same reasoning as above, flush to prevent tee buffering.
+  int err;
+  RETRY_ON_EINTR(err, fclose(out));
 
   FILE* in = fdopen(p.from_child_stderr_fd(), "r");
   PCHECK(in);

http://git-wip-us.apache.org/repos/asf/kudu/blob/b7cf3b2e/src/kudu/util/subprocess.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/subprocess.cc b/src/kudu/util/subprocess.cc
index 9e550ce..d68cb7f 100644
--- a/src/kudu/util/subprocess.cc
+++ b/src/kudu/util/subprocess.cc
@@ -51,7 +51,6 @@
 #include "kudu/util/errno.h"
 #include "kudu/util/faststring.h"
 #include "kudu/util/monotime.h"
-#include "kudu/util/os-util.h"
 #include "kudu/util/path_util.h"
 #include "kudu/util/signal.h"
 #include "kudu/util/status.h"
@@ -146,7 +145,8 @@ void CloseNonStandardFDs(DIR* fd_dir) {
           fd == STDOUT_FILENO ||
           fd == STDERR_FILENO ||
           fd == dir_fd))  {
-      close(fd);
+      int ret;
+      RETRY_ON_EINTR(ret, close(fd));
     }
   }
 }
@@ -163,11 +163,14 @@ void RedirectToDevNull(int fd) {
   // It is expected that the file descriptor allocated when opening /dev/null
   // will be closed when the child process closes all of its "non-standard"
   // file descriptors later on.
-  int dev_null = open("/dev/null", O_WRONLY);
+  int dev_null;
+  RETRY_ON_EINTR(dev_null, open("/dev/null", O_WRONLY));
   if (dev_null < 0) {
     PLOG(WARNING) << "failed to open /dev/null";
   } else {
-    PCHECK(dup2(dev_null, fd));
+    int ret;
+    RETRY_ON_EINTR(ret, dup2(dev_null, fd));
+    PCHECK(ret);
   }
 }
 
@@ -190,14 +193,12 @@ class ReadFdsFullyHelper {
     DCHECK_EQ(ev::READ, revents);
 
     char buf[1024];
-    ssize_t n = read(w.fd, buf, arraysize(buf));
+    ssize_t n;
+    RETRY_ON_EINTR(n, read(w.fd, buf, arraysize(buf)));
     if (n == 0) {
       // EOF, stop watching.
       w.stop();
     } else if (n < 0) {
-      // Interrupted by a signal, do nothing.
-      if (errno == EINTR) return;
-
       // A fatal error. Store it and stop watching.
       status_ = Status::IOError("IO error reading from " + progname_,
                                 ErrnoToString(errno), errno);
@@ -283,7 +284,8 @@ Subprocess::~Subprocess() {
 
   for (int i = 0; i < 3; ++i) {
     if (fd_state_[i] == PIPED && child_fds_[i] >= 0) {
-      close(child_fds_[i]);
+      int ret;
+      RETRY_ON_EINTR(ret, close(child_fds_[i]));
     }
   }
 }
@@ -297,13 +299,15 @@ static int pipe2(int pipefd[2], int flags) {
     return -1;
   }
   if (fcntl(new_fds[0], F_SETFD, O_CLOEXEC) == -1) {
-    close(new_fds[0]);
-    close(new_fds[1]);
+    int ret;
+    RETRY_ON_EINTR(ret, close(new_fds[0]));
+    RETRY_ON_EINTR(ret, close(new_fds[1]));
     return -1;
   }
   if (fcntl(new_fds[1], F_SETFD, O_CLOEXEC) == -1) {
-    close(new_fds[0]);
-    close(new_fds[1]);
+    int ret;
+    RETRY_ON_EINTR(ret, close(new_fds[0]));
+    RETRY_ON_EINTR(ret, close(new_fds[1]));
     return -1;
   }
   pipefd[0] = new_fds[0];
@@ -376,7 +380,9 @@ Status Subprocess::Start() {
 
     // stdin
     if (fd_state_[STDIN_FILENO] == PIPED) {
-      PCHECK(dup2(child_stdin[0], STDIN_FILENO) == STDIN_FILENO);
+      int dup2_ret;
+      RETRY_ON_EINTR(dup2_ret, dup2(child_stdin[0], STDIN_FILENO));
+      PCHECK(dup2_ret == STDIN_FILENO);
     } else {
       DCHECK_EQ(SHARED, fd_state_[STDIN_FILENO]);
     }
@@ -384,7 +390,9 @@ Status Subprocess::Start() {
     // stdout
     switch (fd_state_[STDOUT_FILENO]) {
       case PIPED: {
-        PCHECK(dup2(child_stdout[1], STDOUT_FILENO) == STDOUT_FILENO);
+        int dup2_ret;
+        RETRY_ON_EINTR(dup2_ret, dup2(child_stdout[1], STDOUT_FILENO));
+        PCHECK(dup2_ret == STDOUT_FILENO);
         break;
       }
       case DISABLED: {
@@ -399,7 +407,9 @@ Status Subprocess::Start() {
     // stderr
     switch (fd_state_[STDERR_FILENO]) {
       case PIPED: {
-        PCHECK(dup2(child_stderr[1], STDERR_FILENO) == STDERR_FILENO);
+        int dup2_ret;
+        RETRY_ON_EINTR(dup2_ret, dup2(child_stderr[1], STDERR_FILENO));
+        PCHECK(dup2_ret == STDERR_FILENO);
         break;
       }
       case DISABLED: {
@@ -413,7 +423,9 @@ Status Subprocess::Start() {
 
     // Close the read side of the sync pipe;
     // the write side should be closed upon execvp().
-    PCHECK(close(sync_pipe[0]) == 0);
+    int close_ret;
+    RETRY_ON_EINTR(close_ret, close(sync_pipe[0]));
+    PCHECK(close_ret == 0);
 
     CloseNonStandardFDs(fd_dir);
 
@@ -446,9 +458,10 @@ Status Subprocess::Start() {
     // We are the parent
     child_pid_ = ret;
     // Close child's side of the pipes
-    if (fd_state_[STDIN_FILENO]  == PIPED) close(child_stdin[0]);
-    if (fd_state_[STDOUT_FILENO] == PIPED) close(child_stdout[1]);
-    if (fd_state_[STDERR_FILENO] == PIPED) close(child_stderr[1]);
+    int close_ret;
+    if (fd_state_[STDIN_FILENO]  == PIPED) RETRY_ON_EINTR(close_ret, close(child_stdin[0]));
+    if (fd_state_[STDOUT_FILENO] == PIPED) RETRY_ON_EINTR(close_ret, close(child_stdout[1]));
+    if (fd_state_[STDERR_FILENO] == PIPED) RETRY_ON_EINTR(close_ret, close(child_stderr[1]));
     // Keep parent's side of the pipes
     child_fds_[STDIN_FILENO]  = child_stdin[1];
     child_fds_[STDOUT_FILENO] = child_stdout[0];
@@ -464,19 +477,18 @@ Status Subprocess::Start() {
       // Close the write side of the sync pipe. It's crucial to make sure
       // it succeeds otherwise the blocking read() below might wait forever
       // even if the child process has closed the pipe.
-      PCHECK(close(sync_pipe[1]) == 0);
+      RETRY_ON_EINTR(close_ret, close(sync_pipe[1]));
+      PCHECK(close_ret == 0);
       while (true) {
         uint8_t buf;
         int err = 0;
-        const int rc = read(sync_pipe[0], &buf, 1);
+        int rc;
+        RETRY_ON_EINTR(rc, read(sync_pipe[0], &buf, 1));
         if (rc == -1) {
           err = errno;
-          if (err == EINTR) {
-            // Retry in case of a signal.
-            continue;
-          }
         }
-        PCHECK(close(sync_pipe[0]) == 0);
+        RETRY_ON_EINTR(close_ret, close(sync_pipe[0]));
+        PCHECK(close_ret == 0);
         if (rc == 0) {
           // That's OK -- expecting EOF from the other side of the pipe.
           break;
@@ -673,12 +685,17 @@ Status Subprocess::Call(const vector<string>& argv,
   RETURN_NOT_OK_PREPEND(p.Start(),
                         "Unable to fork " + argv[0]);
 
-  if (!stdin_in.empty() &&
-      write(p.to_child_stdin_fd(), stdin_in.data(), stdin_in.size()) < stdin_in.size())
{
-    return Status::IOError("Unable to write to child process stdin", ErrnoToString(errno),
errno);
+  if (!stdin_in.empty()) {
+    ssize_t written;
+    RETRY_ON_EINTR(written, write(p.to_child_stdin_fd(), stdin_in.data(), stdin_in.size()));
+    if (written < stdin_in.size()) {
+      return Status::IOError("Unable to write to child process stdin",
+                             ErrnoToString(errno), errno);
+    }
   }
 
-  int err = close(p.ReleaseChildStdinFd());
+  int err;
+  RETRY_ON_EINTR(err, close(p.ReleaseChildStdinFd()));
   if (PREDICT_FALSE(err != 0)) {
     return Status::IOError("Unable to close child process stdin", ErrnoToString(errno), errno);
   }


Mime
View raw message