[Support] Fix handle and memory leak for processes that are not waited for

Execute's Data parameter is now optional, so we won't allocate memory
for it on Windows and we'll close the process handle.

The Unix code should probably do something similar to avoid accumulation
of zombie children that haven't been waited on.

Tested on Linux and Windows.

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

diff --git a/lib/Support/Program.cpp b/lib/Support/Program.cpp
index 6e04a1cd536ddaedc272b6e9e9e8b5feb1e3876d..b1a9b98755cac1b881ed8e8c6fd5fbe741184433 100644 (file)
--- a/lib/Support/Program.cpp
+++ b/lib/Support/Program.cpp
@@ -22,7 +22,7 @@ using namespace sys;
 //===          independent code.
 //===----------------------------------------------------------------------===//
 
-static bool Execute(void *&Data, const Path &path, const char **args,
+static bool Execute(void **Data, const Path &path, const char **args,
                     const char **env, const sys::Path **redirects,
                     unsigned memoryLimit, std::string *ErrMsg);
 
@@ -34,7 +34,7 @@ int sys::ExecuteAndWait(const Path &path, const char **args, const char **envp,
                         unsigned memoryLimit, std::string *ErrMsg,
                         bool *ExecutionFailed) {
   void *Data = 0;
-  if (Execute(Data, path, args, envp, redirects, memoryLimit, ErrMsg)) {
+  if (Execute(&Data, path, args, envp, redirects, memoryLimit, ErrMsg)) {
     if (ExecutionFailed) *ExecutionFailed = false;
     return Wait(Data, path, secondsToWait, ErrMsg);
   }
@@ -45,8 +45,7 @@ int sys::ExecuteAndWait(const Path &path, const char **args, const char **envp,
 void sys::ExecuteNoWait(const Path &path, const char **args, const char **envp,
                         const Path **redirects, unsigned memoryLimit,
                         std::string *ErrMsg) {
-  void *Data = 0;
-  Execute(Data, path, args, envp, redirects, memoryLimit, ErrMsg);
+  Execute(/*Data*/ 0, path, args, envp, redirects, memoryLimit, ErrMsg);
 }
 
 // Include the platform-specific parts of this class.

diff --git a/lib/Support/Unix/Program.inc b/lib/Support/Unix/Program.inc
index 0d6543a8a8c87d596830c861331c67252333a844..d8bdd6ce4247d56d19ed1ef1d8e261bf6193823e 100644 (file)
--- a/lib/Support/Unix/Program.inc
+++ b/lib/Support/Unix/Program.inc
@@ -178,7 +178,7 @@ static void SetMemoryLimits (unsigned size)
 
 }
 
-static bool Execute(void *&Data, const Path &path, const char **args,
+static bool Execute(void **Data, const Path &path, const char **args,
                     const char **envp, const Path **redirects,
                     unsigned memoryLimit, std::string *ErrMsg) {
   // If this OS has posix_spawn and there is no memory limit being implied, use
@@ -228,7 +228,8 @@ static bool Execute(void *&Data, const Path &path, const char **args,
     if (Err)
      return !MakeErrMsg(ErrMsg, "posix_spawn failed", Err);
 
-    Data = reinterpret_cast<void*>(PID);
+    if (Data)
+      *Data = reinterpret_cast<void*>(PID);
     return true;
   }
 #endif
@@ -290,7 +291,8 @@ static bool Execute(void *&Data, const Path &path, const char **args,
       break;
   }
 
-  Data = reinterpret_cast<void*>(child);
+  if (Data)
+    *Data = reinterpret_cast<void*>(child);
 
   return true;
 }
@@ -299,11 +301,7 @@ static int Wait(void *&Data, const sys::Path &path, unsigned secondsToWait,
                 std::string *ErrMsg) {
 #ifdef HAVE_SYS_WAIT_H
   struct sigaction Act, Old;
-
-  if (Data == 0) {
-    MakeErrMsg(ErrMsg, "Process not started!");
-    return -1;
-  }
+  assert(Data && "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,

diff --git a/lib/Support/Windows/Program.inc b/lib/Support/Windows/Program.inc
index 0e8bbc80a44fa46b93c40990ec2fe51ce5f09f0e..90a5cdb78ffd01f835433e9ab217934208d8c95d 100644 (file)
--- a/lib/Support/Windows/Program.inc
+++ b/lib/Support/Windows/Program.inc
@@ -170,20 +170,13 @@ static unsigned int ArgLenWithQuotes(const char *Str) {
 
 }
 
-static bool Execute(void *&Data,
+static bool Execute(void **Data,
                     const Path& path,
                     const char** args,
                     const char** envp,
                     const Path** redirects,
                     unsigned memoryLimit,
                     std::string* ErrMsg) {
-  if (Data) {
-    Win32ProcessInfo* wpi = reinterpret_cast<Win32ProcessInfo*>(Data);
-    CloseHandle(wpi->hProcess);
-    delete wpi;
-    Data = 0;
-  }
-
   if (!path.canExecute()) {
     if (ErrMsg)
       *ErrMsg = "program not executable";
@@ -321,10 +314,12 @@ static bool Execute(void *&Data,
                path.str() + "'");
     return false;
   }
-  Win32ProcessInfo* wpi = new Win32ProcessInfo;
-  wpi->hProcess = pi.hProcess;
-  wpi->dwProcessId = pi.dwProcessId;
-  Data = wpi;
+  if (Data) {
+    Win32ProcessInfo* wpi = new Win32ProcessInfo;
+    wpi->hProcess = pi.hProcess;
+    wpi->dwProcessId = pi.dwProcessId;
+    *Data = wpi;
+  }
 
   // Make sure these get closed no matter what.
   ScopedCommonHandle hThread(pi.hThread);
@@ -354,21 +349,17 @@ 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(void *&Data, const Path &path,
-                   unsigned secondsToWait,
-                   std::string* ErrMsg) {
-  if (Data == 0) {
-    MakeErrMsg(ErrMsg, "Process not started!");
-    return -1;
-  }
-
-  Win32ProcessInfo* wpi = reinterpret_cast<Win32ProcessInfo*>(Data);
-  HANDLE hProcess = wpi->hProcess;
-
+static int WaitAux(Win32ProcessInfo *wpi, const Path &path,
+                   unsigned secondsToWait, std::string *ErrMsg) {
   // Wait for the process to terminate.
+  HANDLE hProcess = wpi->hProcess;
   DWORD millisecondsToWait = INFINITE;
   if (secondsToWait > 0)
     millisecondsToWait = secondsToWait * 1000;
@@ -407,12 +398,11 @@ static int WaitAux(void *&Data, const Path &path,
   return 1;
 }
 
-static int Wait(void *&Data, const Path &path,
-                unsigned secondsToWait,
-                std::string* ErrMsg) {
-  int Ret = WaitAux(Data, path, secondsToWait, ErrMsg);
+static int Wait(void *&Data, const Path &path, unsigned secondsToWait,
+                std::string *ErrMsg) {
+  Win32ProcessInfo *wpi = reinterpret_cast<Win32ProcessInfo *>(Data);
+  int Ret = WaitAux(wpi, path, secondsToWait, ErrMsg);
 
-  Win32ProcessInfo* wpi = reinterpret_cast<Win32ProcessInfo*>(Data);
   CloseHandle(wpi->hProcess);
   delete wpi;
   Data = 0;