Skip to content
Snippets Groups Projects
Commit 85581eef authored by Nat Goodspeed's avatar Nat Goodspeed
Browse files

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.
parent 803acbc5
No related branches found
No related tags found
No related merge requests found
...@@ -42,7 +42,7 @@ struct LLProcessError: public std::runtime_error ...@@ -42,7 +42,7 @@ struct LLProcessError: public std::runtime_error
LLProcessError(const std::string& msg): std::runtime_error(msg) {} LLProcessError(const std::string& msg): std::runtime_error(msg) {}
}; };
LLProcessPtr LLProcess::create(const LLSDParamAdapter<Params>& params) LLProcessPtr LLProcess::create(const LLSDOrParams& params)
{ {
try try
{ {
...@@ -55,8 +55,9 @@ LLProcessPtr LLProcess::create(const LLSDParamAdapter<Params>& params) ...@@ -55,8 +55,9 @@ LLProcessPtr LLProcess::create(const LLSDParamAdapter<Params>& params)
} }
} }
LLProcess::LLProcess(const LLSDParamAdapter<Params>& params): LLProcess::LLProcess(const LLSDOrParams& params):
mProcessID(0), mProcessID(0),
mProcessHandle(0),
mAutokill(params.autokill) mAutokill(params.autokill)
{ {
if (! params.validateBlock(true)) if (! params.validateBlock(true))
...@@ -78,8 +79,18 @@ LLProcess::~LLProcess() ...@@ -78,8 +79,18 @@ LLProcess::~LLProcess()
bool LLProcess::isRunning(void) bool LLProcess::isRunning(void)
{ {
mProcessID = isRunning(mProcessID, mDesc); mProcessHandle = isRunning(mProcessHandle, mDesc);
return (mProcessID != 0); 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) std::ostream& operator<<(std::ostream& out, const LLProcess::Params& params)
...@@ -122,7 +133,7 @@ static std::string WindowsErrorString(const std::string& operation); ...@@ -122,7 +133,7 @@ static std::string WindowsErrorString(const std::string& operation);
class LLJob: public LLSingleton<LLJob> class LLJob: public LLSingleton<LLJob>
{ {
public: 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 -- // 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 // but don't keep spamming the log, we already emitted warnings when
...@@ -164,10 +175,10 @@ class LLJob: public LLSingleton<LLJob> ...@@ -164,10 +175,10 @@ class LLJob: public LLSingleton<LLJob>
} }
} }
HANDLE mJob; handle mJob;
}; };
void LLProcess::launch(const LLSDParamAdapter<Params>& params) void LLProcess::launch(const LLSDOrParams& params)
{ {
PROCESS_INFORMATION pinfo; PROCESS_INFORMATION pinfo;
STARTUPINFOA sinfo = { sizeof(sinfo) }; STARTUPINFOA sinfo = { sizeof(sinfo) };
...@@ -201,28 +212,28 @@ void LLProcess::launch(const LLSDParamAdapter<Params>& params) ...@@ -201,28 +212,28 @@ void LLProcess::launch(const LLSDParamAdapter<Params>& params)
throw LLProcessError(WindowsErrorString("CreateProcessA")); 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 // CloseHandle(pinfo.hProcess); // stops leaks - nothing else
mProcessID = pinfo.hProcess; mProcessID = pinfo.dwProcessId;
mProcessHandle = pinfo.hProcess;
CloseHandle(pinfo.hThread); // stops leaks - nothing else CloseHandle(pinfo.hThread); // stops leaks - nothing else
mDesc = STRINGIZE(LLStringUtil::quote(params.executable) << " (" << pinfo.dwProcessId << ')'); mDesc = STRINGIZE(LLStringUtil::quote(params.executable) << " (" << mProcessID << ')');
LL_INFOS("LLProcess") << "Launched " << params << " (" << pinfo.dwProcessId << ")" << LL_ENDL; LL_INFOS("LLProcess") << "Launched " << params << " (" << mProcessID << ")" << LL_ENDL;
// Now associate the new child process with our Job Object -- unless // Now associate the new child process with our Job Object -- unless
// autokill is false, i.e. caller asserts the child should persist. // autokill is false, i.e. caller asserts the child should persist.
if (params.autokill) 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; return 0;
DWORD waitresult = WaitForSingleObject(handle, 0); DWORD waitresult = WaitForSingleObject(h, 0);
if(waitresult == WAIT_OBJECT_0) if(waitresult == WAIT_OBJECT_0)
{ {
// the process has completed. // the process has completed.
...@@ -233,16 +244,16 @@ LLProcess::id LLProcess::isRunning(id handle, const std::string& desc) ...@@ -233,16 +244,16 @@ LLProcess::id LLProcess::isRunning(id handle, const std::string& desc)
return 0; return 0;
} }
return handle; return h;
} }
bool LLProcess::kill(void) bool LLProcess::kill(void)
{ {
if (! mProcessID) if (! mProcessHandle)
return false; return false;
LL_INFOS("LLProcess") << "killing " << mDesc << LL_ENDL; LL_INFOS("LLProcess") << "killing " << mDesc << LL_ENDL;
TerminateProcess(mProcessID, 0); TerminateProcess(mProcessHandle, 0);
return ! isRunning(); return ! isRunning();
} }
...@@ -302,7 +313,7 @@ static bool reap_pid(pid_t pid) ...@@ -302,7 +313,7 @@ static bool reap_pid(pid_t pid)
return false; return false;
} }
void LLProcess::launch(const LLSDParamAdapter<Params>& params) void LLProcess::launch(const LLSDOrParams& params)
{ {
// flush all buffers before the child inherits them // flush all buffers before the child inherits them
::fflush(NULL); ::fflush(NULL);
...@@ -359,6 +370,7 @@ void LLProcess::launch(const LLSDParamAdapter<Params>& params) ...@@ -359,6 +370,7 @@ void LLProcess::launch(const LLSDParamAdapter<Params>& params)
// parent process // parent process
mProcessID = child; mProcessID = child;
mProcessHandle = child;
mDesc = STRINGIZE(LLStringUtil::quote(params.executable) << " (" << mProcessID << ')'); mDesc = STRINGIZE(LLStringUtil::quote(params.executable) << " (" << mProcessID << ')');
LL_INFOS("LLProcess") << "Launched " << params << " (" << mProcessID << ")" << LL_ENDL; LL_INFOS("LLProcess") << "Launched " << params << " (" << mProcessID << ")" << LL_ENDL;
......
...@@ -35,7 +35,7 @@ ...@@ -35,7 +35,7 @@
#if LL_WINDOWS #if LL_WINDOWS
#define WIN32_LEAN_AND_MEAN #define WIN32_LEAN_AND_MEAN
#include <windows.h> #include <windows.h> // HANDLE (eye roll)
#endif #endif
class LLProcess; class LLProcess;
...@@ -79,6 +79,7 @@ class LL_COMMON_API LLProcess: public boost::noncopyable ...@@ -79,6 +79,7 @@ class LL_COMMON_API LLProcess: public boost::noncopyable
/// implicitly kill process on destruction of LLProcess object /// implicitly kill process on destruction of LLProcess object
Optional<bool> autokill; Optional<bool> autokill;
}; };
typedef LLSDParamAdapter<Params> LLSDOrParams;
/** /**
* Factory accepting either plain LLSD::Map or Params block. * Factory accepting either plain LLSD::Map or Params block.
...@@ -91,7 +92,7 @@ class LL_COMMON_API LLProcess: public boost::noncopyable ...@@ -91,7 +92,7 @@ class LL_COMMON_API LLProcess: public boost::noncopyable
* cwd (optional, string, dft no chdir): change to this directory before executing * cwd (optional, string, dft no chdir): change to this directory before executing
* autokill (optional, bool, dft true): implicit kill() on ~LLProcess * autokill (optional, bool, dft true): implicit kill() on ~LLProcess
*/ */
static LLProcessPtr create(const LLSDParamAdapter<Params>& params); static LLProcessPtr create(const LLSDOrParams& params);
virtual ~LLProcess(); virtual ~LLProcess();
// isRunning isn't const because, if child isn't running, it clears stored // isRunning isn't const because, if child isn't running, it clears stored
...@@ -103,35 +104,46 @@ class LL_COMMON_API LLProcess: public boost::noncopyable ...@@ -103,35 +104,46 @@ class LL_COMMON_API LLProcess: public boost::noncopyable
bool kill(void); bool kill(void);
#if LL_WINDOWS #if LL_WINDOWS
typedef HANDLE id; typedef int id; ///< as returned by getProcessID()
typedef HANDLE handle; ///< as returned by getProcessHandle()
#else #else
typedef pid_t id; typedef pid_t id;
typedef pid_t handle;
#endif #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 * Test if a process (@c handle obtained from getProcessHandle()) is still
* running. Return is same nonzero id value if still running, else * 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 * 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: * stored variable as a side effect, you can write code like this:
* @code * @code
* childpid = LLProcess::isRunning(childpid); * hchild = LLProcess::isRunning(hchild);
* @endcode * @endcode
* @note This method is intended as a unit-test hook, not as the first of * @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 * a whole set of operations supported on freestanding @c handle values.
* functionality should be added as nonstatic members operating on * New functionality should be added as nonstatic members operating on
* mProcessID. * the same data as getProcessHandle().
*/ */
static id isRunning(id, const std::string& desc=""); static handle isRunning(handle, const std::string& desc="");
private: private:
/// constructor is private: use create() instead /// constructor is private: use create() instead
LLProcess(const LLSDParamAdapter<Params>& params); LLProcess(const LLSDOrParams& params);
void launch(const LLSDParamAdapter<Params>& params); void launch(const LLSDOrParams& params);
std::string mDesc; std::string mDesc;
id mProcessID; id mProcessID;
handle mProcessHandle;
bool mAutokill; bool mAutokill;
}; };
......
...@@ -599,7 +599,7 @@ namespace tut ...@@ -599,7 +599,7 @@ namespace tut
{ {
set_test_name("implicit kill()"); set_test_name("implicit kill()");
NamedTempFile out("out", "not started"); NamedTempFile out("out", "not started");
LLProcess::id pid(0); LLProcess::handle phandle(0);
{ {
PythonProcessLauncher py("kill()", PythonProcessLauncher py("kill()",
"from __future__ import with_statement\n" "from __future__ import with_statement\n"
...@@ -614,8 +614,8 @@ namespace tut ...@@ -614,8 +614,8 @@ namespace tut
py.mParams.args.add(out.getName()); py.mParams.args.add(out.getName());
py.mPy = LLProcess::create(py.mParams); py.mPy = LLProcess::create(py.mParams);
ensure("couldn't launch kill() script", py.mPy); ensure("couldn't launch kill() script", py.mPy);
// Capture id for later // Capture handle for later
pid = py.mPy->getProcessID(); phandle = py.mPy->getProcessHandle();
// Wait for the script to wake up and do its first write // Wait for the script to wake up and do its first write
int i = 0, timeout = 60; int i = 0, timeout = 60;
for ( ; i < timeout; ++i) for ( ; i < timeout; ++i)
...@@ -630,7 +630,7 @@ namespace tut ...@@ -630,7 +630,7 @@ namespace tut
// Destroy the LLProcess, which should kill the child. // Destroy the LLProcess, which should kill the child.
} }
// wait for the script to terminate... one way or another. // wait for the script to terminate... one way or another.
while (LLProcess::isRunning(pid)) while (LLProcess::isRunning(phandle))
{ {
sleep(1); sleep(1);
} }
...@@ -646,7 +646,7 @@ namespace tut ...@@ -646,7 +646,7 @@ namespace tut
set_test_name("autokill"); set_test_name("autokill");
NamedTempFile from("from", "not started"); NamedTempFile from("from", "not started");
NamedTempFile to("to", ""); NamedTempFile to("to", "");
LLProcess::id pid(0); LLProcess::handle phandle(0);
{ {
PythonProcessLauncher py("autokill", PythonProcessLauncher py("autokill",
"from __future__ import with_statement\n" "from __future__ import with_statement\n"
...@@ -672,8 +672,8 @@ namespace tut ...@@ -672,8 +672,8 @@ namespace tut
py.mParams.autokill = false; py.mParams.autokill = false;
py.mPy = LLProcess::create(py.mParams); py.mPy = LLProcess::create(py.mParams);
ensure("couldn't launch kill() script", py.mPy); ensure("couldn't launch kill() script", py.mPy);
// Capture id for later // Capture handle for later
pid = py.mPy->getProcessID(); phandle = py.mPy->getProcessHandle();
// Wait for the script to wake up and do its first write // Wait for the script to wake up and do its first write
int i = 0, timeout = 60; int i = 0, timeout = 60;
for ( ; i < timeout; ++i) for ( ; i < timeout; ++i)
...@@ -695,7 +695,7 @@ namespace tut ...@@ -695,7 +695,7 @@ namespace tut
outf << "go"; outf << "go";
} // flush and close. } // flush and close.
// now wait for the script to terminate... one way or another. // now wait for the script to terminate... one way or another.
while (LLProcess::isRunning(pid)) while (LLProcess::isRunning(phandle))
{ {
sleep(1); sleep(1);
} }
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment