summaryrefslogtreecommitdiff
path: root/indra/llcommon/llsingleton.cpp
diff options
context:
space:
mode:
authorNat Goodspeed <nat@lindenlab.com>2019-12-12 16:14:38 -0500
committerNat Goodspeed <nat@lindenlab.com>2020-03-25 15:28:17 -0400
commita6f5e55d42f417ea8bc565e473354b64c192802d (patch)
tree03f0127408a05518a8189f9a7b33a48c1058d35f /indra/llcommon/llsingleton.cpp
parent1fc7c994d6232e373fc9f36e72ed9855d4d7cd76 (diff)
DRTVWR-494: Dispatch all LLSingleton construction to the main thread.
Given the viewer's mutually-dependent LLSingletons, given that different threads might simultaneously request different LLSingletons from such a chain of circular dependencies, the key to avoiding deadlock is to serialize all LLSingleton construction on one thread: the main thread. Add comments to LLSingleton::getInstance() explaining the problem and the solution. Recast LLSingleton's static SingletonData to use LockStatic. Instead of using Locker, and simply trusting that every reference to sData is within the dynamic scope of a Locker instance, LockStatic enforces that: you can only access SingletonData members via LockStatic. Reorganize the switch in getInstance() to group the CONSTRUCTING error, the INITIALIZING/INITIALIZED success case, and the DELETED/UNINITIALIZED construction case. When [re]constructing an instance, on the main thread, retain the lock and call constructSingleton() (and capture_dependency()) directly. On a secondary thread, unlock LockStatic and use LLMainThreadTask::dispatch() to call getInstance() on the main thread. Since we might end up enqueuing multiple such tasks, it's important to let getInstance() notice when the instance has already been constructed and simply return the existing pointer. Add loginfos() method, sibling to logerrs(), logwarns() and logdebugs(). Produce loginfos() messages when dispatching to the main thread, when actually running on the main thread and when resuming the suspended requesting thread. Make deleteSingleton() manage all associated state, instead of delegating some of that work to ~LLSingleton(). Now, within LockStatic, extract the instance pointer and set state to DELETED; that lets subsequent code, which retains the only remaining pointer to the instance, remove the master-list entry, call the subclass cleanupSingleton() and destructor without needing to hold the lock. In fact, entirely remove ~LLSingleton(). Import LLSingletonBase::cleanup_() method to wrap the call to subclass cleanupSingleton() in try/catch. Remove cleanupAll() calls from llsingleton_test.cpp, and reorder the success cases to reflect the fact that T::cleanupSingleton() is called immediately before ~T() for each distinct LLSingleton subclass T. When getInstance() on a secondary thread dispatches to the main thread, it necessarily unlocks its LockStatic lock. But an LLSingleton dependency chain strongly depends on the function stack on which getInstance() is invoked -- the task dispatched to the main thread doesn't know the dependencies tracked on the requesting thread stack. So, once the main thread delivers the instance pointer, the requesting thread captures its own dependencies for that instance. Back in the requesting thread, obtaining the current EInitState to pass to capture_dependencies() would have required relocking LockStatic. Instead, I've convinced myself that (a) capture_dependencies() only wanted to know EInitState to produce an error for CONSTRUCTING, and (b) in CONSTRUCTING state, we never get as far as capture_dependencies() because getInstance() produces an error first. Eliminate the EInitState parameter from all capture_dependencies() methods. Remove the LLSingletonBase::capture_dependency() stanza that tested EInitState. Make the capture_dependencies() variants that accepted LockStatic instead accept LLSingletonBase*. That lets getInstance(), in the LLMainThreadTask case, pass the newly-returned instance pointer. For symmetry, make pop_initializing() accept LLSingletonBase* as well, instead of accepting LockStatic and extracting mInstance.
Diffstat (limited to 'indra/llcommon/llsingleton.cpp')
-rw-r--r--indra/llcommon/llsingleton.cpp36
1 files changed, 21 insertions, 15 deletions
diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp
index 812fd31719..bf594f122c 100644
--- a/indra/llcommon/llsingleton.cpp
+++ b/indra/llcommon/llsingleton.cpp
@@ -302,7 +302,7 @@ void LLSingletonBase::MasterList::LockedInitializing::log(const char* verb, cons
}
}
-void LLSingletonBase::capture_dependency(EInitState initState)
+void LLSingletonBase::capture_dependency()
{
MasterList::LockedInitializing locked_list;
list_t& initializing(locked_list.get());
@@ -334,21 +334,8 @@ void LLSingletonBase::capture_dependency(EInitState initState)
LLSingletonBase* foundp(*found);
out << classname(foundp) << " -> ";
}
- // We promise to capture dependencies from both the constructor
- // and the initSingleton() method, so an LLSingleton's instance
- // pointer is on the initializing list during both. Now that we've
- // detected circularity, though, we must distinguish the two. If
- // the recursive call is from the constructor, we CAN'T honor it:
- // otherwise we'd be returning a pointer to a partially-
- // constructed object! But from initSingleton() is okay: that
- // method exists specifically to support circularity.
// Decide which log helper to call.
- if (initState == CONSTRUCTING)
- {
- logerrs("LLSingleton circularity in Constructor: ", out.str().c_str(),
- classname(this).c_str(), "");
- }
- else if (it_next == initializing.end())
+ if (it_next == initializing.end())
{
// Points to self after construction, but during initialization.
// Singletons can initialize other classes that depend onto them,
@@ -457,6 +444,19 @@ void LLSingletonBase::cleanupAll()
}
}
+void LLSingletonBase::cleanup_()
+{
+ logdebugs("calling ", classname(this).c_str(), "::cleanupSingleton()");
+ try
+ {
+ cleanupSingleton();
+ }
+ catch (...)
+ {
+ LOG_UNHANDLED_EXCEPTION(classname(this) + "::cleanupSingleton()");
+ }
+}
+
//static
void LLSingletonBase::deleteAll()
{
@@ -537,6 +537,12 @@ void LLSingletonBase::logwarns(const char* p1, const char* p2, const char* p3, c
}
//static
+void LLSingletonBase::loginfos(const char* p1, const char* p2, const char* p3, const char* p4)
+{
+ log(LLError::LEVEL_INFO, p1, p2, p3, p4);
+}
+
+//static
void LLSingletonBase::logerrs(const char* p1, const char* p2, const char* p3, const char* p4)
{
log(LLError::LEVEL_ERROR, p1, p2, p3, p4);