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

MAINT-1144: Defend against NULL LLPluginProcessParent::mProcess.

The change from LLProcessLauncher to LLProcess introduces the possibility of a
NULL (default-constructed) LLProcessPtr. Add certain static LLProcess methods
accepting LLProcessPtr, forwarding to nonstatic method when non-NULL but doing
something reasonable with NULL. Use these methods in LLPLuginProcessParent.
parent fd3ae758
No related branches found
No related tags found
No related merge requests found
...@@ -819,16 +819,43 @@ bool LLProcess::kill(const std::string& who) ...@@ -819,16 +819,43 @@ bool LLProcess::kill(const std::string& who)
return ! isRunning(); return ! isRunning();
} }
//static
bool LLProcess::kill(const LLProcessPtr& p, const std::string& who)
{
if (! p)
return true; // process dead! (was never running)
return p->kill(who);
}
bool LLProcess::isRunning() const bool LLProcess::isRunning() const
{ {
return getStatus().mState == RUNNING; return getStatus().mState == RUNNING;
} }
//static
bool LLProcess::isRunning(const LLProcessPtr& p)
{
if (! p)
return false;
return p->isRunning();
}
LLProcess::Status LLProcess::getStatus() const LLProcess::Status LLProcess::getStatus() const
{ {
return mStatus; return mStatus;
} }
//static
LLProcess::Status LLProcess::getStatus(const LLProcessPtr& p)
{
if (! p)
{
// default-constructed Status has mState == UNSTARTED
return Status();
}
return p->getStatus();
}
std::string LLProcess::getStatusString() const std::string LLProcess::getStatusString() const
{ {
return getStatusString(getStatus()); return getStatusString(getStatus());
...@@ -839,6 +866,17 @@ std::string LLProcess::getStatusString(const Status& status) const ...@@ -839,6 +866,17 @@ std::string LLProcess::getStatusString(const Status& status) const
return getStatusString(mDesc, status); return getStatusString(mDesc, status);
} }
//static
std::string LLProcess::getStatusString(const std::string& desc, const LLProcessPtr& p)
{
if (! p)
{
// default-constructed Status has mState == UNSTARTED
return getStatusString(desc, Status());
}
return desc + " " + p->getStatusString();
}
//static //static
std::string LLProcess::getStatusString(const std::string& desc, const Status& status) std::string LLProcess::getStatusString(const std::string& desc, const Status& status)
{ {
......
...@@ -239,6 +239,10 @@ public: ...@@ -239,6 +239,10 @@ public:
/// Is child process still running? /// Is child process still running?
bool isRunning() const; bool isRunning() const;
// static isRunning(LLProcessPtr), getStatus(LLProcessPtr),
// getStatusString(LLProcessPtr), kill(LLProcessPtr) handle the case in
// which the passed LLProcessPtr might be NULL (default-constructed).
static bool isRunning(const LLProcessPtr&);
/** /**
* State of child process * State of child process
...@@ -272,8 +276,10 @@ public: ...@@ -272,8 +276,10 @@ public:
/// Status query /// Status query
Status getStatus() const; Status getStatus() const;
static Status getStatus(const LLProcessPtr&);
/// English Status string query, for logging etc. /// English Status string query, for logging etc.
std::string getStatusString() const; std::string getStatusString() const;
static std::string getStatusString(const std::string& desc, const LLProcessPtr&);
/// English Status string query for previously-captured Status /// English Status string query for previously-captured Status
std::string getStatusString(const Status& status) const; std::string getStatusString(const Status& status) const;
/// static English Status string query /// static English Status string query
...@@ -282,6 +288,7 @@ public: ...@@ -282,6 +288,7 @@ public:
// Attempt to kill the process -- returns true if the process is no longer running when it returns. // Attempt to kill the process -- returns true if the process is no longer running when it returns.
// Note that even if this returns false, the process may exit some time after it's called. // Note that even if this returns false, the process may exit some time after it's called.
bool kill(const std::string& who=""); bool kill(const std::string& who="");
static bool kill(const LLProcessPtr& p, const std::string& who="");
#if LL_WINDOWS #if LL_WINDOWS
typedef int id; ///< as returned by getProcessID() typedef int id; ///< as returned by getProcessID()
...@@ -314,10 +321,10 @@ public: ...@@ -314,10 +321,10 @@ public:
* New functionality should be added as nonstatic members operating on * New functionality should be added as nonstatic members operating on
* the same data as getProcessHandle(). * the same data as getProcessHandle().
* *
* In particular, if child termination is detected by static isRunning() * In particular, if child termination is detected by this static isRunning()
* rather than by nonstatic isRunning(), the LLProcess object won't be * rather than by nonstatic isRunning(), the LLProcess object won't be
* aware of the child's changed status and may encounter OS errors trying * aware of the child's changed status and may encounter OS errors trying
* to obtain it. static isRunning() is only intended for after the * to obtain it. This static isRunning() is only intended for after the
* launching LLProcess object has been destroyed. * launching LLProcess object has been destroyed.
*/ */
static handle isRunning(handle, const std::string& desc=""); static handle isRunning(handle, const std::string& desc="");
......
...@@ -134,11 +134,8 @@ LLPluginProcessParent::~LLPluginProcessParent() ...@@ -134,11 +134,8 @@ LLPluginProcessParent::~LLPluginProcessParent()
// and remove it from our map // and remove it from our map
mSharedMemoryRegions.erase(iter); mSharedMemoryRegions.erase(iter);
} }
if (mProcess) LLProcess::kill(mProcess);
{
mProcess->kill();
}
killSockets(); killSockets();
} }
...@@ -471,7 +468,7 @@ void LLPluginProcessParent::idle(void) ...@@ -471,7 +468,7 @@ void LLPluginProcessParent::idle(void)
break; break;
case STATE_EXITING: case STATE_EXITING:
if (! mProcess->isRunning()) if (! LLProcess::isRunning(mProcess))
{ {
setState(STATE_CLEANUP); setState(STATE_CLEANUP);
} }
...@@ -499,7 +496,7 @@ void LLPluginProcessParent::idle(void) ...@@ -499,7 +496,7 @@ void LLPluginProcessParent::idle(void)
break; break;
case STATE_CLEANUP: case STATE_CLEANUP:
mProcess->kill(); LLProcess::kill(mProcess);
killSockets(); killSockets();
setState(STATE_DONE); setState(STATE_DONE);
break; break;
...@@ -1078,7 +1075,7 @@ bool LLPluginProcessParent::pluginLockedUpOrQuit() ...@@ -1078,7 +1075,7 @@ bool LLPluginProcessParent::pluginLockedUpOrQuit()
{ {
bool result = false; bool result = false;
if (! mProcess->isRunning()) if (! LLProcess::isRunning(mProcess))
{ {
LL_WARNS("Plugin") << "child exited" << LL_ENDL; LL_WARNS("Plugin") << "child exited" << LL_ENDL;
result = true; result = true;
......
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