[llvm] r183906 - [Support] Fix handle and memory leak for processes that are not waited for
Reid Kleckner
reid at kleckner.net
Thu Jun 13 08:27:17 PDT 2013
Author: rnk
Date: Thu Jun 13 10:27:17 2013
New Revision: 183906
URL: http://llvm.org/viewvc/llvm-project?rev=183906&view=rev
Log:
[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.
Modified:
llvm/trunk/lib/Support/Program.cpp
llvm/trunk/lib/Support/Unix/Program.inc
llvm/trunk/lib/Support/Windows/Program.inc
Modified: llvm/trunk/lib/Support/Program.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Program.cpp?rev=183906&r1=183905&r2=183906&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Program.cpp (original)
+++ llvm/trunk/lib/Support/Program.cpp Thu Jun 13 10:27:17 2013
@@ -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
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
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.
Modified: llvm/trunk/lib/Support/Unix/Program.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Unix/Program.inc?rev=183906&r1=183905&r2=183906&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Unix/Program.inc (original)
+++ llvm/trunk/lib/Support/Unix/Program.inc Thu Jun 13 10:27:17 2013
@@ -178,7 +178,7 @@ static void SetMemoryLimits (unsigned si
}
-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 P
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 P
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::
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,
Modified: llvm/trunk/lib/Support/Windows/Program.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Program.inc?rev=183906&r1=183905&r2=183906&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Windows/Program.inc (original)
+++ llvm/trunk/lib/Support/Windows/Program.inc Thu Jun 13 10:27:17 2013
@@ -170,20 +170,13 @@ static unsigned int ArgLenWithQuotes(con
}
-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 Pa
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;
More information about the llvm-commits
mailing list