diff options
author | Rider Linden <rider@lindenlab.com> | 2015-11-10 13:45:30 -0800 |
---|---|---|
committer | Rider Linden <rider@lindenlab.com> | 2015-11-10 13:45:30 -0800 |
commit | 059925eafb66dc0e2d8ef9c113ca8980a34c655d (patch) | |
tree | 4189394ddc4257f9e448ed878d0b1b15766fa7a4 | |
parent | a743d939071fbdbbf619f2e282419623da932322 (diff) |
Added code to initiate controlled shutdown of plugins with timeouts for misbeahving plugin.
-rwxr-xr-x | indra/llplugin/llpluginclassmedia.cpp | 9 | ||||
-rwxr-xr-x | indra/llplugin/llpluginclassmedia.h | 2 | ||||
-rwxr-xr-x | indra/llplugin/llpluginprocesschild.cpp | 43 | ||||
-rwxr-xr-x | indra/llplugin/llpluginprocesschild.h | 7 | ||||
-rwxr-xr-x | indra/llplugin/llpluginprocessparent.cpp | 308 | ||||
-rwxr-xr-x | indra/llplugin/llpluginprocessparent.h | 28 | ||||
-rwxr-xr-x | indra/newview/llappviewer.cpp | 3 | ||||
-rwxr-xr-x | indra/newview/tests/llmediadataclient_test.cpp | 2 |
8 files changed, 263 insertions, 139 deletions
diff --git a/indra/llplugin/llpluginclassmedia.cpp b/indra/llplugin/llpluginclassmedia.cpp index 53fae52021..85653a0fcc 100755 --- a/indra/llplugin/llpluginclassmedia.cpp +++ b/indra/llplugin/llpluginclassmedia.cpp @@ -48,7 +48,6 @@ static int nextPowerOf2( int value ) LLPluginClassMedia::LLPluginClassMedia(LLPluginClassMediaOwner *owner) { mOwner = owner; - mPlugin = NULL; reset(); //debug use @@ -68,7 +67,7 @@ bool LLPluginClassMedia::init(const std::string &launcher_filename, const std::s LL_DEBUGS("Plugin") << "dir: " << plugin_dir << LL_ENDL; LL_DEBUGS("Plugin") << "plugin: " << plugin_filename << LL_ENDL; - mPlugin = new LLPluginProcessParent(this); + mPlugin = LLPluginProcessParent::create(this); mPlugin->setSleepTime(mSleepTime); // Queue up the media init message -- it will be sent after all the currently queued messages. @@ -84,10 +83,10 @@ bool LLPluginClassMedia::init(const std::string &launcher_filename, const std::s void LLPluginClassMedia::reset() { - if(mPlugin != NULL) + if(mPlugin) { - delete mPlugin; - mPlugin = NULL; + mPlugin->requestShutdown(); + mPlugin.reset(); } mTextureParamsReceived = false; diff --git a/indra/llplugin/llpluginclassmedia.h b/indra/llplugin/llpluginclassmedia.h index a0167bc5fc..fe02696084 100755 --- a/indra/llplugin/llpluginclassmedia.h +++ b/indra/llplugin/llpluginclassmedia.h @@ -374,7 +374,7 @@ protected: int mPadding; - LLPluginProcessParent *mPlugin; + LLPluginProcessParent::ptr_t mPlugin; LLRect mDirtyRect; diff --git a/indra/llplugin/llpluginprocesschild.cpp b/indra/llplugin/llpluginprocesschild.cpp index f8a282184e..be80d38305 100755 --- a/indra/llplugin/llpluginprocesschild.cpp +++ b/indra/llplugin/llpluginprocesschild.cpp @@ -33,6 +33,7 @@ #include "llpluginmessagepipe.h" #include "llpluginmessageclasses.h" +static const F32 GOODBYE_SECONDS = 20.0f; static const F32 HEARTBEAT_SECONDS = 1.0f; static const F32 PLUGIN_IDLE_SECONDS = 1.0f / 100.0f; // Each call to idle will give the plugin this much time. @@ -194,33 +195,43 @@ void LLPluginProcessChild::idle(void) } } // receivePluginMessage will transition to STATE_UNLOADING - break; + break; + + case STATE_SHUTDOWNREQ: + if (mInstance != NULL) + { + sendMessageToPlugin(LLPluginMessage("base", "cleanup")); + delete mInstance; + mInstance = NULL; + } + setState(STATE_UNLOADING); + mWaitGoodbye.setTimerExpirySec(GOODBYE_SECONDS); + break; case STATE_UNLOADING: - if(mInstance != NULL) - { - sendMessageToPlugin(LLPluginMessage("base", "cleanup")); - delete mInstance; - mInstance = NULL; - } - setState(STATE_UNLOADED); - break; + // waiting for goodbye from plugin. + if (mWaitGoodbye.hasExpired()) + { + LL_WARNS() << "Wait for goodbye expired. Advancing to UNLOADED" << LL_ENDL; + setState(STATE_UNLOADED); + } + break; case STATE_UNLOADED: killSockets(); setState(STATE_DONE); - break; + break; case STATE_ERROR: // Close the socket to the launcher killSockets(); // TODO: Where do we go from here? Just exit()? setState(STATE_DONE); - break; + break; case STATE_DONE: // just sit here. - break; + break; } } while (idle_again); @@ -350,6 +361,10 @@ void LLPluginProcessChild::receiveMessageRaw(const std::string &message) mPluginFile = parsed.getValue("file"); mPluginDir = parsed.getValue("dir"); } + else if (message_name == "shutdown_plugin") + { + setState(STATE_SHUTDOWNREQ); + } else if(message_name == "shm_add") { std::string name = parsed.getValue("name"); @@ -495,6 +510,10 @@ void LLPluginProcessChild::receivePluginMessage(const std::string &message) // Let the parent know it's loaded and initialized. sendMessageToParent(new_message); } + else if (message_name == "goodbye") + { + setState(STATE_UNLOADED); + } else if(message_name == "shm_remove_response") { // Don't pass this message up to the parent diff --git a/indra/llplugin/llpluginprocesschild.h b/indra/llplugin/llpluginprocesschild.h index 531422e792..b916cc9528 100755 --- a/indra/llplugin/llpluginprocesschild.h +++ b/indra/llplugin/llpluginprocesschild.h @@ -80,6 +80,7 @@ private: STATE_PLUGIN_LOADED, // plugin library has been loaded STATE_PLUGIN_INITIALIZING, // plugin is processing init message STATE_RUNNING, // steady state (processing messages) + STATE_SHUTDOWNREQ, // Parent has requested a shutdown. STATE_UNLOADING, // plugin has sent shutdown_response and needs to be unloaded STATE_UNLOADED, // plugin has been unloaded STATE_ERROR, // generic bailout state @@ -101,12 +102,12 @@ private: sharedMemoryRegionsType mSharedMemoryRegions; LLTimer mHeartbeat; - F64 mSleepTime; - F64 mCPUElapsed; + F64 mSleepTime; + F64 mCPUElapsed; bool mBlockingRequest; bool mBlockingResponseReceived; std::queue<std::string> mMessageQueue; - + LLTimer mWaitGoodbye; void deliverQueuedMessages(); }; diff --git a/indra/llplugin/llpluginprocessparent.cpp b/indra/llplugin/llpluginprocessparent.cpp index b5a2588e1e..0a8e58ac90 100755 --- a/indra/llplugin/llpluginprocessparent.cpp +++ b/indra/llplugin/llpluginprocessparent.cpp @@ -46,7 +46,7 @@ bool LLPluginProcessParent::sUseReadThread = false; apr_pollset_t *LLPluginProcessParent::sPollSet = NULL; bool LLPluginProcessParent::sPollsetNeedsRebuild = false; LLMutex *LLPluginProcessParent::sInstancesMutex; -std::list<LLPluginProcessParent*> LLPluginProcessParent::sInstances; +LLPluginProcessParent::mapInstances_t LLPluginProcessParent::sInstances; LLThread *LLPluginProcessParent::sReadThread = NULL; @@ -104,27 +104,12 @@ LLPluginProcessParent::LLPluginProcessParent(LLPluginProcessParentOwner *owner): // Don't start the timer here -- start it when we actually launch the plugin process. mHeartbeat.stop(); - // Don't add to the global list until fully constructed. - { - LLMutexLock lock(sInstancesMutex); - sInstances.push_back(this); - } } LLPluginProcessParent::~LLPluginProcessParent() { LL_DEBUGS("Plugin") << "destructor" << LL_ENDL; - // Remove from the global list before beginning destruction. - { - // Make sure to get the global mutex _first_ here, to avoid a possible deadlock against LLPluginProcessParent::poll() - LLMutexLock lock(sInstancesMutex); - { - LLMutexLock lock2(&mIncomingQueueMutex); - sInstances.remove(this); - } - } - // Destroy any remaining shared memory regions sharedMemoryRegionsType::iterator iter; while((iter = mSharedMemoryRegions.begin()) != mSharedMemoryRegions.end()) @@ -139,9 +124,109 @@ LLPluginProcessParent::~LLPluginProcessParent() } LLProcess::kill(mProcess); - killSockets(); + if (!LLApp::isQuitting()) + { // If we are quitting, the network sockets will already have been destroyed. + killSockets(); + } +} + +/*static*/ +LLPluginProcessParent::ptr_t LLPluginProcessParent::create(LLPluginProcessParentOwner *owner) +{ + ptr_t that(new LLPluginProcessParent(owner)); + + // Don't add to the global list until fully constructed. + { + LLMutexLock lock(sInstancesMutex); + sInstances.insert(mapInstances_t::value_type(that.get(), that)); + } + + return that; +} + +/*static*/ +void LLPluginProcessParent::shutdown() +{ + LLMutexLock lock(sInstancesMutex); + + mapInstances_t::iterator it; + for (it = sInstances.begin(); it != sInstances.end(); ++it) + { + (*it).second->setState(STATE_GOODBYE); + (*it).second->idle(); + } + sInstances.clear(); +} + + +void LLPluginProcessParent::requestShutdown() +{ + setState(STATE_GOODBYE); + mOwner = NULL; + + if (LLApp::isQuitting()) + { // if we're quitting, run the idle once more + idle(); + removeFromProcessing(); + return; + } + + static uint32_t count = 0; + std::stringstream namestream; + + namestream << "LLPluginProcessParentListener" << ++count; + + //*HACK!*// + // After requestShutdown has been called our previous owner will no longer call + // our idle() method. Tie into the event loop here to do that until we are good + // and finished. + LL_DEBUGS("LLPluginProcessParent") << "listening on \"mainloop\"" << LL_ENDL; + mPolling = LLEventPumps::instance().obtain("mainloop") + .listen(namestream.str(), boost::bind(&LLPluginProcessParent::pollTick, this)); + +} + +bool LLPluginProcessParent::pollTick() +{ + if (isDone()) + { + ptr_t that; + { + // this grabs a copy of the smart pointer to ourselves to ensure that we do not + // get destroyed until after this method returns. + LLMutexLock lock(sInstancesMutex); + mapInstances_t::iterator it = sInstances.find(this); + if (it != sInstances.end()) + that = (*it).second; + } + + removeFromProcessing(); + return true; + } + + idle(); + return false; } +void LLPluginProcessParent::removeFromProcessing() +{ + // Remove from the global list before beginning destruction. + { + // Make sure to get the global mutex _first_ here, to avoid a possible deadlock against LLPluginProcessParent::poll() + LLMutexLock lock(sInstancesMutex); + { + LLMutexLock lock2(&mIncomingQueueMutex); + sInstances.erase(this); + } + } +} + +bool LLPluginProcessParent::wantsPolling() const +{ + return (mPollFD.client_data && (mState != STATE_DONE)); +} + + void LLPluginProcessParent::killSockets(void) { { @@ -371,48 +456,48 @@ void LLPluginProcessParent::idle(void) break; case STATE_LISTENING: - { - // Launch the plugin process. + { + // Launch the plugin process. - // Only argument to the launcher is the port number we're listening on - mProcessParams.args.add(stringize(mBoundPort)); - if (! (mProcess = LLProcess::create(mProcessParams))) - { - errorState(); - } - else - { - if(mDebug) - { - #if LL_DARWIN - // If we're set to debug, start up a gdb instance in a new terminal window and have it attach to the plugin process and continue. + // Only argument to the launcher is the port number we're listening on + mProcessParams.args.add(stringize(mBoundPort)); + if (! (mProcess = LLProcess::create(mProcessParams))) + { + errorState(); + } + else + { + if(mDebug) + { +#if LL_DARWIN + // If we're set to debug, start up a gdb instance in a new terminal window and have it attach to the plugin process and continue. - // The command we're constructing would look like this on the command line: - // osascript -e 'tell application "Terminal"' -e 'set win to do script "gdb -pid 12345"' -e 'do script "continue" in win' -e 'end tell' - - LLProcess::Params params; - params.executable = "/usr/bin/osascript"; - params.args.add("-e"); - params.args.add("tell application \"Terminal\""); - params.args.add("-e"); - params.args.add(STRINGIZE("set win to do script \"gdb -pid " - << mProcess->getProcessID() << "\"")); - params.args.add("-e"); - params.args.add("do script \"continue\" in win"); - params.args.add("-e"); - params.args.add("end tell"); - mDebugger = LLProcess::create(params); - - #endif - } + // The command we're constructing would look like this on the command line: + // osascript -e 'tell application "Terminal"' -e 'set win to do script "gdb -pid 12345"' -e 'do script "continue" in win' -e 'end tell' + + LLProcess::Params params; + params.executable = "/usr/bin/osascript"; + params.args.add("-e"); + params.args.add("tell application \"Terminal\""); + params.args.add("-e"); + params.args.add(STRINGIZE("set win to do script \"gdb -pid " + << mProcess->getProcessID() << "\"")); + params.args.add("-e"); + params.args.add("do script \"continue\" in win"); + params.args.add("-e"); + params.args.add("end tell"); + mDebugger = LLProcess::create(params); + +#endif + } - // This will allow us to time out if the process never starts. - mHeartbeat.start(); - mHeartbeat.setTimerExpirySec(mPluginLaunchTimeout); - setState(STATE_LAUNCHED); - } - } - break; + // This will allow us to time out if the process never starts. + mHeartbeat.start(); + mHeartbeat.setTimerExpirySec(mPluginLaunchTimeout); + setState(STATE_LAUNCHED); + } + } + break; case STATE_LAUNCHED: // waiting for the plugin to connect @@ -430,7 +515,7 @@ void LLPluginProcessParent::idle(void) setState(STATE_CONNECTED); } } - break; + break; case STATE_CONNECTED: // waiting for hello message from the plugin @@ -439,7 +524,7 @@ void LLPluginProcessParent::idle(void) { errorState(); } - break; + break; case STATE_HELLO: LL_DEBUGS("Plugin") << "received hello message" << LL_ENDL; @@ -453,7 +538,7 @@ void LLPluginProcessParent::idle(void) } setState(STATE_LOADING); - break; + break; case STATE_LOADING: // The load_plugin_response message will kick us from here into STATE_RUNNING @@ -461,15 +546,23 @@ void LLPluginProcessParent::idle(void) { errorState(); } - break; + break; case STATE_RUNNING: if(pluginLockedUpOrQuit()) { errorState(); } - break; + break; + case STATE_GOODBYE: + { + LLPluginMessage message(LLPLUGIN_MESSAGE_CLASS_INTERNAL, "shutdown_plugin"); + sendMessage(message); + } + setState(STATE_EXITING); + break; + case STATE_EXITING: if (! LLProcess::isRunning(mProcess)) { @@ -480,7 +573,7 @@ void LLPluginProcessParent::idle(void) LL_WARNS("Plugin") << "timeout in exiting state, bailing out" << LL_ENDL; errorState(); } - break; + break; case STATE_LAUNCH_FAILURE: if(mOwner != NULL) @@ -488,7 +581,7 @@ void LLPluginProcessParent::idle(void) mOwner->pluginLaunchFailed(); } setState(STATE_CLEANUP); - break; + break; case STATE_ERROR: if(mOwner != NULL) @@ -496,19 +589,18 @@ void LLPluginProcessParent::idle(void) mOwner->pluginDied(); } setState(STATE_CLEANUP); - break; + break; case STATE_CLEANUP: LLProcess::kill(mProcess); killSockets(); setState(STATE_DONE); - break; - + dirtyPollSet(); + break; case STATE_DONE: // just sit here. - break; - + break; } } while (idle_again); @@ -651,14 +743,14 @@ void LLPluginProcessParent::updatePollset() sPollSet = NULL; } - std::list<LLPluginProcessParent*>::iterator iter; + mapInstances_t::iterator iter; int count = 0; // Count the number of instances that want to be in the pollset for(iter = sInstances.begin(); iter != sInstances.end(); iter++) { - (*iter)->mPolledInput = false; - if((*iter)->mPollFD.client_data) + (*iter).second->mPolledInput = false; + if ((*iter).second->wantsPolling()) { // This instance has a socket that needs to be polled. ++count; @@ -686,12 +778,12 @@ void LLPluginProcessParent::updatePollset() // Pollset was created, add all instances to it. for(iter = sInstances.begin(); iter != sInstances.end(); iter++) { - if((*iter)->mPollFD.client_data) + if ((*iter).second->wantsPolling()) { - status = apr_pollset_add(sPollSet, &((*iter)->mPollFD)); + status = apr_pollset_add(sPollSet, &((*iter).second->mPollFD)); if(status == APR_SUCCESS) { - (*iter)->mPolledInput = true; + (*iter).second->mPolledInput = true; } else { @@ -756,45 +848,27 @@ void LLPluginProcessParent::poll(F64 timeout) if(status == APR_SUCCESS) { // One or more of the descriptors signalled. Call them. - for(int i = 0; i < count; i++) - { - LLPluginProcessParent *self = (LLPluginProcessParent *)(descriptors[i].client_data); - // NOTE: the descriptor returned here is actually a COPY of the original (even though we create the pollset with APR_POLLSET_NOCOPY). - // This means that even if the parent has set its mPollFD.client_data to NULL, the old pointer may still there in this descriptor. - // It's even possible that the old pointer no longer points to a valid LLPluginProcessParent. - // This means that we can't safely dereference the 'self' pointer here without some extra steps... - if(self) - { - // Make sure this pointer is still in the instances list - bool valid = false; - { - LLMutexLock lock(sInstancesMutex); - for(std::list<LLPluginProcessParent*>::iterator iter = sInstances.begin(); iter != sInstances.end(); ++iter) - { - if(*iter == self) - { - // Lock the instance's mutex before unlocking the global mutex. - // This avoids a possible race condition where the instance gets deleted between this check and the servicePoll() call. - self->mIncomingQueueMutex.lock(); - valid = true; - break; - } - } - } - - if(valid) - { - // The instance is still valid. - // Pull incoming messages off the socket - self->servicePoll(); - self->mIncomingQueueMutex.unlock(); - } - else - { - LL_DEBUGS("PluginPoll") << "detected deleted instance " << self << LL_ENDL; - } + for (int i = 0; i < count; i++) + { + void *thatId = descriptors[i].client_data; + + ptr_t that; + mapInstances_t::iterator it; + + { + LLMutexLock lock(sInstancesMutex); + it = sInstances.find(thatId); + if (it != sInstances.end()) + that = (*it).second; + } + + if (that) + { + that->mIncomingQueueMutex.lock(); + that->servicePoll(); + that->mIncomingQueueMutex.unlock(); + } - } } } else if(APR_STATUS_IS_TIMEUP(status)) @@ -812,6 +886,16 @@ void LLPluginProcessParent::poll(F64 timeout) LL_WARNS("PluginPoll") << "apr_pollset_poll failed with status " << status << LL_ENDL; } } + + // Remove instances in the done state from the sInstances map. + mapInstances_t::iterator itClean = sInstances.begin(); + while (itClean != sInstances.end()) + { + if ((*itClean).second->isDone()) + sInstances.erase(itClean++); + else + ++itClean; + } } void LLPluginProcessParent::servicePoll() diff --git a/indra/llplugin/llpluginprocessparent.h b/indra/llplugin/llpluginprocessparent.h index 24be7eb148..df1630255c 100755 --- a/indra/llplugin/llpluginprocessparent.h +++ b/indra/llplugin/llpluginprocessparent.h @@ -30,6 +30,7 @@ #define LL_LLPLUGINPROCESSPARENT_H #include <queue> +#include <boost/enable_shared_from_this.hpp> #include "llapr.h" #include "llprocess.h" @@ -40,8 +41,9 @@ #include "lliosocket.h" #include "llthread.h" #include "llsd.h" +#include "llevents.h" -class LLPluginProcessParentOwner +class LLPluginProcessParentOwner : public boost::enable_shared_from_this < LLPluginProcessParentOwner > { public: virtual ~LLPluginProcessParentOwner(); @@ -55,8 +57,11 @@ public: class LLPluginProcessParent : public LLPluginMessagePipeOwner { LOG_CLASS(LLPluginProcessParent); + + LLPluginProcessParent(LLPluginProcessParentOwner *owner); public: - LLPluginProcessParent(LLPluginProcessParentOwner *owner); + typedef boost::shared_ptr<LLPluginProcessParent> ptr_t; + ~LLPluginProcessParent(); void init(const std::string &launcher_filename, @@ -89,7 +94,10 @@ public: void sendMessage(const LLPluginMessage &message); void receiveMessage(const LLPluginMessage &message); - + + static ptr_t create(LLPluginProcessParentOwner *owner); + void requestShutdown(); + // Inherited from LLPluginMessagePipeOwner /*virtual*/ void receiveMessageRaw(const std::string &message); /*virtual*/ void receiveMessageEarly(const LLPluginMessage &message); @@ -121,7 +129,10 @@ public: static bool canPollThreadRun() { return (sPollSet || sPollsetNeedsRebuild || sUseReadThread); }; static void setUseReadThread(bool use_read_thread); static bool getUseReadThread() { return sUseReadThread; }; + + static void shutdown(); private: + typedef std::map<void *, ptr_t> mapInstances_t; enum EState { @@ -133,6 +144,7 @@ private: STATE_HELLO, // first message from the plugin process has been received STATE_LOADING, // process has been asked to load the plugin STATE_RUNNING, // + STATE_GOODBYE, STATE_LAUNCH_FAILURE, // Failure before plugin loaded STATE_ERROR, // generic bailout state STATE_CLEANUP, // clean everything up @@ -143,6 +155,9 @@ private: EState mState; void setState(EState state); + bool wantsPolling() const; + void removeFromProcessing(); + bool pluginLockedUp(); bool pluginLockedUpOrQuit(); @@ -185,12 +200,15 @@ private: static apr_pollset_t *sPollSet; static bool sPollsetNeedsRebuild; static LLMutex *sInstancesMutex; - static std::list<LLPluginProcessParent*> sInstances; + static mapInstances_t sInstances; static void dirtyPollSet(); static void updatePollset(); void servicePoll(); static LLThread *sReadThread; - + + LLTempBoundListener mPolling; + bool pollTick(); + LLMutex mIncomingQueueMutex; std::queue<LLPluginMessage> mIncomingQueue; }; diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index 03a8756ac8..f0cfd052cf 100755 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -1730,6 +1730,9 @@ bool LLAppViewer::cleanup() // to ensure shutdown order LLMortician::setZealous(TRUE); + // Give any remaining SLPlugin instances a chance to exit cleanly. + LLPluginProcessParent::shutdown(); + LLVoiceClient::getInstance()->terminate(); disconnectViewer(); diff --git a/indra/newview/tests/llmediadataclient_test.cpp b/indra/newview/tests/llmediadataclient_test.cpp index 6f57daf151..7cb4aeb121 100755 --- a/indra/newview/tests/llmediadataclient_test.cpp +++ b/indra/newview/tests/llmediadataclient_test.cpp @@ -26,7 +26,7 @@ #include "linden_common.h" #include "../llviewerprecompiledheaders.h" - + #include <iostream> #include "../test/lltut.h" |