* Add timeout support to POSIX WaitForSingleProcess()

* Unify Linux & Mac implementations of WaitForSingleProcess()

Review URL: http://codereview.chromium.org/12969

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

1 files changed, 42 insertions, 5 deletions

diff --git a/base/process_util_posix.cc b/base/process_util_posix.cc
index 3bead6a..3153a59 100644
--- a/base/process_util_posix.cc
+++ b/base/process_util_posix.cc
@@ -54,19 +54,56 @@ void RaiseProcessToHighPriority() {
 }
 
 bool WaitForSingleProcess(ProcessHandle handle, int wait_milliseconds) {
-  int status;
+  // This POSIX version of this function only guarantees that we wait no less
+  // than |wait_milliseconds| for the proces to exit.  The child process may
+  // exit sometime before the timeout has ended but we may still block for
+  // up to 0.25 seconds after the fact.
+  //
+  // waitpid() has no direct support on POSIX for specifying a timeout, you can
+  // either ask it to block indefinitely or return immediately (WNOHANG).
+  // When a child process terminates a SIGCHLD signal is sent to the parent.
+  // Catching this signal would involve installing a signal handler which may
+  // affect other parts of the application and would be difficult to debug.
+  //
+  // Our strategy is to call waitpid() once up front to check if the process
+  // has already exited, otherwise to loop for wait_milliseconds, sleeping for
+  // at most 0.25 secs each time using usleep() and then calling waitpid().
+  //
+  // usleep() is speced to exit if a signal is received for which a handler
+  // has been installed.  This means that when a SIGCHLD is sent, it will exit
+  // depending on behavior external to this function.
+  //
+  // This function is used primarilly 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);
+  static const int64 kQuarterSecondInMicroseconds = kMicrosecondsPerSecond/4;
 
   // If the process hasn't exited yet, then sleep and try again.
   Time wakeup_time = Time::Now() + TimeDelta::FromMilliseconds(
       wait_milliseconds);
-  while (ret_pid == 0 && Time::Now() < wakeup_time) {
-    int64 sleep_time_usecs = (wakeup_time - Time::Now()).InMicroseconds();
-    usleep(sleep_time_usecs);  // usleep will exit on EINTR.
+  while (ret_pid == 0) {
+    Time now = Time::Now();
+    if (now > wakeup_time)
+      break;
+    // Guaranteed to be non-negative!
+    int64 sleep_time_usecs = (wakeup_time - now).InMicroseconds();
+    // Don't sleep for more than 0.25 secs at a time.
+    if (sleep_time_usecs > kQuarterSecondInMicroseconds) {
+      sleep_time_usecs = kQuarterSecondInMicroseconds;
+    }
+
+    // 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);
   }
 
-  return WIFEXITED(status);
+  if (status != -1) {
+    return WIFEXITED(status);
+  } else {
+    return false;
+  }
 }
 
 namespace {