From f0dbb878337082d3f581874c12e6df2f4659a464 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 20 Jan 2012 18:10:40 -0500 Subject: Per Richard, replace LLProcessLauncher with LLProcess. LLProcessLauncher had the somewhat fuzzy mandate of (1) accumulating parameters with which to launch a child process and (2) sometimes tracking the lifespan of the ensuing child process. But a valid LLProcessLauncher object might or might not have ever been associated with an actual child process. LLProcess specifically tracks a child process. In effect, it's a fairly thin wrapper around a process HANDLE (on Windows) or pid_t (elsewhere), with lifespan management thrown in. A static LLProcess::create() method launches a new child; create() accepts an LLSD bundle with child parameters. So building up a parameter bundle is deferred to LLSD rather than conflated with the process management object. Reconcile all known LLProcessLauncher consumers in the viewer code base, notably the class unit tests. --- indra/llcommon/llprocess.cpp | 338 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 338 insertions(+) create mode 100644 indra/llcommon/llprocess.cpp (limited to 'indra/llcommon/llprocess.cpp') diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp new file mode 100644 index 0000000000..8c0caca680 --- /dev/null +++ b/indra/llcommon/llprocess.cpp @@ -0,0 +1,338 @@ +/** + * @file llprocess.cpp + * @brief Utility class for launching, terminating, and tracking the state of processes. + * + * $LicenseInfo:firstyear=2008&license=viewerlgpl$ + * Second Life Viewer Source Code + * Copyright (C) 2010, Linden Research, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; + * version 2.1 of the License only. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + * Linden Research, Inc., 945 Battery Street, San Francisco, CA 94111 USA + * $/LicenseInfo$ + */ + +#include "linden_common.h" +#include "llprocess.h" +#include "llsd.h" +#include "llsdserialize.h" +#include "stringize.h" + +#include +#include +#include + +/// Need an exception to avoid constructing an invalid LLProcess object, but +/// internal use only +struct LLProcessError: public std::runtime_error +{ + LLProcessError(const std::string& msg): std::runtime_error(msg) {} +}; + +LLProcessPtr LLProcess::create(const LLSD& params) +{ + try + { + return LLProcessPtr(new LLProcess(params)); + } + catch (const LLProcessError& e) + { + LL_WARNS("LLProcess") << e.what() << LL_ENDL; + return LLProcessPtr(); + } +} + +LLProcess::LLProcess(const LLSD& params): + mProcessID(0), + mAutokill(params["autokill"].asBoolean()) +{ + // nonstandard default bool value + if (! params.has("autokill")) + mAutokill = true; + if (! params.has("executable")) + { + throw LLProcessError(STRINGIZE("not launched: missing 'executable'\n" + << LLSDNotationStreamer(params))); + } + + launch(params); +} + +LLProcess::~LLProcess() +{ + if (mAutokill) + { + kill(); + } +} + +bool LLProcess::isRunning(void) +{ + mProcessID = isRunning(mProcessID); + return (mProcessID != 0); +} + +#if LL_WINDOWS + +static std::string quote(const std::string& str) +{ + std::string::size_type len(str.length()); + // If the string is already quoted, assume user knows what s/he's doing. + if (len >= 2 && str[0] == '"' && str[len-1] == '"') + { + return str; + } + + // Not already quoted: do it. + std::string result("\""); + for (std::string::const_iterator ci(str.begin()), cend(str.end()); ci != cend; ++ci) + { + if (*ci == '"') + { + result.append("\\"); + } + result.push_back(*ci); + } + return result + "\""; +} + +void LLProcess::launch(const LLSD& params) +{ + PROCESS_INFORMATION pinfo; + STARTUPINFOA sinfo; + memset(&sinfo, 0, sizeof(sinfo)); + + std::string args = quote(params["executable"]); + BOOST_FOREACH(const std::string& arg, llsd::inArray(params["args"])) + { + args += " "; + args += quote(arg); + } + + // So retarded. Windows requires that the second parameter to + // CreateProcessA be a writable (non-const) string... + std::vector args2(args.begin(), args.end()); + args2.push_back('\0'); + + // Convert wrapper to a real std::string so we can use c_str(); but use a + // named variable instead of a temporary so c_str() pointer remains valid. + std::string cwd(params["cwd"]); + const char * working_directory = 0; + if (! cwd.empty()) + working_directory = cwd.c_str(); + if( ! CreateProcessA( NULL, &args2[0], NULL, NULL, FALSE, 0, NULL, working_directory, &sinfo, &pinfo ) ) + { + int result = GetLastError(); + + LPTSTR error_str = 0; + if( + FormatMessage( FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM, + NULL, + result, + 0, + (LPTSTR)&error_str, + 0, + NULL) + != 0) + { + char message[256]; + wcstombs(message, error_str, sizeof(message)); + message[sizeof(message)-1] = 0; + LocalFree(error_str); + throw LLProcessError(STRINGIZE("CreateProcessA failed (" << result << "): " + << message)); + } + throw LLProcessError(STRINGIZE("CreateProcessA failed (" << result + << "), but FormatMessage() did not explain")); + } + + // foo = pinfo.dwProcessId; // get your pid here if you want to use it later on + // CloseHandle(pinfo.hProcess); // stops leaks - nothing else + mProcessID = pinfo.hProcess; + CloseHandle(pinfo.hThread); // stops leaks - nothing else +} + +LLProcess::id LLProcess::isRunning(id handle) +{ + if (! handle) + return 0; + + DWORD waitresult = WaitForSingleObject(handle, 0); + if(waitresult == WAIT_OBJECT_0) + { + // the process has completed. + return 0; + } + + return handle; +} + +bool LLProcess::kill(void) +{ + if (! mProcessID) + return false; + + TerminateProcess(mProcessID, 0); + return ! isRunning(); +} + +#else // Mac and linux + +#include +#include +#include +#include + +// Attempt to reap a process ID -- returns true if the process has exited and been reaped, false otherwise. +static bool reap_pid(pid_t pid) +{ + pid_t wait_result = ::waitpid(pid, NULL, WNOHANG); + if (wait_result == pid) + { + return true; + } + if (wait_result == -1 && errno == ECHILD) + { + // No such process -- this may mean we're ignoring SIGCHILD. + return true; + } + + return false; +} + +void LLProcess::launch(const LLSD& params) +{ + // flush all buffers before the child inherits them + ::fflush(NULL); + + pid_t child = vfork(); + if (child == 0) + { + // child process + + std::string cwd(params["cwd"]); + if (! cwd.empty()) + { + // change to the desired child working directory + if (::chdir(cwd.c_str())) + { + // chdir failed + LL_WARNS("LLProcess") << "could not chdir(\"" << cwd << "\")" << LL_ENDL; + // pointless to throw; this is child process... + _exit(248); + } + } + + // create an argv vector for the child process + std::vector fake_argv; + + // add the executable path + std::string executable(params["executable"]); + fake_argv.push_back(executable.c_str()); + + // and any arguments + const LLSD& params_args(params["args"]); + std::vector args(params_args.beginArray(), params_args.endArray()); + BOOST_FOREACH(const std::string& arg, args) + { + fake_argv.push_back(arg.c_str()); + } + + // terminate with a null pointer + fake_argv.push_back(NULL); + + ::execv(executable.c_str(), const_cast(&fake_argv[0])); + + // If we reach this point, the exec failed. + LL_WARNS("LLProcess") << "failed to launch: "; + BOOST_FOREACH(const char* arg, fake_argv) + { + LL_CONT << arg << ' '; + } + LL_CONT << LL_ENDL; + // Use _exit() instead of exit() per the vfork man page. Exit with a + // distinctive rc: someday soon we'll be able to retrieve it, and it + // would be nice to be able to tell that the child process failed! + _exit(249); + } + + // parent process + mProcessID = child; +} + +LLProcess::id LLProcess::isRunning(id pid) +{ + if (! pid) + return 0; + + // Check whether the process has exited, and reap it if it has. + if(reap_pid(pid)) + { + // the process has exited. + return 0; + } + + return pid; +} + +bool LLProcess::kill(void) +{ + if (! mProcessID) + return false; + + // Try to kill the process. We'll do approximately the same thing whether + // the kill returns an error or not, so we ignore the result. + (void)::kill(mProcessID, SIGTERM); + + // This will have the side-effect of reaping the zombie if the process has exited. + return ! isRunning(); +} + +/*==========================================================================*| +static std::list sZombies; + +void LLProcess::orphan(void) +{ + // Disassociate the process from this object + if(mProcessID != 0) + { + // We may still need to reap the process's zombie eventually + sZombies.push_back(mProcessID); + + mProcessID = 0; + } +} + +// static +void LLProcess::reap(void) +{ + // Attempt to real all saved process ID's. + + std::list::iterator iter = sZombies.begin(); + while(iter != sZombies.end()) + { + if(reap_pid(*iter)) + { + iter = sZombies.erase(iter); + } + else + { + iter++; + } + } +} +|*==========================================================================*/ + +#endif -- cgit v1.2.3 From 6e214960ce203d1d50d7bd6bd04eedee3afd0fa3 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 20 Jan 2012 20:19:50 -0500 Subject: Define LLProcess::Params; accept create(const LLSDParamAdapter&). This allows callers to pass either LLSD formatted as before -- which all callers still do -- or an actual LLProcess::Params block. --- indra/llcommon/llprocess.cpp | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) (limited to 'indra/llcommon/llprocess.cpp') diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp index 8c0caca680..dfb2ed69e9 100644 --- a/indra/llcommon/llprocess.cpp +++ b/indra/llcommon/llprocess.cpp @@ -26,7 +26,6 @@ #include "linden_common.h" #include "llprocess.h" -#include "llsd.h" #include "llsdserialize.h" #include "stringize.h" @@ -41,7 +40,7 @@ struct LLProcessError: public std::runtime_error LLProcessError(const std::string& msg): std::runtime_error(msg) {} }; -LLProcessPtr LLProcess::create(const LLSD& params) +LLProcessPtr LLProcess::create(const LLSDParamAdapter& params) { try { @@ -54,16 +53,13 @@ LLProcessPtr LLProcess::create(const LLSD& params) } } -LLProcess::LLProcess(const LLSD& params): +LLProcess::LLProcess(const LLSDParamAdapter& params): mProcessID(0), - mAutokill(params["autokill"].asBoolean()) + mAutokill(params.autokill) { - // nonstandard default bool value - if (! params.has("autokill")) - mAutokill = true; - if (! params.has("executable")) + if (! params.validateBlock(true)) { - throw LLProcessError(STRINGIZE("not launched: missing 'executable'\n" + throw LLProcessError(STRINGIZE("not launched: failed parameter validation\n" << LLSDNotationStreamer(params))); } @@ -108,14 +104,14 @@ static std::string quote(const std::string& str) return result + "\""; } -void LLProcess::launch(const LLSD& params) +void LLProcess::launch(const LLSDParamAdapter& params) { PROCESS_INFORMATION pinfo; STARTUPINFOA sinfo; memset(&sinfo, 0, sizeof(sinfo)); - std::string args = quote(params["executable"]); - BOOST_FOREACH(const std::string& arg, llsd::inArray(params["args"])) + std::string args = quote(params.executable); + BOOST_FOREACH(const std::string& arg, params.args) { args += " "; args += quote(arg); @@ -128,7 +124,7 @@ void LLProcess::launch(const LLSD& params) // Convert wrapper to a real std::string so we can use c_str(); but use a // named variable instead of a temporary so c_str() pointer remains valid. - std::string cwd(params["cwd"]); + std::string cwd(params.cwd); const char * working_directory = 0; if (! cwd.empty()) working_directory = cwd.c_str(); @@ -212,7 +208,7 @@ static bool reap_pid(pid_t pid) return false; } -void LLProcess::launch(const LLSD& params) +void LLProcess::launch(const LLSDParamAdapter& params) { // flush all buffers before the child inherits them ::fflush(NULL); @@ -222,7 +218,7 @@ void LLProcess::launch(const LLSD& params) { // child process - std::string cwd(params["cwd"]); + std::string cwd(params.cwd); if (! cwd.empty()) { // change to the desired child working directory @@ -239,12 +235,11 @@ void LLProcess::launch(const LLSD& params) std::vector fake_argv; // add the executable path - std::string executable(params["executable"]); + std::string executable(params.executable); fake_argv.push_back(executable.c_str()); // and any arguments - const LLSD& params_args(params["args"]); - std::vector args(params_args.beginArray(), params_args.endArray()); + std::vector args(params.args.begin(), params.args.end()); BOOST_FOREACH(const std::string& arg, args) { fake_argv.push_back(arg.c_str()); -- cgit v1.2.3 From b9a03b95aafb07eb32a8f99a671f2216acce96d4 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sun, 22 Jan 2012 09:16:11 -0500 Subject: On Windows, introduce viewer Job Object and assign children to it. The idea is that, with the right flag settings, this will cause the OS to terminate remaining viewer child processes when the viewer terminates -- whether or not it terminates intentionally. Of course, if LLProcess's caller specifies autokill=false, e.g. to run the viewer updater, that asserts that we WANT the child to persist beyond the viewer session itself. --- indra/llcommon/llprocess.cpp | 167 +++++++++++++++++++++++++++++++++---------- 1 file changed, 128 insertions(+), 39 deletions(-) (limited to 'indra/llcommon/llprocess.cpp') diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp index dfb2ed69e9..d30d87411d 100644 --- a/indra/llcommon/llprocess.cpp +++ b/indra/llcommon/llprocess.cpp @@ -27,6 +27,7 @@ #include "linden_common.h" #include "llprocess.h" #include "llsdserialize.h" +#include "llsingleton.h" #include "stringize.h" #include @@ -80,43 +81,84 @@ bool LLProcess::isRunning(void) return (mProcessID != 0); } +/***************************************************************************** +* Windows specific +*****************************************************************************/ #if LL_WINDOWS -static std::string quote(const std::string& str) +static std::string WindowsErrorString(const std::string& operation); +static std::string quote(const std::string&); + +/** + * Wrap a Windows Job Object for use in managing child-process lifespan. + * + * On Windows, we use a Job Object to constrain the lifespan of any + * autokill=true child process to the viewer's own lifespan: + * http://stackoverflow.com/questions/53208/how-do-i-automatically-destroy-child-processes-in-windows + * (thanks Richard!). + * + * We manage it using an LLSingleton for a couple of reasons: + * + * # Lazy initialization: if some viewer session never launches a child + * process, we should never have to create a Job Object. + * # Cross-DLL support: be wary of C++ statics when multiple DLLs are + * involved. + */ +class LLJob: public LLSingleton { - std::string::size_type len(str.length()); - // If the string is already quoted, assume user knows what s/he's doing. - if (len >= 2 && str[0] == '"' && str[len-1] == '"') +public: + void assignProcess(const std::string& prog, HANDLE hProcess) { - return str; + // If we never managed to initialize this Job Object, can't use it -- + // but don't keep spamming the log, we already emitted warnings when + // we first tried to create. + if (! mJob) + return; + + if (! AssignProcessToJobObject(mJob, hProcess)) + { + LL_WARNS("LLProcess") << WindowsErrorString(STRINGIZE("AssignProcessToJobObject(\"" + << prog << "\")")) << LL_ENDL; + } } - // Not already quoted: do it. - std::string result("\""); - for (std::string::const_iterator ci(str.begin()), cend(str.end()); ci != cend; ++ci) +private: + LLJob(): + mJob(0) { - if (*ci == '"') + mJob = CreateJobObject(NULL, NULL); + if (! mJob) { - result.append("\\"); + LL_WARNS("LLProcess") << WindowsErrorString("CreateJobObject()") << LL_ENDL; + return; + } + + JOBOBJECT_EXTENDED_LIMIT_INFORMATION jeli = { 0 }; + + // Configure all child processes associated with this new job object + // to terminate when the calling process (us!) terminates. + jeli.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; + if (! SetInformationJobObject(mJob, JobObjectExtendedLimitInformation, &jeli, sizeof(jeli))) + { + LL_WARNS("LLProcess") << WindowsErrorString("SetInformationJobObject()") << LL_ENDL; } - result.push_back(*ci); } - return result + "\""; -} + + HANDLE mJob; +}; void LLProcess::launch(const LLSDParamAdapter& params) { PROCESS_INFORMATION pinfo; - STARTUPINFOA sinfo; - memset(&sinfo, 0, sizeof(sinfo)); - + STARTUPINFOA sinfo = { sizeof(sinfo) }; + std::string args = quote(params.executable); BOOST_FOREACH(const std::string& arg, params.args) { args += " "; args += quote(arg); } - + // So retarded. Windows requires that the second parameter to // CreateProcessA be a writable (non-const) string... std::vector args2(args.begin(), args.end()); @@ -130,28 +172,14 @@ void LLProcess::launch(const LLSDParamAdapter& params) working_directory = cwd.c_str(); if( ! CreateProcessA( NULL, &args2[0], NULL, NULL, FALSE, 0, NULL, working_directory, &sinfo, &pinfo ) ) { - int result = GetLastError(); - - LPTSTR error_str = 0; - if( - FormatMessage( FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM, - NULL, - result, - 0, - (LPTSTR)&error_str, - 0, - NULL) - != 0) - { - char message[256]; - wcstombs(message, error_str, sizeof(message)); - message[sizeof(message)-1] = 0; - LocalFree(error_str); - throw LLProcessError(STRINGIZE("CreateProcessA failed (" << result << "): " - << message)); - } - throw LLProcessError(STRINGIZE("CreateProcessA failed (" << result - << "), but FormatMessage() did not explain")); + throw LLProcessError(WindowsErrorString("CreateProcessA")); + } + + // Now associate the new child process with our Job Object -- unless + // autokill is false, i.e. caller asserts the child should persist. + if (params.autokill) + { + LLJob::instance().assignProcess(params.executable, pinfo.hProcess); } // foo = pinfo.dwProcessId; // get your pid here if you want to use it later on @@ -184,6 +212,67 @@ bool LLProcess::kill(void) return ! isRunning(); } +/** + * Double-quote an argument string, unless it's already double-quoted. If we + * quote it, escape any embedded double-quote with backslash. + * + * LLProcess::create()'s caller passes a Unix-style array of strings for + * command-line arguments. Our caller can and should expect that these will be + * passed to the child process as individual arguments, regardless of content + * (e.g. embedded spaces). But because Windows invokes any child process with + * a single command-line string, this means we must quote each argument behind + * the scenes. + */ +static std::string quote(const std::string& str) +{ + std::string::size_type len(str.length()); + // If the string is already quoted, assume user knows what s/he's doing. + if (len >= 2 && str[0] == '"' && str[len-1] == '"') + { + return str; + } + + // Not already quoted: do it. + std::string result("\""); + for (std::string::const_iterator ci(str.begin()), cend(str.end()); ci != cend; ++ci) + { + if (*ci == '"') + { + result.append("\\"); + } + result.push_back(*ci); + } + return result + "\""; +} + +/// GetLastError()/FormatMessage() boilerplate +static std::string WindowsErrorString(const std::string& operation) +{ + int result = GetLastError(); + + LPTSTR error_str = 0; + if (FormatMessage( FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM, + NULL, + result, + 0, + (LPTSTR)&error_str, + 0, + NULL) + != 0) + { + char message[256]; + wcstombs(message, error_str, sizeof(message)); + message[sizeof(message)-1] = 0; + LocalFree(error_str); + return STRINGIZE(operation << " failed (" << result << "): " << message); + } + return STRINGIZE(operation << " failed (" << result + << "), but FormatMessage() did not explain"); +} + +/***************************************************************************** +* Non-Windows specific +*****************************************************************************/ #else // Mac and linux #include -- cgit v1.2.3 From aa1bbe3277842a9a6e7db5227b35f1fbea50b7a6 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sun, 22 Jan 2012 10:58:16 -0500 Subject: Make LLProcess::Params streamable; use that in LLExternalEditor. --- indra/llcommon/llprocess.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'indra/llcommon/llprocess.cpp') diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp index d30d87411d..9d6c19f1dd 100644 --- a/indra/llcommon/llprocess.cpp +++ b/indra/llcommon/llprocess.cpp @@ -81,6 +81,21 @@ bool LLProcess::isRunning(void) return (mProcessID != 0); } +std::ostream& operator<<(std::ostream& out, const LLProcess::Params& params) +{ + std::string cwd(params.cwd); + if (! cwd.empty()) + { + out << "cd '" << cwd << "': "; + } + out << '"' << std::string(params.executable) << '"'; + BOOST_FOREACH(const std::string& arg, params.args) + { + out << " \"" << arg << '"'; + } + return out; +} + /***************************************************************************** * Windows specific *****************************************************************************/ -- cgit v1.2.3 From 748d1b311fdecf123df40bd7d22dd7e19afaca84 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sun, 22 Jan 2012 11:56:38 -0500 Subject: Add LLProcess logging on launch(), kill(), isRunning(). Much as I dislike viewer log spam, seems to me starting a child process, killing it and observing its termination are noteworthy events. New logging makes LLExternalEditor launch message redundant; removed. --- indra/llcommon/llprocess.cpp | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) (limited to 'indra/llcommon/llprocess.cpp') diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp index 9d6c19f1dd..6d329a3fa1 100644 --- a/indra/llcommon/llprocess.cpp +++ b/indra/llcommon/llprocess.cpp @@ -77,7 +77,7 @@ LLProcess::~LLProcess() bool LLProcess::isRunning(void) { - mProcessID = isRunning(mProcessID); + mProcessID = isRunning(mProcessID, mDesc); return (mProcessID != 0); } @@ -190,20 +190,23 @@ void LLProcess::launch(const LLSDParamAdapter& params) throw LLProcessError(WindowsErrorString("CreateProcessA")); } + // foo = pinfo.dwProcessId; // get your pid here if you want to use it later on + // CloseHandle(pinfo.hProcess); // stops leaks - nothing else + mProcessID = pinfo.hProcess; + CloseHandle(pinfo.hThread); // stops leaks - nothing else + + mDesc = STRINGIZE('"' << std::string(params.executable) << "\" (" << pinfo.dwProcessId << ')'); + LL_INFOS("LLProcess") << "Launched " << params << " (" << pinfo.dwProcessId << ")" << LL_ENDL; + // Now associate the new child process with our Job Object -- unless // autokill is false, i.e. caller asserts the child should persist. if (params.autokill) { - LLJob::instance().assignProcess(params.executable, pinfo.hProcess); + LLJob::instance().assignProcess(mDesc, mProcessID); } - - // foo = pinfo.dwProcessId; // get your pid here if you want to use it later on - // CloseHandle(pinfo.hProcess); // stops leaks - nothing else - mProcessID = pinfo.hProcess; - CloseHandle(pinfo.hThread); // stops leaks - nothing else } -LLProcess::id LLProcess::isRunning(id handle) +LLProcess::id LLProcess::isRunning(id handle, const std::string& desc) { if (! handle) return 0; @@ -212,6 +215,10 @@ LLProcess::id LLProcess::isRunning(id handle) if(waitresult == WAIT_OBJECT_0) { // the process has completed. + if (! desc.empty()) + { + LL_INFOS("LLProcess") << desc << " terminated" << LL_ENDL; + } return 0; } @@ -223,6 +230,7 @@ bool LLProcess::kill(void) if (! mProcessID) return false; + LL_INFOS("LLProcess") << "killing " << mDesc << LL_ENDL; TerminateProcess(mProcessID, 0); return ! isRunning(); } @@ -369,9 +377,12 @@ void LLProcess::launch(const LLSDParamAdapter& params) // parent process mProcessID = child; + + mDesc = STRINGIZE('"' << std::string(params.executable) << "\" (" << mProcessID << ')'); + LL_INFOS("LLProcess") << "Launched " << params << " (" << mProcessID << ")" << LL_ENDL; } -LLProcess::id LLProcess::isRunning(id pid) +LLProcess::id LLProcess::isRunning(id pid, const std::string& desc) { if (! pid) return 0; @@ -380,6 +391,10 @@ LLProcess::id LLProcess::isRunning(id pid) if(reap_pid(pid)) { // the process has exited. + if (! desc.empty()) + { + LL_INFOS("LLProcess") << desc << " terminated" << LL_ENDL; + } return 0; } @@ -393,6 +408,7 @@ bool LLProcess::kill(void) // Try to kill the process. We'll do approximately the same thing whether // the kill returns an error or not, so we ignore the result. + LL_INFOS("LLProcess") << "killing " << mDesc << LL_ENDL; (void)::kill(mProcessID, SIGTERM); // This will have the side-effect of reaping the zombie if the process has exited. -- cgit v1.2.3 From 738483e6302af5a9ad00fa3df17efe5336a03a41 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sun, 22 Jan 2012 13:05:34 -0500 Subject: Every singleton needs a friend... --- indra/llcommon/llprocess.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'indra/llcommon/llprocess.cpp') diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp index 6d329a3fa1..eb7ce4129b 100644 --- a/indra/llcommon/llprocess.cpp +++ b/indra/llcommon/llprocess.cpp @@ -138,6 +138,7 @@ public: } private: + friend class LLSingleton; LLJob(): mJob(0) { -- cgit v1.2.3 From 507e136f9a25179992b2093e10ade1093cc71698 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 23 Jan 2012 16:24:33 -0500 Subject: Per Richard: close unusable Job Object; move quote() to LLStringUtil. If LLProcess can't set the right flag on a Windows Job Object, the object isn't useful to us, so we might as well discard it. quote() is sufficiently general that it belongs in LLStringUtil instead of buried as a static helper function in llprocess.cpp. --- indra/llcommon/llprocess.cpp | 49 ++++++++++++-------------------------------- 1 file changed, 13 insertions(+), 36 deletions(-) (limited to 'indra/llcommon/llprocess.cpp') diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp index eb7ce4129b..2c7512419d 100644 --- a/indra/llcommon/llprocess.cpp +++ b/indra/llcommon/llprocess.cpp @@ -28,6 +28,7 @@ #include "llprocess.h" #include "llsdserialize.h" #include "llsingleton.h" +#include "llstring.h" #include "stringize.h" #include @@ -102,7 +103,6 @@ std::ostream& operator<<(std::ostream& out, const LLProcess::Params& params) #if LL_WINDOWS static std::string WindowsErrorString(const std::string& operation); -static std::string quote(const std::string&); /** * Wrap a Windows Job Object for use in managing child-process lifespan. @@ -157,6 +157,10 @@ private: if (! SetInformationJobObject(mJob, JobObjectExtendedLimitInformation, &jeli, sizeof(jeli))) { LL_WARNS("LLProcess") << WindowsErrorString("SetInformationJobObject()") << LL_ENDL; + // This Job Object is useless to us + CloseHandle(mJob); + // prevent assignProcess() from trying to use it + mJob = 0; } } @@ -168,11 +172,17 @@ void LLProcess::launch(const LLSDParamAdapter& params) PROCESS_INFORMATION pinfo; STARTUPINFOA sinfo = { sizeof(sinfo) }; - std::string args = quote(params.executable); + // LLProcess::create()'s caller passes a Unix-style array of strings for + // command-line arguments. Our caller can and should expect that these will be + // passed to the child process as individual arguments, regardless of content + // (e.g. embedded spaces). But because Windows invokes any child process with + // a single command-line string, this means we must quote each argument behind + // the scenes. + std::string args = LLStringUtil::quote(params.executable); BOOST_FOREACH(const std::string& arg, params.args) { args += " "; - args += quote(arg); + args += LLStringUtil::quote(arg); } // So retarded. Windows requires that the second parameter to @@ -236,39 +246,6 @@ bool LLProcess::kill(void) return ! isRunning(); } -/** - * Double-quote an argument string, unless it's already double-quoted. If we - * quote it, escape any embedded double-quote with backslash. - * - * LLProcess::create()'s caller passes a Unix-style array of strings for - * command-line arguments. Our caller can and should expect that these will be - * passed to the child process as individual arguments, regardless of content - * (e.g. embedded spaces). But because Windows invokes any child process with - * a single command-line string, this means we must quote each argument behind - * the scenes. - */ -static std::string quote(const std::string& str) -{ - std::string::size_type len(str.length()); - // If the string is already quoted, assume user knows what s/he's doing. - if (len >= 2 && str[0] == '"' && str[len-1] == '"') - { - return str; - } - - // Not already quoted: do it. - std::string result("\""); - for (std::string::const_iterator ci(str.begin()), cend(str.end()); ci != cend; ++ci) - { - if (*ci == '"') - { - result.append("\\"); - } - result.push_back(*ci); - } - return result + "\""; -} - /// GetLastError()/FormatMessage() boilerplate static std::string WindowsErrorString(const std::string& operation) { -- cgit v1.2.3 From 27df0a84564d3a886661aae0faae74c2157cd31b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 27 Jan 2012 23:46:00 -0500 Subject: On Windows, only quote LLProcess arguments if they seem to need it. On Posix platforms, the OS argument mechanism makes quoting/reparsing unnecessary anyway, so this only affects Windows. Add optional 'triggers' parameter to LLStringUtils::quote() (default: space and double-quote). Only if the passed string contains a character in 'triggers' will it be double-quoted. This is observed to fix a Windows-specific problem in which plugin child process would fail to start because it wasn't expecting a quoted number. Use LLStringUtils::quote() more consistently in LLProcess implementation for logging. --- indra/llcommon/llprocess.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'indra/llcommon/llprocess.cpp') diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp index 2c7512419d..2b7a534fb3 100644 --- a/indra/llcommon/llprocess.cpp +++ b/indra/llcommon/llprocess.cpp @@ -87,12 +87,12 @@ std::ostream& operator<<(std::ostream& out, const LLProcess::Params& params) std::string cwd(params.cwd); if (! cwd.empty()) { - out << "cd '" << cwd << "': "; + out << "cd " << LLStringUtil::quote(cwd) << ": "; } - out << '"' << std::string(params.executable) << '"'; + out << LLStringUtil::quote(params.executable); BOOST_FOREACH(const std::string& arg, params.args) { - out << " \"" << arg << '"'; + out << ' ' << LLStringUtil::quote(arg); } return out; } @@ -132,8 +132,8 @@ public: if (! AssignProcessToJobObject(mJob, hProcess)) { - LL_WARNS("LLProcess") << WindowsErrorString(STRINGIZE("AssignProcessToJobObject(\"" - << prog << "\")")) << LL_ENDL; + LL_WARNS("LLProcess") << WindowsErrorString(STRINGIZE("AssignProcessToJobObject(" + << prog << ")")) << LL_ENDL; } } @@ -206,7 +206,7 @@ void LLProcess::launch(const LLSDParamAdapter& params) mProcessID = pinfo.hProcess; CloseHandle(pinfo.hThread); // stops leaks - nothing else - mDesc = STRINGIZE('"' << std::string(params.executable) << "\" (" << pinfo.dwProcessId << ')'); + mDesc = STRINGIZE(LLStringUtil::quote(params.executable) << " (" << pinfo.dwProcessId << ')'); LL_INFOS("LLProcess") << "Launched " << params << " (" << pinfo.dwProcessId << ")" << LL_ENDL; // Now associate the new child process with our Job Object -- unless @@ -356,7 +356,7 @@ void LLProcess::launch(const LLSDParamAdapter& params) // parent process mProcessID = child; - mDesc = STRINGIZE('"' << std::string(params.executable) << "\" (" << mProcessID << ')'); + mDesc = STRINGIZE(LLStringUtil::quote(params.executable) << " (" << mProcessID << ')'); LL_INFOS("LLProcess") << "Launched " << params << " (" << mProcessID << ")" << LL_ENDL; } -- cgit v1.2.3 From 803acbc5efde19c0acacfc7fe4990841dbf31a3e Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 30 Jan 2012 10:14:10 -0500 Subject: Trim trailing "\r\n" from Windows FormatMessage() string for logging. --- indra/llcommon/llprocess.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'indra/llcommon/llprocess.cpp') diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp index 2b7a534fb3..8c0e8fe65e 100644 --- a/indra/llcommon/llprocess.cpp +++ b/indra/llcommon/llprocess.cpp @@ -261,11 +261,15 @@ static std::string WindowsErrorString(const std::string& operation) NULL) != 0) { + // convert from wide-char string to multi-byte string char message[256]; wcstombs(message, error_str, sizeof(message)); message[sizeof(message)-1] = 0; LocalFree(error_str); - return STRINGIZE(operation << " failed (" << result << "): " << message); + // convert to std::string to trim trailing whitespace + std::string mbsstr(message); + mbsstr.erase(mbsstr.find_last_not_of(" \t\r\n")); + return STRINGIZE(operation << " failed (" << result << "): " << mbsstr); } return STRINGIZE(operation << " failed (" << result << "), but FormatMessage() did not explain"); -- cgit v1.2.3 From 85581eefa63d8f8e8c5132c4cd7e137f6cb88869 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 30 Jan 2012 12:11:44 -0500 Subject: Expose 'handle' as well as 'id' on LLProcess objects. On Posix, these and the corresponding getProcessID()/getProcessHandle() accessors produce the same pid_t value; but on Windows, it's useful to distinguish an int-like 'id' useful to human log readers versus an opaque 'handle' for passing to platform-specific API functions. So make the distinction in a platform-independent way. --- indra/llcommon/llprocess.cpp | 50 +++++++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 19 deletions(-) (limited to 'indra/llcommon/llprocess.cpp') diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp index 8c0e8fe65e..a7bafb8cb0 100644 --- a/indra/llcommon/llprocess.cpp +++ b/indra/llcommon/llprocess.cpp @@ -42,7 +42,7 @@ struct LLProcessError: public std::runtime_error LLProcessError(const std::string& msg): std::runtime_error(msg) {} }; -LLProcessPtr LLProcess::create(const LLSDParamAdapter& params) +LLProcessPtr LLProcess::create(const LLSDOrParams& params) { try { @@ -55,8 +55,9 @@ LLProcessPtr LLProcess::create(const LLSDParamAdapter& params) } } -LLProcess::LLProcess(const LLSDParamAdapter& params): +LLProcess::LLProcess(const LLSDOrParams& params): mProcessID(0), + mProcessHandle(0), mAutokill(params.autokill) { if (! params.validateBlock(true)) @@ -78,8 +79,18 @@ LLProcess::~LLProcess() bool LLProcess::isRunning(void) { - mProcessID = isRunning(mProcessID, mDesc); - return (mProcessID != 0); + mProcessHandle = isRunning(mProcessHandle, mDesc); + return (mProcessHandle != 0); +} + +LLProcess::id LLProcess::getProcessID() const +{ + return mProcessID; +} + +LLProcess::handle LLProcess::getProcessHandle() const +{ + return mProcessHandle; } std::ostream& operator<<(std::ostream& out, const LLProcess::Params& params) @@ -122,7 +133,7 @@ static std::string WindowsErrorString(const std::string& operation); class LLJob: public LLSingleton { public: - void assignProcess(const std::string& prog, HANDLE hProcess) + void assignProcess(const std::string& prog, handle hProcess) { // If we never managed to initialize this Job Object, can't use it -- // but don't keep spamming the log, we already emitted warnings when @@ -164,10 +175,10 @@ private: } } - HANDLE mJob; + handle mJob; }; -void LLProcess::launch(const LLSDParamAdapter& params) +void LLProcess::launch(const LLSDOrParams& params) { PROCESS_INFORMATION pinfo; STARTUPINFOA sinfo = { sizeof(sinfo) }; @@ -201,28 +212,28 @@ void LLProcess::launch(const LLSDParamAdapter& params) throw LLProcessError(WindowsErrorString("CreateProcessA")); } - // foo = pinfo.dwProcessId; // get your pid here if you want to use it later on // CloseHandle(pinfo.hProcess); // stops leaks - nothing else - mProcessID = pinfo.hProcess; + mProcessID = pinfo.dwProcessId; + mProcessHandle = pinfo.hProcess; CloseHandle(pinfo.hThread); // stops leaks - nothing else - mDesc = STRINGIZE(LLStringUtil::quote(params.executable) << " (" << pinfo.dwProcessId << ')'); - LL_INFOS("LLProcess") << "Launched " << params << " (" << pinfo.dwProcessId << ")" << LL_ENDL; + mDesc = STRINGIZE(LLStringUtil::quote(params.executable) << " (" << mProcessID << ')'); + LL_INFOS("LLProcess") << "Launched " << params << " (" << mProcessID << ")" << LL_ENDL; // Now associate the new child process with our Job Object -- unless // autokill is false, i.e. caller asserts the child should persist. if (params.autokill) { - LLJob::instance().assignProcess(mDesc, mProcessID); + LLJob::instance().assignProcess(mDesc, mProcessHandle); } } -LLProcess::id LLProcess::isRunning(id handle, const std::string& desc) +LLProcess::handle LLProcess::isRunning(handle h, const std::string& desc) { - if (! handle) + if (! h) return 0; - DWORD waitresult = WaitForSingleObject(handle, 0); + DWORD waitresult = WaitForSingleObject(h, 0); if(waitresult == WAIT_OBJECT_0) { // the process has completed. @@ -233,16 +244,16 @@ LLProcess::id LLProcess::isRunning(id handle, const std::string& desc) return 0; } - return handle; + return h; } bool LLProcess::kill(void) { - if (! mProcessID) + if (! mProcessHandle) return false; LL_INFOS("LLProcess") << "killing " << mDesc << LL_ENDL; - TerminateProcess(mProcessID, 0); + TerminateProcess(mProcessHandle, 0); return ! isRunning(); } @@ -302,7 +313,7 @@ static bool reap_pid(pid_t pid) return false; } -void LLProcess::launch(const LLSDParamAdapter& params) +void LLProcess::launch(const LLSDOrParams& params) { // flush all buffers before the child inherits them ::fflush(NULL); @@ -359,6 +370,7 @@ void LLProcess::launch(const LLSDParamAdapter& params) // parent process mProcessID = child; + mProcessHandle = child; mDesc = STRINGIZE(LLStringUtil::quote(params.executable) << " (" << mProcessID << ')'); LL_INFOS("LLProcess") << "Launched " << params << " (" << mProcessID << ")" << LL_ENDL; -- cgit v1.2.3 From 60a777d2e3f9bec7a20e5de2df7cb3ecd2455b98 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 30 Jan 2012 12:32:05 -0500 Subject: LLProcess::handle must be qualified when used in LLJob class. --- indra/llcommon/llprocess.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llcommon/llprocess.cpp') diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp index a7bafb8cb0..1e27f8ce1d 100644 --- a/indra/llcommon/llprocess.cpp +++ b/indra/llcommon/llprocess.cpp @@ -133,7 +133,7 @@ static std::string WindowsErrorString(const std::string& operation); class LLJob: public LLSingleton { public: - void assignProcess(const std::string& prog, handle hProcess) + void assignProcess(const std::string& prog, LLProcess::handle hProcess) { // If we never managed to initialize this Job Object, can't use it -- // but don't keep spamming the log, we already emitted warnings when @@ -175,7 +175,7 @@ private: } } - handle mJob; + LLProcess::handle mJob; }; void LLProcess::launch(const LLSDOrParams& params) -- cgit v1.2.3 From 491cd825561be1cf4a6f428a535811cbe0e3f179 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 30 Jan 2012 17:47:57 -0500 Subject: Set bit flag on CreateProcess() to allow AssignProcessToJobObject(). Windows 7 and friends tend to create a process already implicitly allocated to a job object, and a process can only belong to a single job object. Passing CREATE_BREAKAWAY_FROM_JOB in CreateProcessA()'s dwCreationFlags seems to bypass the access-denied error observed with AssignProcessToJobObject() otherwise. This change should (!) enable OS lifespan management for SLVoice.exe et al. --- indra/llcommon/llprocess.cpp | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) (limited to 'indra/llcommon/llprocess.cpp') diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp index 1e27f8ce1d..8611d67f25 100644 --- a/indra/llcommon/llprocess.cpp +++ b/indra/llcommon/llprocess.cpp @@ -207,7 +207,26 @@ void LLProcess::launch(const LLSDOrParams& params) const char * working_directory = 0; if (! cwd.empty()) working_directory = cwd.c_str(); - if( ! CreateProcessA( NULL, &args2[0], NULL, NULL, FALSE, 0, NULL, working_directory, &sinfo, &pinfo ) ) + + // It's important to pass CREATE_BREAKAWAY_FROM_JOB because Windows 7 et + // al. tend to implicitly launch new processes already bound to a job. From + // http://msdn.microsoft.com/en-us/library/windows/desktop/ms681949%28v=vs.85%29.aspx : + // "The process must not already be assigned to a job; if it is, the + // function fails with ERROR_ACCESS_DENIED." ... + // "If the process is being monitored by the Program Compatibility + // Assistant (PCA), it is placed into a compatibility job. Therefore, the + // process must be created using CREATE_BREAKAWAY_FROM_JOB before it can + // be placed in another job." + if( ! CreateProcessA(NULL, // lpApplicationName + &args2[0], // lpCommandLine + NULL, // lpProcessAttributes + NULL, // lpThreadAttributes + FALSE, // bInheritHandles + CREATE_BREAKAWAY_FROM_JOB, // dwCreationFlags + NULL, // lpEnvironment + working_directory, // lpCurrentDirectory + &sinfo, // lpStartupInfo + &pinfo ) ) // lpProcessInformation { throw LLProcessError(WindowsErrorString("CreateProcessA")); } @@ -225,7 +244,7 @@ void LLProcess::launch(const LLSDOrParams& params) if (params.autokill) { LLJob::instance().assignProcess(mDesc, mProcessHandle); - } +} } LLProcess::handle LLProcess::isRunning(handle h, const std::string& desc) @@ -287,7 +306,7 @@ static std::string WindowsErrorString(const std::string& operation) } /***************************************************************************** -* Non-Windows specific +* Posix specific *****************************************************************************/ #else // Mac and linux @@ -444,4 +463,4 @@ void LLProcess::reap(void) } |*==========================================================================*/ -#endif +#endif // Posix -- cgit v1.2.3 From aafb03b29f5166e8978931ad8b717be32d942836 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 7 Feb 2012 10:53:23 -0500 Subject: Convert LLProcess implementation from platform-specific to using APR. Include logic to engage Linden apr_procattr_autokill_set() extension: on Windows, magic CreateProcess() flag must be pushed down into apr_proc_create() level. When using an APR package without that extension, present implementation should lock (e.g.) SLVoice.exe lifespan to viewer's on Windows XP but probably won't on Windows 7: need magic flag on CreateProcess(). Using APR child-termination callback requires us to define state (e.g. LLProcess::RUNNING). Take the opportunity to present Status, capturing state and (if terminated) rc or signal number; but since most of the time all caller really wants is to log the outcome, also present status string, encapsulating logic to examine state and describe exited-with-rc vs. killed-by-signal. New Status logic may report clearer results in the case of a Windows child process killed by exception. Clarify that static LLProcess::isRunning(handle) overload is only for use when the original LLProcess object has been destroyed: really only for unit tests. We necessarily retain our original platform-specific implementations for just that one method. (Nonstatic isRunning() no longer calls static method.) Clarify log output from llprocess_test.cpp in a couple places. --- indra/llcommon/llprocess.cpp | 552 +++++++++++++++++++++++++++++++------------ 1 file changed, 395 insertions(+), 157 deletions(-) (limited to 'indra/llcommon/llprocess.cpp') diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp index 8611d67f25..bc27002701 100644 --- a/indra/llcommon/llprocess.cpp +++ b/indra/llcommon/llprocess.cpp @@ -30,11 +30,15 @@ #include "llsingleton.h" #include "llstring.h" #include "stringize.h" +#include "llapr.h" #include #include #include +static std::string empty; +static LLProcess::Status interpret_status(int status); + /// Need an exception to avoid constructing an invalid LLProcess object, but /// internal use only struct LLProcessError: public std::runtime_error @@ -55,9 +59,14 @@ LLProcessPtr LLProcess::create(const LLSDOrParams& params) } } +/// Call an apr function returning apr_status_t. On failure, log warning and +/// throw LLProcessError mentioning the function call that produced that +/// result. +#define chkapr(func) \ + if (ll_apr_warn_status(func)) \ + throw LLProcessError(#func " failed") + LLProcess::LLProcess(const LLSDOrParams& params): - mProcessID(0), - mProcessHandle(0), mAutokill(params.autokill) { if (! params.validateBlock(true)) @@ -66,31 +75,298 @@ LLProcess::LLProcess(const LLSDOrParams& params): << LLSDNotationStreamer(params))); } - launch(params); + apr_procattr_t *procattr = NULL; + chkapr(apr_procattr_create(&procattr, gAPRPoolp)); + + // For which of stdin, stdout, stderr should we create a pipe to the + // child? In the viewer, there are only a couple viable + // apr_procattr_io_set() alternatives: inherit the viewer's own stdxxx + // handle (APR_NO_PIPE, e.g. for stdout, stderr), or create a pipe that's + // blocking on the child end but nonblocking at the viewer end + // (APR_CHILD_BLOCK). The viewer can't block for anything: the parent end + // MUST be nonblocking. As the APR documentation itself points out, it + // makes very little sense to set nonblocking I/O for the child end of a + // pipe: only a specially-written child could deal with that. + // Other major options could include explicitly creating a single APR pipe + // and passing it as both stdout and stderr (apr_procattr_child_out_set(), + // apr_procattr_child_err_set()), or accepting a filename, opening it and + // passing that apr_file_t (simple <, >, 2> redirect emulation). +// chkapr(apr_procattr_io_set(procattr, APR_CHILD_BLOCK, APR_CHILD_BLOCK, APR_CHILD_BLOCK)); + chkapr(apr_procattr_io_set(procattr, APR_NO_PIPE, APR_NO_PIPE, APR_NO_PIPE)); + + // Thumbs down on implicitly invoking the shell to invoke the child. From + // our point of view, the other major alternative to APR_PROGRAM_PATH + // would be APR_PROGRAM_ENV: still copy environment, but require full + // executable pathname. I don't see a downside to searching the PATH, + // though: if our caller wants (e.g.) a specific Python interpreter, s/he + // can still pass the full pathname. + chkapr(apr_procattr_cmdtype_set(procattr, APR_PROGRAM_PATH)); + // YES, do extra work if necessary to report child exec() failures back to + // parent process. + chkapr(apr_procattr_error_check_set(procattr, 1)); + // Do not start a non-autokill child in detached state. On Posix + // platforms, this setting attempts to daemonize the new child, closing + // std handles and the like, and that's a bit more detachment than we + // want. autokill=false just means not to implicitly kill the child when + // the parent terminates! +// chkapr(apr_procattr_detach_set(procattr, params.autokill? 0 : 1)); + + if (params.autokill) + { +#if defined(APR_HAS_PROCATTR_AUTOKILL_SET) + apr_status_t ok = apr_procattr_autokill_set(procattr, 1); +# if LL_WINDOWS + // As of 2012-02-02, we only expect this to be implemented on Windows. + // Avoid spamming the log with warnings we fully expect. + ll_apr_warn_status(ok); +# endif // LL_WINDOWS +#else + LL_WARNS("LLProcess") << "This version of APR lacks Linden apr_procattr_autokill_set() extension" << LL_ENDL; +#endif + } + + // Have to instantiate named std::strings for string params items so their + // c_str() values persist. + std::string cwd(params.cwd); + if (! cwd.empty()) + { + chkapr(apr_procattr_dir_set(procattr, cwd.c_str())); + } + + // create an argv vector for the child process + std::vector argv; + + // add the executable path + std::string executable(params.executable); + argv.push_back(executable.c_str()); + + // and any arguments + std::vector args(params.args.begin(), params.args.end()); + BOOST_FOREACH(const std::string& arg, args) + { + argv.push_back(arg.c_str()); + } + + // terminate with a null pointer + argv.push_back(NULL); + + // Launch! The NULL would be the environment block, if we were passing one. + chkapr(apr_proc_create(&mProcess, argv[0], &argv[0], NULL, procattr, gAPRPoolp)); + + // arrange to call status_callback() + apr_proc_other_child_register(&mProcess, &LLProcess::status_callback, this, mProcess.in, + gAPRPoolp); + mStatus.mState = RUNNING; + + mDesc = STRINGIZE(LLStringUtil::quote(params.executable) << " (" << mProcess.pid << ')'); + LL_INFOS("LLProcess") << "Launched " << params << " (" << mProcess.pid << ")" << LL_ENDL; + + // Unless caller explicitly turned off autokill (child should persist), + // take steps to terminate the child. This is all suspenders-and-belt: in + // theory our destructor should kill an autokill child, but in practice + // that doesn't always work (e.g. VWR-21538). + if (params.autokill) + { + // Tie the lifespan of this child process to the lifespan of our APR + // pool: on destruction of the pool, forcibly kill the process. Tell + // APR to try SIGTERM and wait 3 seconds. If that didn't work, use + // SIGKILL. + apr_pool_note_subprocess(gAPRPoolp, &mProcess, APR_KILL_AFTER_TIMEOUT); + + // On Windows, associate the new child process with our Job Object. + autokill(); + } } LLProcess::~LLProcess() { + // Only in state RUNNING are we registered for callback. In UNSTARTED we + // haven't yet registered. And since receiving the callback is the only + // way we detect child termination, we only change from state RUNNING at + // the same time we unregister. + if (mStatus.mState == RUNNING) + { + // We're still registered for a callback: unregister. Do it before + // we even issue the kill(): even if kill() somehow prompted an + // instantaneous callback (unlikely), this object is going away! Any + // information updated in this object by such a callback is no longer + // available to any consumer anyway. + apr_proc_other_child_unregister(this); + } + if (mAutokill) { - kill(); + kill("destructor"); + } +} + +bool LLProcess::kill(const std::string& who) +{ + if (isRunning()) + { + LL_INFOS("LLProcess") << who << " killing " << mDesc << LL_ENDL; + +#if LL_WINDOWS + int sig = -1; +#else // Posix + int sig = SIGTERM; +#endif + + ll_apr_warn_status(apr_proc_kill(&mProcess, sig)); } + + return ! isRunning(); } bool LLProcess::isRunning(void) { - mProcessHandle = isRunning(mProcessHandle, mDesc); - return (mProcessHandle != 0); + return getStatus().mState == RUNNING; +} + +LLProcess::Status LLProcess::getStatus() +{ + // Only when mState is RUNNING might the status change dynamically. For + // any other value, pointless to attempt to update status: it won't + // change. + if (mStatus.mState == RUNNING) + { + // Tell APR to sense whether the child is still running and call + // handle_status() appropriately. We should be able to get the same + // info from an apr_proc_wait(APR_NOWAIT) call; but at least in APR + // 1.4.2, testing suggests that even with APR_NOWAIT, apr_proc_wait() + // blocks the caller. We can't have that in the viewer. Hence the + // callback rigmarole. Once we update APR, it's probably worth testing + // again. Also -- although there's an apr_proc_other_child_refresh() + // call, i.e. get that information for one specific child, it accepts + // an 'apr_other_child_rec_t*' that's mentioned NOWHERE else in the + // documentation or header files! I would use the specific call if I + // knew how. As it is, each call to this method will call callbacks + // for ALL still-running child processes. Sigh... + apr_proc_other_child_refresh_all(APR_OC_REASON_RUNNING); + } + + return mStatus; +} + +std::string LLProcess::getStatusString() +{ + return getStatusString(getStatus()); +} + +std::string LLProcess::getStatusString(const Status& status) +{ + return getStatusString(mDesc, status); +} + +//static +std::string LLProcess::getStatusString(const std::string& desc, const Status& status) +{ + if (status.mState == UNSTARTED) + return desc + " was never launched"; + + if (status.mState == RUNNING) + return desc + " running"; + + if (status.mState == EXITED) + return STRINGIZE(desc << " exited with code " << status.mData); + + if (status.mState == KILLED) +#if LL_WINDOWS + return STRINGIZE(desc << " killed with exception " << std::hex << status.mData); +#else + return STRINGIZE(desc << " killed by signal " << status.mData); +#endif + + + return STRINGIZE(desc << " in unknown state " << status.mState << " (" << status.mData << ")"); +} + +// Classic-C-style APR callback +void LLProcess::status_callback(int reason, void* data, int status) +{ + // Our only role is to bounce this static method call back into object + // space. + static_cast(data)->handle_status(reason, status); +} + +#define tabent(symbol) { symbol, #symbol } +static struct ReasonCode +{ + int code; + const char* name; +} reasons[] = +{ + tabent(APR_OC_REASON_DEATH), + tabent(APR_OC_REASON_UNWRITABLE), + tabent(APR_OC_REASON_RESTART), + tabent(APR_OC_REASON_UNREGISTER), + tabent(APR_OC_REASON_LOST), + tabent(APR_OC_REASON_RUNNING) +}; +#undef tabent + +// Object-oriented callback +void LLProcess::handle_status(int reason, int status) +{ + { + // This odd appearance of LL_DEBUGS is just to bracket a lookup that will + // only be performed if in fact we're going to produce the log message. + LL_DEBUGS("LLProcess") << empty; + std::string reason_str; + BOOST_FOREACH(const ReasonCode& rcp, reasons) + { + if (reason == rcp.code) + { + reason_str = rcp.name; + break; + } + } + if (reason_str.empty()) + { + reason_str = STRINGIZE("unknown reason " << reason); + } + LL_CONT << mDesc << ": handle_status(" << reason_str << ", " << status << ")" << LL_ENDL; + } + + if (! (reason == APR_OC_REASON_DEATH || reason == APR_OC_REASON_LOST)) + { + // We're only interested in the call when the child terminates. + return; + } + + // Somewhat oddly, APR requires that you explicitly unregister even when + // it already knows the child has terminated. We must pass the same 'data' + // pointer as for the register() call, which was our 'this'. + apr_proc_other_child_unregister(this); + // We overload mStatus.mState to indicate whether the child is registered + // for APR callback: only RUNNING means registered. Track that we've + // unregistered. We know the child has terminated; might be EXITED or + // KILLED; refine below. + mStatus.mState = EXITED; + +// wi->rv = apr_proc_wait(wi->child, &wi->rc, &wi->why, APR_NOWAIT); + // It's just wrong to call apr_proc_wait() here. The only way APR knows to + // call us with APR_OC_REASON_DEATH is that it's already reaped this child + // process, so calling wait() will only produce "huh?" from the OS. We + // must rely on the status param passed in, which unfortunately comes + // straight from the OS wait() call, which means we have to decode it by + // hand. + mStatus = interpret_status(status); + LL_INFOS("LLProcess") << getStatusString() << LL_ENDL; } LLProcess::id LLProcess::getProcessID() const { - return mProcessID; + return mProcess.pid; } LLProcess::handle LLProcess::getProcessHandle() const { - return mProcessHandle; +#if LL_WINDOWS + return mProcess.hproc; +#else + return mProcess.pid; +#endif } std::ostream& operator<<(std::ostream& out, const LLProcess::Params& params) @@ -178,77 +454,15 @@ private: LLProcess::handle mJob; }; -void LLProcess::launch(const LLSDOrParams& params) +void LLProcess::autokill() { - PROCESS_INFORMATION pinfo; - STARTUPINFOA sinfo = { sizeof(sinfo) }; - - // LLProcess::create()'s caller passes a Unix-style array of strings for - // command-line arguments. Our caller can and should expect that these will be - // passed to the child process as individual arguments, regardless of content - // (e.g. embedded spaces). But because Windows invokes any child process with - // a single command-line string, this means we must quote each argument behind - // the scenes. - std::string args = LLStringUtil::quote(params.executable); - BOOST_FOREACH(const std::string& arg, params.args) - { - args += " "; - args += LLStringUtil::quote(arg); - } - - // So retarded. Windows requires that the second parameter to - // CreateProcessA be a writable (non-const) string... - std::vector args2(args.begin(), args.end()); - args2.push_back('\0'); - - // Convert wrapper to a real std::string so we can use c_str(); but use a - // named variable instead of a temporary so c_str() pointer remains valid. - std::string cwd(params.cwd); - const char * working_directory = 0; - if (! cwd.empty()) - working_directory = cwd.c_str(); - - // It's important to pass CREATE_BREAKAWAY_FROM_JOB because Windows 7 et - // al. tend to implicitly launch new processes already bound to a job. From - // http://msdn.microsoft.com/en-us/library/windows/desktop/ms681949%28v=vs.85%29.aspx : - // "The process must not already be assigned to a job; if it is, the - // function fails with ERROR_ACCESS_DENIED." ... - // "If the process is being monitored by the Program Compatibility - // Assistant (PCA), it is placed into a compatibility job. Therefore, the - // process must be created using CREATE_BREAKAWAY_FROM_JOB before it can - // be placed in another job." - if( ! CreateProcessA(NULL, // lpApplicationName - &args2[0], // lpCommandLine - NULL, // lpProcessAttributes - NULL, // lpThreadAttributes - FALSE, // bInheritHandles - CREATE_BREAKAWAY_FROM_JOB, // dwCreationFlags - NULL, // lpEnvironment - working_directory, // lpCurrentDirectory - &sinfo, // lpStartupInfo - &pinfo ) ) // lpProcessInformation - { - throw LLProcessError(WindowsErrorString("CreateProcessA")); - } - - // CloseHandle(pinfo.hProcess); // stops leaks - nothing else - mProcessID = pinfo.dwProcessId; - mProcessHandle = pinfo.hProcess; - CloseHandle(pinfo.hThread); // stops leaks - nothing else - - mDesc = STRINGIZE(LLStringUtil::quote(params.executable) << " (" << mProcessID << ')'); - LL_INFOS("LLProcess") << "Launched " << params << " (" << mProcessID << ")" << LL_ENDL; - - // Now associate the new child process with our Job Object -- unless - // autokill is false, i.e. caller asserts the child should persist. - if (params.autokill) - { - LLJob::instance().assignProcess(mDesc, mProcessHandle); -} + LLJob::instance().assignProcess(mDesc, mProcess.hproc); } LLProcess::handle LLProcess::isRunning(handle h, const std::string& desc) { + // This direct Windows implementation is because we have no access to the + // apr_proc_t struct: we expect it's been destroyed. if (! h) return 0; @@ -258,22 +472,44 @@ LLProcess::handle LLProcess::isRunning(handle h, const std::string& desc) // the process has completed. if (! desc.empty()) { - LL_INFOS("LLProcess") << desc << " terminated" << LL_ENDL; + DWORD status = 0; + if (! GetExitCodeProcess(h, &status)) + { + LL_WARNS("LLProcess") << desc << " terminated, but " + << WindowsErrorString("GetExitCodeProcess()") << LL_ENDL; + } + { + LL_INFOS("LLProcess") << getStatusString(desc, interpret_status(status)) + << LL_ENDL; + } } + CloseHandle(h); return 0; } return h; } -bool LLProcess::kill(void) +static LLProcess::Status interpret_status(int status) { - if (! mProcessHandle) - return false; + LLProcess::Status result; + + // This bit of code is cribbed from apr/threadproc/win32/proc.c, a + // function (unfortunately static) called why_from_exit_code(): + /* See WinNT.h STATUS_ACCESS_VIOLATION and family for how + * this class of failures was determined + */ + if ((status & 0xFFFF0000) == 0xC0000000) + { + result.mState = KILLED; + } + else + { + result.mState = EXITED; + } + result.mData = status; - LL_INFOS("LLProcess") << "killing " << mDesc << LL_ENDL; - TerminateProcess(mProcessHandle, 0); - return ! isRunning(); + return result; } /// GetLastError()/FormatMessage() boilerplate @@ -315,98 +551,91 @@ static std::string WindowsErrorString(const std::string& operation) #include #include +void LLProcess::autokill() +{ + // What we ought to do here is to: + // 1. create a unique process group and run all autokill children in that + // group (see https://jira.secondlife.com/browse/SWAT-563); + // 2. figure out a way to intercept control when the viewer exits -- + // gracefully or not; + // 3. when the viewer exits, kill off the aforementioned process group. + + // It's point 2 that's troublesome. Although I've seen some signal- + // handling logic in the Posix viewer code, I haven't yet found any bit of + // code that's run no matter how the viewer exits (a try/finally for the + // whole process, as it were). +} + // Attempt to reap a process ID -- returns true if the process has exited and been reaped, false otherwise. -static bool reap_pid(pid_t pid) +static bool reap_pid(pid_t pid, LLProcess::Status* pstatus=NULL) { - pid_t wait_result = ::waitpid(pid, NULL, WNOHANG); + LLProcess::Status dummy; + if (! pstatus) + { + // If caller doesn't want to see Status, give us a target anyway so we + // don't have to have a bunch of conditionals. + pstatus = &dummy; + } + + int status = 0; + pid_t wait_result = ::waitpid(pid, &status, WNOHANG); if (wait_result == pid) { + *pstatus = interpret_status(status); return true; } - if (wait_result == -1 && errno == ECHILD) + if (wait_result == 0) { - // No such process -- this may mean we're ignoring SIGCHILD. - return true; + pstatus->mState = LLProcess::RUNNING; + pstatus->mData = 0; + return false; } - - return false; -} -void LLProcess::launch(const LLSDOrParams& params) -{ - // flush all buffers before the child inherits them - ::fflush(NULL); + // Clear caller's Status block; caller must interpret UNSTARTED to mean + // "if this PID was ever valid, it no longer is." + *pstatus = LLProcess::Status(); - pid_t child = vfork(); - if (child == 0) + // We've dealt with the success cases: we were able to reap the child + // (wait_result == pid) or it's still running (wait_result == 0). It may + // be that the child terminated but didn't hang around long enough for us + // to reap. In that case we still have no Status to report, but we can at + // least state that it's not running. + if (wait_result == -1 && errno == ECHILD) { - // child process - - std::string cwd(params.cwd); - if (! cwd.empty()) - { - // change to the desired child working directory - if (::chdir(cwd.c_str())) - { - // chdir failed - LL_WARNS("LLProcess") << "could not chdir(\"" << cwd << "\")" << LL_ENDL; - // pointless to throw; this is child process... - _exit(248); - } - } - - // create an argv vector for the child process - std::vector fake_argv; - - // add the executable path - std::string executable(params.executable); - fake_argv.push_back(executable.c_str()); - - // and any arguments - std::vector args(params.args.begin(), params.args.end()); - BOOST_FOREACH(const std::string& arg, args) - { - fake_argv.push_back(arg.c_str()); - } - - // terminate with a null pointer - fake_argv.push_back(NULL); - - ::execv(executable.c_str(), const_cast(&fake_argv[0])); - - // If we reach this point, the exec failed. - LL_WARNS("LLProcess") << "failed to launch: "; - BOOST_FOREACH(const char* arg, fake_argv) - { - LL_CONT << arg << ' '; - } - LL_CONT << LL_ENDL; - // Use _exit() instead of exit() per the vfork man page. Exit with a - // distinctive rc: someday soon we'll be able to retrieve it, and it - // would be nice to be able to tell that the child process failed! - _exit(249); + // No such process -- this may mean we're ignoring SIGCHILD. + return true; } - // parent process - mProcessID = child; - mProcessHandle = child; - - mDesc = STRINGIZE(LLStringUtil::quote(params.executable) << " (" << mProcessID << ')'); - LL_INFOS("LLProcess") << "Launched " << params << " (" << mProcessID << ")" << LL_ENDL; + // Uh, should never happen?! + LL_WARNS("LLProcess") << "LLProcess::reap_pid(): waitpid(" << pid << ") returned " + << wait_result << "; not meaningful?" << LL_ENDL; + // If caller is looping until this pid terminates, and if we can't find + // out, better to break the loop than to claim it's still running. + return true; } LLProcess::id LLProcess::isRunning(id pid, const std::string& desc) { + // This direct Posix implementation is because we have no access to the + // apr_proc_t struct: we expect it's been destroyed. if (! pid) return 0; // Check whether the process has exited, and reap it if it has. - if(reap_pid(pid)) + LLProcess::Status status; + if(reap_pid(pid, &status)) { // the process has exited. if (! desc.empty()) { - LL_INFOS("LLProcess") << desc << " terminated" << LL_ENDL; + std::string statstr(desc + " apparently terminated: no status available"); + // We don't just pass UNSTARTED to getStatusString() because, in + // the context of reap_pid(), that state has special meaning. + if (status.mState != UNSTARTED) + { + statstr = getStatusString(desc, status); + } + LL_INFOS("LLProcess") << statstr << LL_ENDL; } return 0; } @@ -414,18 +643,27 @@ LLProcess::id LLProcess::isRunning(id pid, const std::string& desc) return pid; } -bool LLProcess::kill(void) +static LLProcess::Status interpret_status(int status) { - if (! mProcessID) - return false; + LLProcess::Status result; - // Try to kill the process. We'll do approximately the same thing whether - // the kill returns an error or not, so we ignore the result. - LL_INFOS("LLProcess") << "killing " << mDesc << LL_ENDL; - (void)::kill(mProcessID, SIGTERM); + if (WIFEXITED(status)) + { + result.mState = LLProcess::EXITED; + result.mData = WEXITSTATUS(status); + } + else if (WIFSIGNALED(status)) + { + result.mState = LLProcess::KILLED; + result.mData = WTERMSIG(status); + } + else // uh, shouldn't happen? + { + result.mState = LLProcess::EXITED; + result.mData = status; // someone else will have to decode + } - // This will have the side-effect of reaping the zombie if the process has exited. - return ! isRunning(); + return result; } /*==========================================================================*| -- cgit v1.2.3 From 219a010aaf4891fc2ab8480a2121b1244191c24d Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 7 Feb 2012 11:17:04 -0500 Subject: LLProcess::Status enum values need qualification in helper function. --- indra/llcommon/llprocess.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llcommon/llprocess.cpp') diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp index bc27002701..1305c1d764 100644 --- a/indra/llcommon/llprocess.cpp +++ b/indra/llcommon/llprocess.cpp @@ -501,11 +501,11 @@ static LLProcess::Status interpret_status(int status) */ if ((status & 0xFFFF0000) == 0xC0000000) { - result.mState = KILLED; + result.mState = LLProcess::KILLED; } else { - result.mState = EXITED; + result.mState = LLProcess::EXITED; } result.mData = status; -- cgit v1.2.3 From 90a1b67cc43792e1ca0047eaca51530aa14e134c Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 9 Feb 2012 15:22:24 -0500 Subject: Remove LLJob class: apr_procattr_autokill_set() should now handle. LLJob was vestigial code from before migrating Job Object support into APR. Also add APR signal-name string to getStatusString() output. --- indra/llcommon/llprocess.cpp | 70 +++----------------------------------------- 1 file changed, 4 insertions(+), 66 deletions(-) (limited to 'indra/llcommon/llprocess.cpp') diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp index 1305c1d764..7ccbdeed01 100644 --- a/indra/llcommon/llprocess.cpp +++ b/indra/llcommon/llprocess.cpp @@ -31,6 +31,7 @@ #include "llstring.h" #include "stringize.h" #include "llapr.h" +#include "apr_signal.h" #include #include @@ -274,10 +275,10 @@ std::string LLProcess::getStatusString(const std::string& desc, const Status& st #if LL_WINDOWS return STRINGIZE(desc << " killed with exception " << std::hex << status.mData); #else - return STRINGIZE(desc << " killed by signal " << status.mData); + return STRINGIZE(desc << " killed by signal " << status.mData + << " (" << apr_signal_description_get(status.mData) << ")"); #endif - return STRINGIZE(desc << " in unknown state " << status.mState << " (" << status.mData << ")"); } @@ -391,72 +392,9 @@ std::ostream& operator<<(std::ostream& out, const LLProcess::Params& params) static std::string WindowsErrorString(const std::string& operation); -/** - * Wrap a Windows Job Object for use in managing child-process lifespan. - * - * On Windows, we use a Job Object to constrain the lifespan of any - * autokill=true child process to the viewer's own lifespan: - * http://stackoverflow.com/questions/53208/how-do-i-automatically-destroy-child-processes-in-windows - * (thanks Richard!). - * - * We manage it using an LLSingleton for a couple of reasons: - * - * # Lazy initialization: if some viewer session never launches a child - * process, we should never have to create a Job Object. - * # Cross-DLL support: be wary of C++ statics when multiple DLLs are - * involved. - */ -class LLJob: public LLSingleton -{ -public: - void assignProcess(const std::string& prog, LLProcess::handle hProcess) - { - // If we never managed to initialize this Job Object, can't use it -- - // but don't keep spamming the log, we already emitted warnings when - // we first tried to create. - if (! mJob) - return; - - if (! AssignProcessToJobObject(mJob, hProcess)) - { - LL_WARNS("LLProcess") << WindowsErrorString(STRINGIZE("AssignProcessToJobObject(" - << prog << ")")) << LL_ENDL; - } - } - -private: - friend class LLSingleton; - LLJob(): - mJob(0) - { - mJob = CreateJobObject(NULL, NULL); - if (! mJob) - { - LL_WARNS("LLProcess") << WindowsErrorString("CreateJobObject()") << LL_ENDL; - return; - } - - JOBOBJECT_EXTENDED_LIMIT_INFORMATION jeli = { 0 }; - - // Configure all child processes associated with this new job object - // to terminate when the calling process (us!) terminates. - jeli.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; - if (! SetInformationJobObject(mJob, JobObjectExtendedLimitInformation, &jeli, sizeof(jeli))) - { - LL_WARNS("LLProcess") << WindowsErrorString("SetInformationJobObject()") << LL_ENDL; - // This Job Object is useless to us - CloseHandle(mJob); - // prevent assignProcess() from trying to use it - mJob = 0; - } - } - - LLProcess::handle mJob; -}; - void LLProcess::autokill() { - LLJob::instance().assignProcess(mDesc, mProcess.hproc); + // hopefully now handled by apr_procattr_autokill_set() } LLProcess::handle LLProcess::isRunning(handle h, const std::string& desc) -- cgit v1.2.3