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.

1 files changed, 128 insertions, 39 deletions

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 <boost/foreach.hpp>
@@ -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<LLJob>
 {
-	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>& 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<char> args2(args.begin(), args.end());
@@ -130,28 +172,14 @@ void LLProcess::launch(const LLSDParamAdapter<Params>& 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 <signal.h>