From 7fc5f7e649c564fa8479a72a45459d0cc427d0f8 Mon Sep 17 00:00:00 2001 From: RunitaiLinden Date: Thu, 2 May 2024 10:57:39 -0500 Subject: #1354 Make coroutines use LLCoros::Mutex instead of LLMutex (#1356) * #1354 Make coroutines use LLCoros::Mutex instead of LLMutex * #1354 Fix some more unsafe coroutine executions. * #1354 Implement changes requested by Nat --- indra/llplugin/llpluginprocessparent.cpp | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) (limited to 'indra/llplugin/llpluginprocessparent.cpp') diff --git a/indra/llplugin/llpluginprocessparent.cpp b/indra/llplugin/llpluginprocessparent.cpp index f5966b71de..a28f6c648f 100644 --- a/indra/llplugin/llpluginprocessparent.cpp +++ b/indra/llplugin/llpluginprocessparent.cpp @@ -34,7 +34,6 @@ #include "llpluginmessageclasses.h" #include "llsdserialize.h" #include "stringize.h" - #include "llapr.h" //virtual @@ -46,7 +45,7 @@ LLPluginProcessParentOwner::~LLPluginProcessParentOwner() bool LLPluginProcessParent::sUseReadThread = false; apr_pollset_t *LLPluginProcessParent::sPollSet = NULL; bool LLPluginProcessParent::sPollsetNeedsRebuild = false; -LLMutex *LLPluginProcessParent::sInstancesMutex; +LLCoros::Mutex *LLPluginProcessParent::sInstancesMutex; LLPluginProcessParent::mapInstances_t LLPluginProcessParent::sInstances; LLThread *LLPluginProcessParent::sReadThread = NULL; @@ -106,7 +105,7 @@ LLPluginProcessParent::LLPluginProcessParent(LLPluginProcessParentOwner *owner): { if(!sInstancesMutex) { - sInstancesMutex = new LLMutex(); + sInstancesMutex = new LLCoros::Mutex(); } mOwner = owner; @@ -176,7 +175,7 @@ LLPluginProcessParent::ptr_t LLPluginProcessParent::create(LLPluginProcessParent // Don't add to the global list until fully constructed. { - LLMutexLock lock(sInstancesMutex); + LLCoros::LockType lock(*sInstancesMutex); sInstances.insert(mapInstances_t::value_type(that.get(), that)); } @@ -186,7 +185,7 @@ LLPluginProcessParent::ptr_t LLPluginProcessParent::create(LLPluginProcessParent /*static*/ void LLPluginProcessParent::shutdown() { - LLMutexLock lock(sInstancesMutex); + LLCoros::LockType lock(*sInstancesMutex); mapInstances_t::iterator it; for (it = sInstances.begin(); it != sInstances.end(); ++it) @@ -244,7 +243,7 @@ bool LLPluginProcessParent::pollTick() { // 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); + LLCoros::LockType lock(*sInstancesMutex); mapInstances_t::iterator it = sInstances.find(this); if (it != sInstances.end()) that = (*it).second; @@ -263,7 +262,7 @@ 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); + LLCoros::LockType lock(*sInstancesMutex); { LLMutexLock lock2(&mIncomingQueueMutex); sInstances.erase(this); @@ -845,7 +844,7 @@ void LLPluginProcessParent::updatePollset() return; } - LLMutexLock lock(sInstancesMutex); + LLCoros::LockType lock(*sInstancesMutex); if(sPollSet) { @@ -968,7 +967,7 @@ void LLPluginProcessParent::poll(F64 timeout) mapInstances_t::iterator it; { - LLMutexLock lock(sInstancesMutex); + LLCoros::LockType lock(*sInstancesMutex); it = sInstances.find(thatId); if (it != sInstances.end()) that = (*it).second; -- cgit v1.3 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/llpluginprocessparent.cpp') 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.3