[llvm] r183906 - [Support] Fix handle and memory leak for processes that are not waited for
Rafael Espíndola
rafael.espindola at gmail.com
Thu Jun 13 09:02:17 PDT 2013
Thanks!
On 13 June 2013 11:27, Reid Kleckner <reid at kleckner.net> wrote:
> 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;
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list