From ac2f20fc0393371d41296267c654df8cffdeedb4 Mon Sep 17 00:00:00 2001 From: Rye Mutt Date: Mon, 22 Jul 2024 09:10:19 -0400 Subject: Use thread pool to create plugin processes to reduce created/destroyed threads and fix various crashes (#2079) --- indra/llplugin/llpluginprocessparent.cpp | 149 +++++++++++++------------------ indra/llplugin/llpluginprocessparent.h | 9 +- 2 files changed, 62 insertions(+), 96 deletions(-) (limited to 'indra/llplugin') diff --git a/indra/llplugin/llpluginprocessparent.cpp b/indra/llplugin/llpluginprocessparent.cpp index 00abcf740f..bd1e19c294 100644 --- a/indra/llplugin/llpluginprocessparent.cpp +++ b/indra/llplugin/llpluginprocessparent.cpp @@ -34,6 +34,9 @@ #include "llpluginmessageclasses.h" #include "llsdserialize.h" #include "stringize.h" +#include "threadpool.h" +#include "workqueue.h" + #include "llapr.h" //virtual @@ -79,29 +82,8 @@ protected: }; - -class LLPluginProcessCreationThread : public LLThread -{ -public: - LLPluginProcessCreationThread(LLPluginProcessParent *parent) : - LLThread("LLPluginProcessCreationThread", gAPRPoolp), - pParent(parent) - { - } -protected: - // Inherited from LLThread, should run once - /*virtual*/ void run(void) - { - pParent->createPluginProcess(); - } -private: - LLPluginProcessParent *pParent; - -}; - LLPluginProcessParent::LLPluginProcessParent(LLPluginProcessParentOwner *owner): - mIncomingQueueMutex(), - pProcessCreationThread(NULL) + mIncomingQueueMutex() { if(!sInstancesMutex) { @@ -130,18 +112,6 @@ LLPluginProcessParent::LLPluginProcessParent(LLPluginProcessParentOwner *owner): LLPluginProcessParent::~LLPluginProcessParent() { LL_DEBUGS("Plugin") << "destructor" << LL_ENDL; - if (pProcessCreationThread) - { - if (!pProcessCreationThread->isStopped()) - { - // Shouldn't happen at this stage - LL_WARNS("Plugin") << "Shutting down active pProcessCreationThread" << LL_ENDL; - pProcessCreationThread->shutdown(); - ms_sleep(20); - } - delete pProcessCreationThread; - pProcessCreationThread = NULL; - } // Destroy any remaining shared memory regions sharedMemoryRegionsType::iterator iter; @@ -352,35 +322,6 @@ bool LLPluginProcessParent::accept() return result; } -bool LLPluginProcessParent::createPluginProcess() -{ - if (!mProcess) - { - // Only argument to the launcher is the port number we're listening on - mProcessParams.args.add(stringize(mBoundPort)); - mProcess = LLProcess::create(mProcessParams); - return mProcess != NULL; - } - - return false; -} - -void LLPluginProcessParent::clearProcessCreationThread() -{ - if (pProcessCreationThread) - { - if (!pProcessCreationThread->isStopped()) - { - pProcessCreationThread->shutdown(); - } - else - { - delete pProcessCreationThread; - pProcessCreationThread = NULL; - } - } -} - void LLPluginProcessParent::idle(void) { bool idle_again; @@ -542,30 +483,72 @@ void LLPluginProcessParent::idle(void) case STATE_LISTENING: { + // Only argument to the launcher is the port number we're listening on + mProcessParams.args.add(stringize(mBoundPort)); + // Launch the plugin process. - if (mDebug && !pProcessCreationThread) + if (mDebug && !mProcess) { - createPluginProcess(); - if (!mProcess) + if (!(mProcess = LLProcess::create(mProcessParams))) { errorState(); } } - else if (pProcessCreationThread == NULL) - { - // exe plugin process allocation can be hindered by a number - // of factors, don't hold whole viewer because of it, use thread - pProcessCreationThread = new LLPluginProcessCreationThread(this); - pProcessCreationThread->start(); - } - else if (!mProcess && pProcessCreationThread->isStopped()) + else if (!mProcess && !mProcessCreationRequested) { - delete pProcessCreationThread; - pProcessCreationThread = NULL; - errorState(); + mProcessCreationRequested = true; + LL::WorkQueue::ptr_t main_queue = LL::WorkQueue::getInstance("mainloop"); + // *NOTE: main_queue->postTo casts this refcounted smart pointer to a weak + // pointer + LL::WorkQueue::ptr_t general_queue = LL::WorkQueue::getInstance("General"); + llassert_always(main_queue); + llassert_always(general_queue); + + auto process_params = mProcessParams; + + bool posted = main_queue->postTo( + general_queue, + [process_params]() // Work done on general queue + { + return LLProcess::create(process_params); + }, + [this](LLProcessPtr new_process) // Callback to main thread + mutable { + 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. + LLCoros::LockType lock(*sInstancesMutex); + mapInstances_t::iterator it = sInstances.find(this); + if (it != sInstances.end()) + that = (*it).second; + } + + if (that) + { + if (new_process) + { + that->mProcess = new_process; + } + else + { + that->mProcessCreationRequested = false; + that->errorState(); + } + } + + }); + if (!posted) + { + LL_WARNS("Plugin") << "Failed to dispath process creation to threadpool" << LL_ENDL; + if (!(mProcess = LLProcess::create(mProcessParams))) + { + mProcessCreationRequested = false; + errorState(); + } + } } - if (mProcess) { if(mDebug) @@ -595,15 +578,6 @@ void LLPluginProcessParent::idle(void) // This will allow us to time out if the process never starts. mHeartbeat.start(); mHeartbeat.setTimerExpirySec(mPluginLaunchTimeout); - - // pProcessCreationThread should have stopped by this point, - // but check just in case it paused on statistics sync - if (pProcessCreationThread && pProcessCreationThread->isStopped()) - { - delete pProcessCreationThread; - pProcessCreationThread = NULL; - } - setState(STATE_LAUNCHED); } } @@ -706,7 +680,6 @@ void LLPluginProcessParent::idle(void) killSockets(); setState(STATE_DONE); dirtyPollSet(); - clearProcessCreationThread(); break; case STATE_DONE: diff --git a/indra/llplugin/llpluginprocessparent.h b/indra/llplugin/llpluginprocessparent.h index d1c4933d81..334f1411af 100644 --- a/indra/llplugin/llpluginprocessparent.h +++ b/indra/llplugin/llpluginprocessparent.h @@ -69,11 +69,6 @@ public: const std::string &plugin_filename, bool debug); - // Creates a process - // returns true if process already exists or if created, - // false if failed to create - bool createPluginProcess(); - void idle(void); // returns true if the plugin is on its way to steady state @@ -168,15 +163,13 @@ private: bool accept(); - void clearProcessCreationThread(); - LLSocket::ptr_t mListenSocket; LLSocket::ptr_t mSocket; U32 mBoundPort; LLProcess::Params mProcessParams; LLProcessPtr mProcess; - LLThread *pProcessCreationThread; + bool mProcessCreationRequested = false; std::string mPluginFile; std::string mPluginDir; -- cgit v1.2.3