POSIX: Add a macro for handling EINTR.

On POSIX systems, system calls can be interrupted by signals. In this
case, they'll return EINTR, indicating that the system call needs to
be restarted.

(The situation is a little more complicated than this with SA_RESTART,
but you can read man 7 signal if you like.)

The short of it is that you need to catch EINTR and restart the call
for these system calls:

 * read, readv, write, writev, ioctl
 * open() when dealing with a fifo
 * wait*
 * Anything socket based (send*, recv*, connect, accept etc)
 * flock and lock control with fcntl
 * mq_ functions which can block
 * futex
 * sem_wait (and timed wait)
 * pause, sigsuspend, sigtimedwait, sigwaitinfo
 * poll, epoll_wait, select and 'p' versions of the same
 * msgrcv, msgsnd, semop, semtimedop
 * close (although, on Linux, EINTR won't happen here)
 * any sleep functions (careful, you need to handle this are restart
   with different arguments)

We've been a little sloppy with this until now. This patch adds a
macro for dealing with this and corrects every case of these system
calls (that I found).

The macro is HANDLE_EINTR in base/eintr_wrapper.h. It's safe to
include on Windows and is a no-op there.

On POSIX, it uses GCC magic to return the correct type based on the
expression and restarts the system call if it throws EINTR.

And you can use it like:
 HANDLE_EINTR(close(fd));

Or:
 ssize_t bytes_read = HANDLE_EINTR(read(fd, buffer, len));

*BEWARE* that it will evaluate the argument multiple times, so this is
not safe:
 HANDLE_EINTR(close(FireMissiles()));

http://groups.google.com/group/chromium-dev/browse_thread/thread/41a35b2a457d73a0
http://codereview.chromium.org/100225


git-svn-id: svn://svn.chromium.org/chrome/trunk/src@15102 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 15 insertions, 25 deletions

diff --git a/base/process_util_posix.cc b/base/process_util_posix.cc
index c0e792b..e66576a 100644
--- a/base/process_util_posix.cc
+++ b/base/process_util_posix.cc
@@ -17,6 +17,7 @@
 #include <set>
 
 #include "base/basictypes.h"
+#include "base/eintr_wrapper.h"
 #include "base/logging.h"
 #include "base/platform_thread.h"
 #include "base/process_util.h"
@@ -69,7 +70,7 @@ bool KillProcess(ProcessHandle process_id, int exit_code, bool wait) {
     int tries = 60;
     // The process may not end immediately due to pending I/O
     while (tries-- > 0) {
-      int pid = waitpid(process_id, NULL, WNOHANG);
+      int pid = HANDLE_EINTR(waitpid(process_id, NULL, WNOHANG));
       if (pid == process_id)
         break;
 
@@ -139,10 +140,7 @@ void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
       if (saved_fds.find(fd) != saved_fds.end())
         continue;
 
-      int result;
-      do {
-        result = close(fd);
-      } while (result == -1 && errno == EINTR);
+      HANDLE_EINTR(close(fd));
     }
     return;
   }
@@ -161,10 +159,7 @@ void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
     if (saved_fds.find(fd) != saved_fds.end())
       continue;
 
-    int result;
-    do {
-      result = close(fd);
-    } while (result == -1 && errno == EINTR);
+    HANDLE_EINTR(close(fd));
   }
 }
 
@@ -226,7 +221,7 @@ void RaiseProcessToHighPriority() {
 
 bool DidProcessCrash(bool* child_exited, ProcessHandle handle) {
   int status;
-  const int result = waitpid(handle, &status, WNOHANG);
+  const int result = HANDLE_EINTR(waitpid(handle, &status, WNOHANG));
   if (result == -1) {
     LOG(ERROR) << "waitpid failed pid:" << handle << " errno:" << errno;
     if (child_exited)
@@ -262,11 +257,9 @@ bool DidProcessCrash(bool* child_exited, ProcessHandle handle) {
 
 bool WaitForExitCode(ProcessHandle handle, int* exit_code) {
   int status;
-  while (waitpid(handle, &status, 0) == -1) {
-    if (errno != EINTR) {
-      NOTREACHED();
-      return false;
-    }
+  if (HANDLE_EINTR(waitpid(handle, &status, 0)) == -1) {
+    NOTREACHED();
+    return false;
   }
 
   if (WIFEXITED(status)) {
@@ -305,7 +298,7 @@ int WaitpidWithTimeout(ProcessHandle handle, int wait_milliseconds,
   // This function is used primarily for unit tests, if we want to use it in
   // the application itself it would probably be best to examine other routes.
   int status = -1;
-  pid_t ret_pid = waitpid(handle, &status, WNOHANG);
+  pid_t ret_pid = HANDLE_EINTR(waitpid(handle, &status, WNOHANG));
   static const int64 kQuarterSecondInMicroseconds = kMicrosecondsPerSecond/4;
 
   // If the process hasn't exited yet, then sleep and try again.
@@ -325,7 +318,7 @@ int WaitpidWithTimeout(ProcessHandle handle, int wait_milliseconds,
     // usleep() will return 0 and set errno to EINTR on receipt of a signal
     // such as SIGCHLD.
     usleep(sleep_time_usecs);
-    ret_pid = waitpid(handle, &status, WNOHANG);
+    ret_pid = HANDLE_EINTR(waitpid(handle, &status, WNOHANG));
   }
 
   if (success)
@@ -340,7 +333,7 @@ bool WaitForSingleProcess(ProcessHandle handle, int wait_milliseconds) {
   bool waitpid_success;
   int status;
   if (wait_milliseconds == base::kNoTimeout)
-    waitpid_success = (waitpid(handle, &status, 0) != -1);
+    waitpid_success = (HANDLE_EINTR(waitpid(handle, &status, 0)) != -1);
   else
     status = WaitpidWithTimeout(handle, wait_milliseconds, &waitpid_success);
   if (status != -1) {
@@ -463,16 +456,13 @@ bool GetAppOutput(const CommandLine& cl, std::string* output) {
 
         char buffer[256];
         std::string buf_output;
-        ssize_t bytes_read = 0;
 
         while (true) {
-          bytes_read = read(pipe_fd[0], buffer, sizeof(buffer));
-          if (bytes_read == 0)
-            break;
-          if (bytes_read == -1 && errno != EINTR)
+          ssize_t bytes_read =
+              HANDLE_EINTR(read(pipe_fd[0], buffer, sizeof(buffer)));
+          if (bytes_read <= 0)
             break;
-          if (bytes_read > 0)
-            buf_output.append(buffer, bytes_read);
+          buf_output.append(buffer, bytes_read);
         }
         output->swap(buf_output);
         close(pipe_fd[0]);