Add non-blocking Wait() for launched processes

- New ProcessInfo class to encapsulate information about child processes.
- Generalized the Wait() to support non-blocking wait on child processes.
- ExecuteNoWait() now returns a ProcessInfo object with information about
  the launched child. Users will be able to use this object to
  perform non-blocking wait.
- ExecuteNoWait() now accepts an ExecutionFailed param that tells if execution
  failed or not.

These changes will allow users to implement basic process parallel
tools.

Differential Revision: http://llvm-reviews.chandlerc.com/D1728


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

include/llvm/Support/Program.h

@@ -30,6 +30,28 @@ namespace sys {
   const char EnvPathSeparator = ';';
 #endif
 
+/// @brief This struct encapsulates information about a process.
+struct ProcessInfo {
+#if defined(LLVM_ON_UNIX)
+  typedef pid_t ProcessId;
+#elif defined(LLVM_ON_WIN32)
+  typedef unsigned long ProcessId; // Must match the type of DWORD on Windows.
+  typedef void * HANDLE; // Must match the type of HANDLE on Windows.
+  /// The handle to the process (available on Windows only).
+  HANDLE ProcessHandle;
+#else
+#error "ProcessInfo is not defined for this platform!"
+#endif
+
+  /// The process identifier.
+  ProcessId Pid;
+
+  /// The return code, set after execution.
+  int ReturnCode;
+
+  ProcessInfo();
+};
+
   /// This static constructor (factory) will attempt to locate a program in
   /// the operating system's file system using some pre-determined set of
   /// locations to search (e.g. the PATH on Unix). Paths with slashes are
@@ -87,15 +109,41 @@ namespace sys {
       ///< program.
       bool *ExecutionFailed = 0);
 
-  /// Similar to ExecuteAndWait, but return immediately.
-  void ExecuteNoWait(StringRef Program, const char **args, const char **env = 0,
-                     const StringRef **redirects = 0, unsigned memoryLimit = 0,
-                     std::string *ErrMsg = 0);
+  /// Similar to ExecuteAndWait, but returns immediately.
+  /// @returns The \see ProcessInfo of the newly launced process.
+  /// \Note On Microsoft Windows systems, users will need to either call \see
+  /// Wait until the process finished execution or win32 CloseHandle() API on
+  /// ProcessInfo.ProcessHandle to avoid memory leaks.
+  ProcessInfo
+  ExecuteNoWait(StringRef Program, const char **args, const char **env = 0,
+                const StringRef **redirects = 0, unsigned memoryLimit = 0,
+                std::string *ErrMsg = 0, bool *ExecutionFailed = 0);
 
-  // Return true if the given arguments fit within system-specific
-  // argument length limits.
+  /// Return true if the given arguments fit within system-specific
+  /// argument length limits.
   bool argumentsFitWithinSystemLimits(ArrayRef<const char*> Args);
-}
+
+  /// This function waits for the process specified by \p PI to finish.
+  /// \returns A \see ProcessInfo struct with Pid set to:
+  /// \li The process id of the child process if the child process has changed
+  /// state.
+  /// \li 0 if the child process has not changed state.
+  /// \Note Users of this function should always check the ReturnCode member of
+  /// the \see ProcessInfo returned from this function.
+  ProcessInfo Wait(
+      const ProcessInfo &PI, ///< The child process that should be waited on.
+      unsigned SecondsToWait, ///< If non-zero, this specifies the amount of
+      ///< time to wait for the child process to exit. If the time expires, the
+      ///< child is killed and this function returns. If zero, this function
+      ///< will perform a non-blocking wait on the child process.
+      bool WaitUntilTerminates, ///< If true, ignores \p SecondsToWait and waits
+      ///< until child has terminated.
+      std::string *ErrMsg = 0 ///< If non-zero, provides a pointer to a string
+      ///< instance in which error messages will be returned. If the string
+      ///< is non-empty upon return an error occurred while invoking the
+      ///< program.
+      );
+  }
 }
 
 #endif

lib/Support/Program.cpp

@@ -22,30 +22,40 @@ using namespace sys;
 //===          independent code.
 //===----------------------------------------------------------------------===//
 
