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.

3 files changed, 66 insertions, 42 deletions

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>& params)
+LLProcessPtr LLProcess::create(const LLSDOrParams& params)
 {
 	try
 	{
@@ -55,8 +55,9 @@ LLProcessPtr LLProcess::create(const LLSDParamAdapter<Params>& params)
 	}
 }
 
-LLProcess::LLProcess(const LLSDParamAdapter<Params>& 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<LLJob>
 {
 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>& params)
+void LLProcess::launch(const LLSDOrParams& params)
 {
 	PROCESS_INFORMATION pinfo;
 	STARTUPINFOA sinfo = { sizeof(sinfo) };
@@ -201,28 +212,28 @@ void LLProcess::launch(const LLSDParamAdapter<Params>& 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>& 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>& params)
 
 	// parent process
 	mProcessID = child;
+	mProcessHandle = child;
 
 	mDesc = STRINGIZE(LLStringUtil::quote(params.executable) << " (" << mProcessID << ')');
 	LL_INFOS("LLProcess") << "Launched " << params << " (" << mProcessID << ")" << LL_ENDL;
diff --git a/indra/llcommon/llprocess.h b/indra/llcommon/llprocess.h
index 51c42582ea..8a842589ec 100644
--- a/indra/llcommon/llprocess.h
+++ b/indra/llcommon/llprocess.h
@@ -35,7 +35,7 @@
 
 #if LL_WINDOWS
 #define WIN32_LEAN_AND_MEAN
-#include <windows.h>
+#include <windows.h>                // HANDLE (eye roll)
 #endif
 
 class LLProcess;
@@ -79,6 +79,7 @@ public:
 		/// implicitly kill process on destruction of LLProcess object
 		Optional<bool> autokill;
 	};
+	typedef LLSDParamAdapter<Params> LLSDOrParams;
 
 	/**
 	 * Factory accepting either plain LLSD::Map or Params block.
@@ -91,7 +92,7 @@ public:
 	 * cwd		  (optional, string, dft no chdir): change to this directory before executing
 	 * autokill	  (optional, bool, dft true):		implicit kill() on ~LLProcess
 	 */
-	static LLProcessPtr create(const LLSDParamAdapter<Params>& params);
+	static LLProcessPtr create(const LLSDOrParams& params);
 	virtual ~LLProcess();
 
 	// isRunning isn't const because, if child isn't running, it clears stored
@@ -103,35 +104,46 @@ public:
 	bool kill(void);
 
 #if LL_WINDOWS
-	typedef HANDLE id;
+	typedef int id;                 ///< as returned by getProcessID()
+	typedef HANDLE handle;          ///< as returned by getProcessHandle()
 #else
-	typedef pid_t  id;
+	typedef pid_t id;
+	typedef pid_t handle;
 #endif
-	/// Get platform-specific process ID
-	id getProcessID() const { return mProcessID; };
+	/**
+	 * Get an int-like id value. This is primarily intended for a human reader
+	 * to differentiate processes.
+	 */
+	id getProcessID() const;
+	/**
+	 * Get a "handle" of a kind that you might pass to platform-specific API
+	 * functions to engage features not directly supported by LLProcess.
+	 */
+	handle getProcessHandle() const;
 
 	/**
-	 * Test if a process (id obtained from getProcessID()) is still
-	 * running. Return is same nonzero id value if still running, else
+	 * Test if a process (@c handle obtained from getProcessHandle()) is still
+	 * running. Return same nonzero @c handle value if still running, else
 	 * zero, so you can test it like a bool. But if you want to update a
 	 * stored variable as a side effect, you can write code like this:
 	 * @code
-	 * childpid = LLProcess::isRunning(childpid);
+	 * hchild = LLProcess::isRunning(hchild);
 	 * @endcode
 	 * @note This method is intended as a unit-test hook, not as the first of
-	 * a whole set of operations supported on freestanding @c id values. New
-	 * functionality should be added as nonstatic members operating on
-	 * mProcessID.
+	 * a whole set of operations supported on freestanding @c handle values.
+	 * New functionality should be added as nonstatic members operating on
+	 * the same data as getProcessHandle().
 	 */
-	static id isRunning(id, const std::string& desc="");
+	static handle isRunning(handle, const std::string& desc="");
 
 private:
 	/// constructor is private: use create() instead
-	LLProcess(const LLSDParamAdapter<Params>& params);
-	void launch(const LLSDParamAdapter<Params>& params);
+	LLProcess(const LLSDOrParams& params);
+	void launch(const LLSDOrParams& params);
 
 	std::string mDesc;
 	id mProcessID;
+	handle mProcessHandle;
 	bool mAutokill;
 };
 
diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp
index 405540e436..4ad45bdf27 100644
--- a/indra/llcommon/tests/llprocess_test.cpp
+++ b/indra/llcommon/tests/llprocess_test.cpp
@@ -599,7 +599,7 @@ namespace tut
     {
         set_test_name("implicit kill()");
         NamedTempFile out("out", "not started");
-        LLProcess::id pid(0);
+        LLProcess::handle phandle(0);
         {
             PythonProcessLauncher py("kill()",
                                      "from __future__ import with_statement\n"
@@ -614,8 +614,8 @@ namespace tut
             py.mParams.args.add(out.getName());
             py.mPy = LLProcess::create(py.mParams);
             ensure("couldn't launch kill() script", py.mPy);
-            // Capture id for later
-            pid = py.mPy->getProcessID();
+            // Capture handle for later
+            phandle = py.mPy->getProcessHandle();
             // Wait for the script to wake up and do its first write
             int i = 0, timeout = 60;
             for ( ; i < timeout; ++i)
@@ -630,7 +630,7 @@ namespace tut
             // Destroy the LLProcess, which should kill the child.
         }
         // wait for the script to terminate... one way or another.
-        while (LLProcess::isRunning(pid))
+        while (LLProcess::isRunning(phandle))
         {
             sleep(1);
         }
@@ -646,7 +646,7 @@ namespace tut
         set_test_name("autokill");
         NamedTempFile from("from", "not started");
         NamedTempFile to("to", "");
-        LLProcess::id pid(0);
+        LLProcess::handle phandle(0);
         {
             PythonProcessLauncher py("autokill",
                                      "from __future__ import with_statement\n"
@@ -672,8 +672,8 @@ namespace tut
             py.mParams.autokill = false;
             py.mPy = LLProcess::create(py.mParams);
             ensure("couldn't launch kill() script", py.mPy);
-            // Capture id for later
-            pid = py.mPy->getProcessID();
+            // Capture handle for later
+            phandle = py.mPy->getProcessHandle();
             // Wait for the script to wake up and do its first write
             int i = 0, timeout = 60;
             for ( ; i < timeout; ++i)
@@ -695,7 +695,7 @@ namespace tut
             outf << "go";
         } // flush and close.
         // now wait for the script to terminate... one way or another.
-        while (LLProcess::isRunning(pid))
+        while (LLProcess::isRunning(phandle))
         {
             sleep(1);
         }