From 2c204767da6c8f80355803e03c2f8b9a5e64aa61 Mon Sep 17 00:00:00 2001 From: "Tareq A. Siraj" Date: Tue, 1 Oct 2013 14:28:18 +0000 Subject: [PATCH] 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 llvm-svn: 191763 --- include/llvm/Support/Program.h | 62 ++++++++++++++-- lib/Support/Program.cpp | 36 ++++++---- lib/Support/Unix/Program.inc | 116 +++++++++++++++++------------- lib/Support/Windows/Program.inc | 111 +++++++++++++--------------- unittests/Support/ProgramTest.cpp | 103 ++++++++++++++++++++++++++ 5 files changed, 298 insertions(+), 130 deletions(-) diff --git a/include/llvm/Support/Program.h b/include/llvm/Support/Program.h index 513435187f4..91781f9a5fb 100644 --- a/include/llvm/Support/Program.h +++ b/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 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 diff --git a/lib/Support/Program.cpp b/lib/Support/Program.cpp index 79f7e5fbb7e..83f2ec4f0fc 100644 --- a/lib/Support/Program.cpp +++ b/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. diff --git a/lib/Support/Unix/Program.inc b/lib/Support/Unix/Program.inc index a93a9120eaa..2822e912c81 100644 --- a/lib/Support/Unix/Program.inc +++ b/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(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(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(Data); - pid_t child = static_cast(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 Args) { } return true; } - } diff --git a/lib/Support/Windows/Program.inc b/lib/Support/Windows/Program.inc index 8165ef41115..28690852b0d 100644 --- a/lib/Support/Windows/Program.inc +++ b/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(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(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 Args) { } return true; } - } diff --git a/unittests/Support/ProgramTest.cpp b/unittests/Support/ProgramTest.cpp index a57f60e5e9e..6eb990f29d2 100644 --- a/unittests/Support/ProgramTest.cpp +++ b/unittests/Support/ProgramTest.cpp @@ -21,6 +21,20 @@ extern char **environ; #endif +#if defined(LLVM_ON_UNIX) +#include +void sleep_for(unsigned int seconds) { + sleep(seconds); +} +#elif defined(LLVM_ON_WIN32) +#include +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 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