-static bool Execute(void **Data, StringRef Program, const char **args,
+static bool Execute(ProcessInfo &PI, StringRef Program, const char **args,
                     const char **env, const StringRef **Redirects,
                     unsigned memoryLimit, std::string *ErrMsg);
 
-static int Wait(void *&Data, StringRef Program, unsigned secondsToWait,
-                std::string *ErrMsg);
-
 int sys::ExecuteAndWait(StringRef Program, const char **args, const char **envp,
                         const StringRef **redirects, unsigned secondsToWait,
                         unsigned memoryLimit, std::string *ErrMsg,
                         bool *ExecutionFailed) {
-  void *Data = 0;
-  if (Execute(&Data, Program, args, envp, redirects, memoryLimit, ErrMsg)) {
-    if (ExecutionFailed) *ExecutionFailed = false;
-    return Wait(Data, Program, secondsToWait, ErrMsg);
+  ProcessInfo PI;
+  if (Execute(PI, Program, args, envp, redirects, memoryLimit, ErrMsg)) {
+    if (ExecutionFailed)
+      *ExecutionFailed = false;
+    ProcessInfo Result = Wait(PI, secondsToWait, true, ErrMsg);
+    return Result.ReturnCode;
   }
-  if (ExecutionFailed) *ExecutionFailed = true;
+
+  if (ExecutionFailed)
+    *ExecutionFailed = true;
+
   return -1;
 }
 
-void sys::ExecuteNoWait(StringRef Program, const char **args, const char **envp,
-                        const StringRef **redirects, unsigned memoryLimit,
-                        std::string *ErrMsg) {
-  Execute(/*Data*/ 0, Program, args, envp, redirects, memoryLimit, ErrMsg);
+ProcessInfo sys::ExecuteNoWait(StringRef Program, const char **args,
+                               const char **envp, const StringRef **redirects,
+                               unsigned memoryLimit, std::string *ErrMsg,
+                               bool *ExecutionFailed) {
+  ProcessInfo PI;
+  if (ExecutionFailed)
+    *ExecutionFailed = false;
+  if (!Execute(PI, Program, args, envp, redirects, memoryLimit, ErrMsg))
+    if (ExecutionFailed)
+      *ExecutionFailed = true;
+
+  return PI;
 }
 
 // Include the platform-specific parts of this class.

lib/Support/Unix/Program.inc

@@ -47,6 +47,8 @@
 namespace llvm {
 using namespace sys;
 
+ProcessInfo::ProcessInfo() : Pid(0), ReturnCode(0) {}
+
 // This function just uses the PATH environment variable to find the program.
 std::string
 sys::FindProgramByName(const std::string& progName) {
@@ -175,9 +177,16 @@ static void SetMemoryLimits (unsigned size)
 
 }
 
-static bool Execute(void **Data, StringRef Program, const char **args,
+static bool Execute(ProcessInfo &PI, StringRef Program, const char **args,
                     const char **envp, const StringRef **redirects,
                     unsigned memoryLimit, std::string *ErrMsg) {
+  if (!llvm::sys::fs::exists(Program)) {
+    if (ErrMsg)
+      *ErrMsg = std::string("Executable \"") + Program.str() +
+                std::string("\" doesn't exist!");
+    return false;
+  }
+
   // If this OS has posix_spawn and there is no memory limit being implied, use
   // posix_spawn.  It is more efficient than fork/exec.
 #ifdef HAVE_POSIX_SPAWN
@@ -239,8 +248,8 @@ static bool Execute(void **Data, StringRef Program, const char **args,
     if (Err)
      return !MakeErrMsg(ErrMsg, "posix_spawn failed", Err);
 
-    if (Data)
-      *Data = reinterpret_cast<void*>(PID);
+    PI.Pid = PID;
+
     return true;
   }
 #endif
@@ -303,56 +312,71 @@ static bool Execute(void **Data, StringRef Program, const char **args,
       break;
   }
 
-  if (Data)
-    *Data = reinterpret_cast<void*>(child);
+  PI.Pid = child;
 
   return true;
 }
 
-static int Wait(void *&Data, StringRef Program, unsigned secondsToWait,
-                std::string *ErrMsg) {
+namespace llvm {
+
+ProcessInfo sys::Wait(const ProcessInfo &PI, unsigned SecondsToWait,
+                      bool WaitUntilTerminates, std::string *ErrMsg) {
 #ifdef HAVE_SYS_WAIT_H
   struct sigaction Act, Old;
-  assert(Data && "invalid pid to wait on, process not started?");
+  assert(PI.Pid && "invalid pid to wait on, process not started?");
 
-  // Install a timeout handler.  The handler itself does nothing, but the simple
-  // fact of having a handler at all causes the wait below to return with EINTR,
-  // unlike if we used SIG_IGN.
-  if (secondsToWait) {
+  int WaitPidOptions = 0;
+  pid_t ChildPid = PI.Pid;
+  if (WaitUntilTerminates) {
+    SecondsToWait = 0;
+    ChildPid = -1; // mimic a wait() using waitpid()
+  } else if (SecondsToWait) {
+    // Install a timeout handler.  The handler itself does nothing, but the
+    // simple fact of having a handler at all causes the wait below to return
+    // with EINTR, unlike if we used SIG_IGN.
     memset(&Act, 0, sizeof(Act));
     Act.sa_handler = TimeOutHandler;
     sigemptyset(&Act.sa_mask);
     sigaction(SIGALRM, &Act, &Old);
-    alarm(secondsToWait);
-  }
+    alarm(SecondsToWait);
+  } else if (SecondsToWait == 0)
+    WaitPidOptions = WNOHANG;
 
   // Parent process: Wait for the child process to terminate.
   int status;
-  uint64_t pid = reinterpret_cast<uint64_t>(Data);
-  pid_t child = static_cast<pid_t>(pid);
-  while (waitpid(pid, &status, 0) != child)
-    if (secondsToWait && errno == EINTR) {
-      // Kill the child.
-      kill(child, SIGKILL);
+  ProcessInfo WaitResult;
+  WaitResult.Pid = waitpid(ChildPid, &status, WaitPidOptions);
+  if (WaitResult.Pid != PI.Pid) {
+    if (WaitResult.Pid == 0) {
+      // Non-blocking wait.
+      return WaitResult;
+    } else {
+      if (SecondsToWait && errno == EINTR) {
+        // Kill the child.
+        kill(PI.Pid, SIGKILL);
 
-      // Turn off the alarm and restore the signal handler
-      alarm(0);
-      sigaction(SIGALRM, &Old, 0);
+        // Turn off the alarm and restore the signal handler
+        alarm(0);
+        sigaction(SIGALRM, &Old, 0);
 
-      // Wait for child to die
-      if (wait(&status) != child)
-        MakeErrMsg(ErrMsg, "Child timed out but wouldn't die");
-      else
-        MakeErrMsg(ErrMsg, "Child timed out", 0);
+        // Wait for child to die
+        if (wait(&status) != ChildPid)
+          MakeErrMsg(ErrMsg, "Child timed out but wouldn't die");
+        else
+          MakeErrMsg(ErrMsg, "Child timed out", 0);
 
-      return -2;   // Timeout detected
-    } else if (errno != EINTR) {
-      MakeErrMsg(ErrMsg, "Error waiting for child process");
-      return -1;
+        WaitResult.ReturnCode = -2; // Timeout detected
+        return WaitResult;
+      } else if (errno != EINTR) {
+        MakeErrMsg(ErrMsg, "Error waiting for child process");
+        WaitResult.ReturnCode = -1;
+        return WaitResult;
+      }
     }
+  }
 
   // We exited normally without timeout, so turn off the timer.
-  if (secondsToWait) {
+  if (SecondsToWait && !WaitUntilTerminates) {
     alarm(0);
     sigaction(SIGALRM, &Old, 0);
   }
@@ -362,24 +386,19 @@ static int Wait(void *&Data, StringRef Program, unsigned secondsToWait,
   int result = 0;
   if (WIFEXITED(status)) {
     result = WEXITSTATUS(status);
-#ifdef HAVE_POSIX_SPAWN
-    // The posix_spawn child process returns 127 on any kind of error.
-    // Following the POSIX convention for command-line tools (which posix_spawn
-    // itself apparently does not), check to see if the failure was due to some
-    // reason other than the file not existing, and return 126 in this case.
-    bool Exists;
-    if (result == 127 && !llvm::sys::fs::exists(Program, Exists) && Exists)
-      result = 126;
-#endif
+    WaitResult.ReturnCode = result;
+
     if (result == 127) {
       if (ErrMsg)
         *ErrMsg = llvm::sys::StrError(ENOENT);
-      return -1;
+      WaitResult.ReturnCode = -1;
+      return WaitResult;
     }
     if (result == 126) {
       if (ErrMsg)
         *ErrMsg = "Program could not be executed";
-      return -1;
+      WaitResult.ReturnCode = -1;
+      return WaitResult;
     }
   } else if (WIFSIGNALED(status)) {
     if (ErrMsg) {
@@ -391,18 +410,16 @@ static int Wait(void *&Data, StringRef Program, unsigned secondsToWait,
     }
     // Return a special value to indicate that the process received an unhandled
     // signal during execution as opposed to failing to execute.
-    return -2;
+    WaitResult.ReturnCode = -2;
   }
-  return result;
 #else
   if (ErrMsg)
     *ErrMsg = "Program::Wait is not implemented on this platform yet!";
-  return -1;
+  WaitResult.ReturnCode = -2;
 #endif
+  return WaitResult;
 }
 
-namespace llvm {
-
 error_code sys::ChangeStdinToBinary(){
   // Do nothing, as Unix doesn't differentiate between text and binary.
   return make_error_code(errc::success);
@@ -438,5 +455,4 @@ bool llvm::sys::argumentsFitWithinSystemLimits(ArrayRef<const char*> Args) {
   }
   return true;
 }
-
 }

lib/Support/Windows/Program.inc

@@ -24,16 +24,11 @@
 //===          and must not be UNIX code
 //===----------------------------------------------------------------------===//
 
-namespace {
-  struct Win32ProcessInfo {
-    HANDLE hProcess;
-    DWORD  dwProcessId;
-  };
-}
-
 namespace llvm {
 using namespace sys;
 
+ProcessInfo::ProcessInfo() : Pid(0), ProcessHandle(0), ReturnCode(0) {}
+
 // This function just uses the PATH environment variable to find the program.
 std::string sys::FindProgramByName(const std::string &progName) {
   // Check some degenerate cases
@@ -171,13 +166,9 @@ static unsigned int ArgLenWithQuotes(const char *Str) {
 
 }
 
-static bool Execute(void **Data,
-                    StringRef Program,
-                    const char** args,
-                    const char** envp,
-                    const StringRef** redirects,
-                    unsigned memoryLimit,
-                    std::string* ErrMsg) {
+static bool Execute(ProcessInfo &PI, StringRef Program, const char **args,
+                    const char **envp, const StringRef **redirects,
+                    unsigned memoryLimit, std::string *ErrMsg) {
   if (!sys::fs::can_execute(Program)) {
     if (ErrMsg)
       *ErrMsg = "program not executable";
@@ -316,12 +307,9 @@ static bool Execute(void **Data,
                ProgramStr + "'");
     return false;
   }
-  if (Data) {
-    Win32ProcessInfo* wpi = new Win32ProcessInfo;
-    wpi->hProcess = pi.hProcess;
-    wpi->dwProcessId = pi.dwProcessId;
-    *Data = wpi;
-  }
+
+  PI.Pid = pi.dwProcessId;
+  PI.ProcessHandle = pi.hProcess;
 
   // Make sure these get closed no matter what.
   ScopedCommonHandle hThread(pi.hThread);
@@ -351,68 +339,72 @@ static bool Execute(void **Data,
     }
   }
 
-  // Don't leak the handle if the caller doesn't want it.
-  if (!Data)
-    CloseHandle(pi.hProcess);
-
   return true;
 }
 
-static int WaitAux(Win32ProcessInfo *wpi, unsigned secondsToWait,
-                   std::string *ErrMsg) {
-  // Wait for the process to terminate.
-  HANDLE hProcess = wpi->hProcess;
-  DWORD millisecondsToWait = INFINITE;
-  if (secondsToWait > 0)
-    millisecondsToWait = secondsToWait * 1000;
+namespace llvm {
+ProcessInfo sys::Wait(const ProcessInfo &PI, unsigned SecondsToWait,
+                      bool WaitUntilChildTerminates, std::string *ErrMsg) {
+  assert(PI.Pid && "invalid pid to wait on, process not started?");
+  assert(PI.ProcessHandle &&
+         "invalid process handle to wait on, process not started?");
+  DWORD milliSecondsToWait = 0;
+  if (WaitUntilChildTerminates)
+    milliSecondsToWait = INFINITE;
+  else if (SecondsToWait > 0)
+    milliSecondsToWait = SecondsToWait * 1000;
 
-  if (WaitForSingleObject(hProcess, millisecondsToWait) == WAIT_TIMEOUT) {
-    if (!TerminateProcess(hProcess, 1)) {
-      MakeErrMsg(ErrMsg, "Failed to terminate timed-out program.");
-      // -2 indicates a crash or timeout as opposed to failure to execute.
-      return -2;
+  ProcessInfo WaitResult = PI;
+  DWORD WaitStatus = WaitForSingleObject(PI.ProcessHandle, milliSecondsToWait);
+  if (WaitStatus == WAIT_TIMEOUT) {
+    if (SecondsToWait) {
+      if (!TerminateProcess(PI.ProcessHandle, 1)) {
+        if (ErrMsg)
+          MakeErrMsg(ErrMsg, "Failed to terminate timed-out program.");
+
+        // -2 indicates a crash or timeout as opposed to failure to execute.
+        WaitResult.ReturnCode = -2;
+        CloseHandle(PI.ProcessHandle);
+        return WaitResult;
+      }
+      WaitForSingleObject(PI.ProcessHandle, INFINITE);
+      CloseHandle(PI.ProcessHandle);
+    } else {
+      // Non-blocking wait.
+      return ProcessInfo();
     }
-    WaitForSingleObject(hProcess, INFINITE);
   }
 
   // Get its exit status.
   DWORD status;
-  BOOL rc = GetExitCodeProcess(hProcess, &status);
+  BOOL rc = GetExitCodeProcess(PI.ProcessHandle, &status);
   DWORD err = GetLastError();
+  CloseHandle(PI.ProcessHandle);
 
   if (!rc) {
     SetLastError(err);
-    MakeErrMsg(ErrMsg, "Failed getting status for program.");
+    if (ErrMsg)
+      MakeErrMsg(ErrMsg, "Failed getting status for program.");
+
     // -2 indicates a crash or timeout as opposed to failure to execute.
-    return -2;
+    WaitResult.ReturnCode = -2;
+    return WaitResult;
   }
 
   if (!status)
-    return 0;
+    return WaitResult;
 
   // Pass 10(Warning) and 11(Error) to the callee as negative value.
   if ((status & 0xBFFF0000U) == 0x80000000U)
-    return (int)status;
+    WaitResult.ReturnCode = static_cast<int>(status);
+  else if (status & 0xFF)
+    WaitResult.ReturnCode = status & 0x7FFFFFFF;
+  else
+    WaitResult.ReturnCode = 1;
 
-  if (status & 0xFF)
-    return status & 0x7FFFFFFF;
-
-  return 1;
+  return WaitResult;
 }
 
-static int Wait(void *&Data, StringRef Program, unsigned secondsToWait,
-                std::string *ErrMsg) {
-  Win32ProcessInfo *wpi = reinterpret_cast<Win32ProcessInfo *>(Data);
-  int Ret = WaitAux(wpi, secondsToWait, ErrMsg);
-
-  CloseHandle(wpi->hProcess);
-  delete wpi;
-  Data = 0;
-
-  return Ret;
-}
-
-namespace llvm {
 error_code sys::ChangeStdinToBinary(){
   int result = _setmode( _fileno(stdin), _O_BINARY );
   if (result == -1)
@@ -449,5 +441,4 @@ bool llvm::sys::argumentsFitWithinSystemLimits(ArrayRef<const char*> Args) {
   }
   return true;
 }
-
 }

unittests/Support/ProgramTest.cpp

@@ -21,6 +21,20 @@
 extern char **environ;
 #endif
 
+#if defined(LLVM_ON_UNIX)
+#include <unistd.h>
+void sleep_for(unsigned int seconds) {
+  sleep(seconds);
+}
+#elif defined(LLVM_ON_WIN32)
+#include <windows.h>
+void sleep_for(unsigned int seconds) {
+  Sleep(seconds * 1000);
+}
+#else
+#error sleep_for is not implemented on your platform.
+#endif
+
 // From TestMain.cpp.
 extern const char *TestMainArgv0;
 
@@ -88,4 +102,93 @@ TEST(ProgramTest, CreateProcessTrailingSlash) {
   EXPECT_EQ(0, rc);
 }
 
+TEST(ProgramTest, TestExecuteNoWait) {
+  using namespace llvm::sys;
+
+  if (getenv("LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT")) {
+    sleep_for(/*seconds*/ 1);
+    exit(0);
+  }
+
+  std::string Executable =
+      sys::fs::getMainExecutable(TestMainArgv0, &ProgramTestStringArg1);
+  const char *argv[] = {
+    Executable.c_str(),
+    "--gtest_filter=ProgramTest.TestExecuteNoWait",
+    0
+  };
+
+  // Add LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT to the environment of the child.
+  std::vector<const char *> envp;
+  CopyEnvironment(envp);
+  envp.push_back("LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT=1");
+  envp.push_back(0);
+
+  std::string Error;
+  bool ExecutionFailed;
+  ProcessInfo PI1 =
+      ExecuteNoWait(Executable, argv, &envp[0], 0, 0, &Error, &ExecutionFailed);
+  ASSERT_FALSE(ExecutionFailed) << Error;
+  ASSERT_NE(PI1.Pid, 0) << "Invalid process id";
+
+  unsigned LoopCount = 0;
+
+  // Test that Wait() with WaitUntilTerminates=true works. In this case,
+  // LoopCount should only be incremented once.
+  while (true) {
+    ++LoopCount;
+    ProcessInfo WaitResult = Wait(PI1, 0, true, &Error);
+    ASSERT_TRUE(Error.empty());
+    if (WaitResult.Pid == PI1.Pid)
+      break;
+  }
+
+  EXPECT_EQ(LoopCount, 1u) << "LoopCount should be 1";
+
+  ProcessInfo PI2 =
+      ExecuteNoWait(Executable, argv, &envp[0], 0, 0, &Error, &ExecutionFailed);
+  ASSERT_FALSE(ExecutionFailed) << Error;
+  ASSERT_NE(PI2.Pid, 0) << "Invalid process id";
+
+  // Test that Wait() with SecondsToWait=0 performs a non-blocking wait. In this
+  // cse, LoopCount should be greater than 1 (more than one increment occurs).
+  while (true) {
+    ++LoopCount;
+    ProcessInfo WaitResult = Wait(PI2, 0, false, &Error);
+    ASSERT_TRUE(Error.empty());
+    if (WaitResult.Pid == PI2.Pid)
+      break;
+  }
+
+  ASSERT_GT(LoopCount, 1u) << "LoopCount should be >1";
+}
+
+TEST(ProgramTest, TestExecuteNegative) {
+  std::string Executable = "i_dont_exist";
+  const char *argv[] = { Executable.c_str(), 0 };
+
+  {
+    std::string Error;
+    bool ExecutionFailed;
+    int RetCode =
+        ExecuteAndWait(Executable, argv, 0, 0, 0, 0, &Error, &ExecutionFailed);
+    ASSERT_TRUE(RetCode < 0) << "On error ExecuteAndWait should return 0 or "
+                                "positive value indicating the result code";
+    ASSERT_TRUE(ExecutionFailed);
+    ASSERT_FALSE(Error.empty());
+  }
+
+  {
+    std::string Error;
+    bool ExecutionFailed;
+    ProcessInfo PI =
+        ExecuteNoWait(Executable, argv, 0, 0, 0, &Error, &ExecutionFailed);
+    ASSERT_EQ(PI.Pid, 0)
+        << "On error ExecuteNoWait should return an invalid ProcessInfo";
+    ASSERT_TRUE(ExecutionFailed);
+    ASSERT_FALSE(Error.empty());
+  }
+
+}
+
 } // end anonymous namespace