livie
/
phoenix-firestorm
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

#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
master
RunitaiLinden 2024-05-02 10:57:39 -05:00 committed by GitHub
parent 9b6979f458
commit 7fc5f7e649
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
23 changed files with 143 additions and 109 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
indra/llcommon/llcommon.cpp
View File

@ -128,8 +128,7 @@ void LLCommon::initClass()
sAprInitialized = TRUE;
}
LLTimer::initClass();
LLThreadSafeRefCount::initThreadSafeRefCount();
assert_main_thread(); // Make sure we record the main thread
assert_main_thread(); // Make sure we record the main thread
if (!sMasterThreadRecorder)
{
sMasterThreadRecorder = new LLTrace::ThreadRecorder();
@ -143,7 +142,6 @@ void LLCommon::cleanupClass()
delete sMasterThreadRecorder;
sMasterThreadRecorder = NULL;
LLTrace::set_master_thread_recorder(NULL);
LLThreadSafeRefCount::cleanupThreadSafeRefCount();
SUBSYSTEM_CLEANUP_DBG(LLTimer);
if (sAprInitialized)
{

17
indra/llcommon/llcoros.cpp
View File

@ -61,6 +61,23 @@
#include <excpt.h>
#endif
// static
bool LLCoros::on_main_coro()
{
if (!LLCoros::instanceExists() || LLCoros::getName().empty())
{
return true;
}
return false;
}
// static
bool LLCoros::on_main_thread_main_coro()
{
return on_main_coro() && on_main_thread();
}
// static
LLCoros::CoroData& LLCoros::get_CoroData(const std::string& caller)
{

10
indra/llcommon/llcoros.h
View File

@ -94,6 +94,16 @@ class LL_COMMON_API LLCoros: public LLSingleton<LLCoros>
void cleanupSingleton() override;
public:
// For debugging, return true if on the main coroutine for the current thread
// Code that should not be executed from a coroutine should be protected by
// llassert(LLCoros::on_main_coro())
static bool on_main_coro();
// For debugging, return true if on the main thread and not in a coroutine
// Non-thread-safe code in the main loop should be protected by
// llassert(LLCoros::on_main_thread_main_coro())
static bool on_main_thread_main_coro();
/// The viewer's use of the term "coroutine" became deeply embedded before
/// the industry term "fiber" emerged to distinguish userland threads from
/// simpler, more transient kinds of coroutines. Semantically they've

12
indra/llcommon/llerror.cpp
View File

@ -506,7 +506,7 @@ namespace
LLError::TimeFunction mTimeFunction;
Recorders mRecorders;
LLMutex mRecorderMutex;
LLCoros::Mutex mRecorderMutex;
int mShouldLogCallCounter;
@ -1044,7 +1044,7 @@ namespace LLError
return;
}
SettingsConfigPtr s = Globals::getInstance()->getSettingsConfig();
LLMutexLock lock(&s->mRecorderMutex);
LLCoros::LockType lock(s->mRecorderMutex);
s->mRecorders.push_back(recorder);
}
@ -1055,7 +1055,7 @@ namespace LLError
return;
}
SettingsConfigPtr s = Globals::getInstance()->getSettingsConfig();
LLMutexLock lock(&s->mRecorderMutex);
LLCoros::LockType lock(s->mRecorderMutex);
s->mRecorders.erase(std::remove(s->mRecorders.begin(), s->mRecorders.end(), recorder),
s->mRecorders.end());
}
@ -1104,7 +1104,7 @@ namespace LLError
std::shared_ptr<RECORDER> findRecorder()
{
SettingsConfigPtr s = Globals::getInstance()->getSettingsConfig();
LLMutexLock lock(&s->mRecorderMutex);
LLCoros::LockType lock(s->mRecorderMutex);
return findRecorderPos<RECORDER>(s).first;
}
@ -1115,7 +1115,7 @@ namespace LLError
bool removeRecorder()
{
SettingsConfigPtr s = Globals::getInstance()->getSettingsConfig();
LLMutexLock lock(&s->mRecorderMutex);
LLCoros::LockType lock(s->mRecorderMutex);
auto found = findRecorderPos<RECORDER>(s);
if (found.first)
{
@ -1221,7 +1221,7 @@ namespace
std::string escaped_message;
LLMutexLock lock(&s->mRecorderMutex);
LLCoros::LockType lock(s->mRecorderMutex);
for (LLError::RecorderPtr& r : s->mRecorders)
{
if (!r->enabled())

10
indra/llcommon/llevents.cpp
View File

@ -382,7 +382,7 @@ std::string LLEventPump::inventName(const std::string& pfx)
void LLEventPump::clear()
{
LLMutexLock lock(&mConnectionListMutex);
LLCoros::LockType lock(mConnectionListMutex);
// Destroy the original LLStandardSignal instance, replacing it with a
// whole new one.
mSignal = std::make_shared<LLStandardSignal>();
@ -394,7 +394,7 @@ void LLEventPump::reset()
{
// Resetting mSignal is supposed to disconnect everything on its own
// But due to crash on 'reset' added explicit cleanup to get more data
LLMutexLock lock(&mConnectionListMutex);
LLCoros::LockType lock(mConnectionListMutex);
ConnectionMap::const_iterator iter = mConnections.begin();
ConnectionMap::const_iterator end = mConnections.end();
while (iter!=end)
@ -419,7 +419,7 @@ LLBoundListener LLEventPump::listen_impl(const std::string& name, const LLEventL
return LLBoundListener();
}
LLMutexLock lock(&mConnectionListMutex);
LLCoros::LockType lock(mConnectionListMutex);
float nodePosition = 1.0;
@ -582,7 +582,7 @@ LLBoundListener LLEventPump::listen_impl(const std::string& name, const LLEventL
LLBoundListener LLEventPump::getListener(const std::string& name)
{
LLMutexLock lock(&mConnectionListMutex);
LLCoros::LockType lock(mConnectionListMutex);
ConnectionMap::const_iterator found = mConnections.find(name);
if (found != mConnections.end())
{
@ -594,7 +594,7 @@ LLBoundListener LLEventPump::getListener(const std::string& name)
void LLEventPump::stopListening(const std::string& name)
{
LLMutexLock lock(&mConnectionListMutex);
LLCoros::LockType lock(mConnectionListMutex);
ConnectionMap::iterator found = mConnections.find(name);
if (found != mConnections.end())
{

3
indra/llcommon/llevents.h
View File

@ -61,6 +61,7 @@
#include "llstl.h"
#include "llexception.h"
#include "llhandle.h"
#include "llcoros.h"
/*==========================================================================*|
// override this to allow binding free functions with more parameters
@ -601,7 +602,7 @@ private:
LLHandle<LLEventPumps> mRegistry;
std::string mName;
LLMutex mConnectionListMutex;
LLCoros::Mutex mConnectionListMutex;
protected:
virtual LLBoundListener listen_impl(const std::string& name, const LLEventListener&,

3
indra/llcommon/llfixedbuffer.h
View File

@ -33,6 +33,7 @@
#include "llstring.h"
#include "llthread.h"
#include "llerrorcontrol.h"
#include "llcoros.h"
// fixed buffer implementation
class LL_COMMON_API LLFixedBuffer : public LLLineBuffer
@ -58,7 +59,7 @@ protected:
void addWLine(const LLWString& line);
protected:
LLMutex mMutex ;
LLCoros::Mutex mMutex ;
};
#endif //LL_FIXED_BUFFER_H

18
indra/llcommon/llmutex.cpp
View File

@ -28,6 +28,7 @@
#include "llmutex.h"
#include "llthread.h"
#include "lltimer.h"
#include "llcoros.h"
//============================================================================
@ -44,7 +45,17 @@ LLMutex::~LLMutex()
void LLMutex::lock()
{
LL_PROFILE_ZONE_SCOPED_CATEGORY_THREAD
LL_PROFILE_ZONE_SCOPED_CATEGORY_THREAD;
// LLMutex is not coroutine aware and should not be used from a coroutine
// If your code is running in a coroutine, you should use LLCoros::Mutex instead
// NOTE: If the stack trace you're staring at contains non-thread-safe code,
// you should use LLAppViewer::instance().postToMainThread() to shuttle execution
// back to the main loop.
// NOTE: If you got here from seeing this assert in your log and you're not seeing
// a stack trace that points here, put a breakpoint in on_main_coro and try again.
llassert(LLCoros::on_main_coro());
if(isSelfLocked())
{ //redundant lock
mCount++;
@ -66,7 +77,8 @@ void LLMutex::lock()
void LLMutex::unlock()
{
LL_PROFILE_ZONE_SCOPED_CATEGORY_THREAD
LL_PROFILE_ZONE_SCOPED_CATEGORY_THREAD;
if (mCount > 0)
{ //not the root unlock
mCount--;
@ -111,7 +123,7 @@ LLThread::id_t LLMutex::lockingThread() const
bool LLMutex::trylock()
{
LL_PROFILE_ZONE_SCOPED_CATEGORY_THREAD
LL_PROFILE_ZONE_SCOPED_CATEGORY_THREAD;
if(isSelfLocked())
{ //redundant lock
mCount++;

7
indra/llcommon/llrefcount.h
View File

@ -89,13 +89,6 @@ private:
class LL_COMMON_API LLThreadSafeRefCount
{
public:
static void initThreadSafeRefCount(); // creates sMutex
static void cleanupThreadSafeRefCount(); // destroys sMutex
private:
static LLMutex* sMutex;
protected:
virtual ~LLThreadSafeRefCount(); // use unref()

24
indra/llcommon/llthread.cpp
View File

@ -421,30 +421,6 @@ void LLThread::unlockData()
//============================================================================
//----------------------------------------------------------------------------
//static
LLMutex* LLThreadSafeRefCount::sMutex = 0;
//static
void LLThreadSafeRefCount::initThreadSafeRefCount()
{
if (!sMutex)
{
sMutex = new LLMutex();
}
}
//static
void LLThreadSafeRefCount::cleanupThreadSafeRefCount()
{
delete sMutex;
sMutex = NULL;
}
//----------------------------------------------------------------------------
LLThreadSafeRefCount::LLThreadSafeRefCount() :
mRef(0)
{

32
indra/llmessage/llavatarnamecache.cpp
View File

@ -45,6 +45,7 @@
#include "llcorehttputil.h"
#include "llexception.h"
#include "stringize.h"
#include "workqueue.h"
#include <map>
#include <set>
@ -179,19 +180,26 @@ void LLAvatarNameCache::requestAvatarNameCache_(std::string url, std::vector<LLU
if (LLAvatarNameCache::instanceExists())
{
if (!success)
{ // on any sort of failure add dummy records for any agent IDs
// in this request that we do not have cached already
std::vector<LLUUID>::const_iterator it = agentIds.begin();
for (; it != agentIds.end(); ++it)
{
const LLUUID& agent_id = *it;
LLAvatarNameCache::getInstance()->handleAgentError(agent_id);
}
return;
}
// dispatch handler execution back to mainloop
auto workqueue = LL::WorkQueue::getInstance("mainloop");
LLAvatarNameCache::getInstance()->handleAvNameCacheSuccess(results, httpResults);
if (workqueue)
{
workqueue->post([=]()
{
if (!success)
{ // on any sort of failure add dummy records for any agent IDs
// in this request that we do not have cached already
for (const auto& agent_id : agentIds)
{
LLAvatarNameCache::getInstance()->handleAgentError(agent_id);
}
return;
}
LLAvatarNameCache::getInstance()->handleAvNameCacheSuccess(results, httpResults);
});
}
}
}
catch (const LLCoros::Stop&)

17
indra/llplugin/llpluginprocessparent.cpp
View File

@ -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;

2
indra/llplugin/llpluginprocessparent.h
View File

@ -207,7 +207,7 @@ private:
apr_pollfd_t mPollFD;
static apr_pollset_t *sPollSet;
static bool sPollsetNeedsRebuild;
static LLMutex *sInstancesMutex;
static LLCoros::Mutex *sInstancesMutex;
static mapInstances_t sInstances;
static void dirtyPollSet();
static void updatePollset();

2
indra/llui/llconsole.cpp
View File

@ -380,7 +380,7 @@ void LLConsole::updateClass()
void LLConsole::update()
{
{
LLMutexLock lock(&mMutex);
LLCoros::LockType lock(mMutex);
while (!mLines.empty())
{

5
indra/newview/llappviewer.cpp
View File

@ -5200,6 +5200,11 @@ void LLAppViewer::updateNameLookupUrl(const LLViewerRegion * regionp)
}
}
void LLAppViewer::postToMainCoro(const LL::WorkQueue::Work& work)
{
gMainloopWork.post(work);
}
void LLAppViewer::idleNameCache()
{
// Neither old nor new name cache can function before agent has a region

3
indra/newview/llappviewer.h
View File

@ -226,6 +226,9 @@ public:
void updateNameLookupUrl(const LLViewerRegion* regionp);
// post given work to the "mainloop" work queue for handling on the main thread
void postToMainCoro(const LL::WorkQueue::Work& work);
protected:
virtual bool initWindow(); // Initialize the viewer's window.
virtual void initLoggingAndGetLastDuration(); // Initialize log files, logging system

2
indra/newview/llavatarrenderinfoaccountant.cpp
View File

@ -236,8 +236,6 @@ void LLAvatarRenderInfoAccountant::avatarRenderInfoReportCoro(std::string url, U
!avatar->isControlAvatar() && // Not part of an animated object
avatar->getObjectHost() == regionp->getHost()) // Ensure it's on the same region
{
avatar->calculateUpdateRenderComplexity(); // Make sure the numbers are up-to-date
LLSD info = LLSD::emptyMap();
U32 avatar_complexity = avatar->getVisualComplexity();
if (avatar_complexity > 0)

2
indra/newview/lldrawpoolbump.cpp
View File

@ -699,6 +699,8 @@ void LLBumpImageList::addTextureStats(U8 bump, const LLUUID& base_image_id, F32
void LLBumpImageList::updateImages()
{
llassert(LLCoros::on_main_thread_main_coro()); // This code is not thread safe
for (bump_image_map_t::iterator iter = mBrightnessEntries.begin(); iter != mBrightnessEntries.end(); )
{
LLViewerTexture* image = iter->second;

14
indra/newview/llstartup.cpp
View File

@ -334,10 +334,18 @@ void update_texture_fetch()
void set_flags_and_update_appearance()
{
LLAppearanceMgr::instance().setAttachmentInvLinkEnable(true);
LLAppearanceMgr::instance().updateAppearanceFromCOF(true, true, no_op);
// this may be called from a coroutine but has many side effects
// in non-thread-safe classes, post to main loop
auto work = []()
{
LLAppearanceMgr::instance().setAttachmentInvLinkEnable(true);
LLAppearanceMgr::instance().updateAppearanceFromCOF(true, true, no_op);
LLInventoryModelBackgroundFetch::instance().start();
};
LLAppViewer::instance()->postToMainCoro(work);
LLInventoryModelBackgroundFetch::instance().start();
}
// Returns false to skip other idle processing. Should only return

4
indra/newview/llviewermedia.cpp
View File

@ -1667,7 +1667,7 @@ void LLViewerMediaImpl::destroyMediaSource()
cancelMimeTypeProbe();
{
LLMutexLock lock(&mLock); // Delay tear-down while bg thread is updating
LLCoros::LockType lock(mLock); // Delay tear-down while bg thread is updating
if(mMediaSource)
{
mMediaSource->setDeleteOK(true) ;
@ -2968,7 +2968,7 @@ bool LLViewerMediaImpl::preMediaTexUpdate(LLViewerMediaTexture*& media_tex, U8*&
void LLViewerMediaImpl::doMediaTexUpdate(LLViewerMediaTexture* media_tex, U8* data, S32 data_width, S32 data_height, S32 x_pos, S32 y_pos, S32 width, S32 height, bool sync)
{
LL_PROFILE_ZONE_SCOPED_CATEGORY_MEDIA;
LLMutexLock lock(&mLock); // don't allow media source tear-down during update
LLCoros::LockType lock(mLock); // don't allow media source tear-down during update
// wrap "data" in an LLImageRaw but do NOT make a copy
LLPointer<LLImageRaw> raw = new LLImageRaw(data, media_tex->getWidth(), media_tex->getHeight(), media_tex->getComponents(), true);

4
indra/newview/llviewermedia.h
View File

@ -182,7 +182,7 @@ private:
// Implementation functions not exported into header file
class LLViewerMediaImpl
: public LLMouseHandler, public LLRefCount, public LLPluginClassMediaOwner, public LLViewerMediaEventEmitter, public LLEditMenuHandler
: public LLMouseHandler, public LLThreadSafeRefCount, public LLPluginClassMediaOwner, public LLViewerMediaEventEmitter, public LLEditMenuHandler
{
LOG_CLASS(LLViewerMediaImpl);
public:
@ -432,7 +432,7 @@ private:
private:
// a single media url with some data and an impl.
std::shared_ptr<LLPluginClassMedia> mMediaSource;
LLMutex mLock;
LLCoros::Mutex mLock;
F64 mZoomFactor;
LLUUID mTextureId;
bool mMovieImageHasMips;

7
indra/newview/llviewerregion.cpp
View File

@ -2498,11 +2498,8 @@ void LLViewerRegion::setSimulatorFeatures(const LLSD& sim_features)
}
};
auto workqueue = LL::WorkQueue::getInstance("mainloop");
if (workqueue)
{
LL::WorkQueue::postMaybe(workqueue, work);
}
LLAppViewer::instance()->postToMainCoro(work);
}
//this is called when the parent is not cacheable.

50
indra/newview/llvoicevivox.cpp
View File

@ -6437,37 +6437,43 @@ void LLVivoxVoiceClient::notifyStatusObservers(LLVoiceClientStatusObserver::ESta
}
}
}
LL_DEBUGS("Voice")
<< " " << LLVoiceClientStatusObserver::status2string(status)
<< ", session URI " << getAudioSessionURI()
<< ", proximal is " << inSpatialChannel()
<< LL_ENDL;
for (status_observer_set_t::iterator it = mStatusObservers.begin();
it != mStatusObservers.end();
)
{
LLVoiceClientStatusObserver* observer = *it;
observer->onChange(status, getAudioSessionURI(), inSpatialChannel());
// In case onError() deleted an entry.
it = mStatusObservers.upper_bound(observer);
}
// this function is called from a coroutine, shuttle application hook back to main loop
auto work = [=]()
{
for (status_observer_set_t::iterator it = mStatusObservers.begin();
it != mStatusObservers.end();
)
{
LLVoiceClientStatusObserver* observer = *it;
observer->onChange(status, getAudioSessionURI(), inSpatialChannel());
// In case onError() deleted an entry.
it = mStatusObservers.upper_bound(observer);
}
// skipped to avoid speak button blinking
if ( status != LLVoiceClientStatusObserver::STATUS_JOINING
&& status != LLVoiceClientStatusObserver::STATUS_LEFT_CHANNEL
&& status != LLVoiceClientStatusObserver::STATUS_VOICE_DISABLED)
{
bool voice_status = LLVoiceClient::getInstance()->voiceEnabled() && LLVoiceClient::getInstance()->isVoiceWorking();
// skipped to avoid speak button blinking
if (status != LLVoiceClientStatusObserver::STATUS_JOINING
&& status != LLVoiceClientStatusObserver::STATUS_LEFT_CHANNEL
&& status != LLVoiceClientStatusObserver::STATUS_VOICE_DISABLED)
{
bool voice_status = LLVoiceClient::getInstance()->voiceEnabled() && LLVoiceClient::getInstance()->isVoiceWorking();
gAgent.setVoiceConnected(voice_status);
gAgent.setVoiceConnected(voice_status);
if (voice_status)
{
LLFirstUse::speak(true);
}
}
if (voice_status)
{
LLFirstUse::speak(true);
}
}
};
LLAppViewer::instance()->postToMainCoro(work);
}
void LLVivoxVoiceClient::addObserver(LLFriendObserver* observer)