Remove call from LLAppViewer::cleanup().
Instead, make each LLSingleton<T>::deleteSingleton() call cleanupSingleton()
just before destroying the instance. Since deleteSingleton() is not a
destructor, it's fine to call cleanupSingleton() from there; and since
deleteAll() calls deleteSingleton() on every remaining instance, the former
cleanupAll() functionality has been subsumed into deleteAll().
Since cleanupSingleton() is now called at exactly one point in the instance's
lifetime, we no longer need a bool indicating whether it has been called.
The previous protocol of calling cleanupAll() before deleteAll() implemented a
two-phase cleanup strategy for the application. That is no longer needed.
Moreover, the cleanupAll() / deleteAll() sequence created a time window during
which individual LLSingleton<T> instances weren't usable (to the extent that
their cleanupSingleton() methods released essential resources) but still
existed -- so a getInstance() call would return the crippled instance rather
than recreating it.
Remove cleanupAll() calls from tests; adjust to new order of expected side
effects: instead of A::cleanupSingleton(), B::cleanupSingleton(), ~A(), ~B(),
now we get A::cleanupSingleton(), ~A(), B::cleanupSingleton(), ~B().
Use function-static LLMutex instances instead of module-static instances,
since some log calls are evidently issued before we get around to initializing
llerror.cpp module-static variables.
instead of deleteSingleton().
Specifically, clear static SingletonData and remove the instance from the
MasterList in the destructor.
Empirically, some consumers are manually deleting LLSingleton instances,
instead of calling deleteSingleton(). If deleteSingleton() handles cleanup
rather than the destructor, we're left with dangling pointers in the Master
List.
We don't also call cleanupSingleton() from the destructor because only
deleteSingleton() promises to call cleanupSingleton(). Hopefully whoever is
directly deleting an LLSingleton subclass instance isn't relying on
cleanupSingleton().
When calling LLParamSingleton::initParamSingleton() on a secondary thread, use
LLMainThreadTask::dispatch() to construct the instance on the main thread --
as with LLSingleton::getInstance().
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.
The Windows implementation of demangle() assumed that a "mangled" class name
produced by typeid(class).name() always starts with the prefix "class ",
checked for that and removed it. If the mangled name didn't start with that
prefix, it would emit a debug message and return the full name.
When the class in question is actually a struct, the prefix is "struct "
instead. But when demangle() was being called before logging had been fully
initialized, the debug message remarking that it didn't start with "class "
crashed.
Look for either "class " or "struct " prefix. Remove whichever is found and
return the rest of the name. If neither is found, only log if logging is
available.
If already running on the main thread, LLMaintThreadTask simply runs the work
inline. Otherwise it queues it for the main thread using LLEventTimer, using
std::future to retrieve the result.
LLThread::currentID() used to return a U32, a distinct unsigned value
incremented by explicitly constructing LLThread or by calling LLThread::
registerThreadID() early in a thread launched by other means. The latter
imposed an unobvious requirement on new code based on std::thread. Using
std:🧵:id instead delegates to the compiler/library the problem of
distinguishing threads launched by any means.
Change lots of explicit U32 declarations. Introduce LLThread::id_t typedef to
avoid having to run around fixing uses again if we later revisit this decision.
LLMutex, which stores an LLThread::id_t, wants a distinguished value meaning
NO_THREAD, and had an enum with that name. But as std:🧵:id promises
that the default-constructed value is distinct from every valid value,
NO_THREAD becomes unnecessary and goes away.
Because LLMutex now stores LLThread::id_t instead of U32, make llmutex.h
#include "llthread.h" instead of the other way around. This makes LLMutex an
incomplete type within llthread.h, so move LLThread::lockData() and
unlockData() to the .cpp file. Similarly, remove llrefcount.h's #include
"llmutex.h" to break circularity; instead forward-declare LLMutex.
It turns out that a number of source files assumed that #include "llthread.h"
would get the definition for LLMutex. Sprinkle #include "llmutex.h" as needed.
In the SAFE_SSL code in llcorehttp/httpcommon.cpp, there's an ssl_thread_id()
callback that returns an unsigned long to the SSL library. When LLThread::
currentID() was U32, we could simply return that. But std:🧵:id is very
deliberately opaque, and can't be reinterpret_cast to unsigned long.
Fortunately it can be hashed because std::hash is specialized with that type.
The pattern of requiring a lock to permit *any* access to a static instance of
something seems generally useful. Break out lockstatic.h; recast
LLInstanceTracker to use it.
Moving LockStatic to an external template class instead of a nested class in
LLInstanceTrackerBase leaves LLInstanceTrackerBase pretty empty. Get rid of it.
And *that* means we can move the definition of the StaticData used by each
LLInstanceTracker specialization into the class itself, rather than having to
define it beforehand in namespace LLInstanceTrackerStuff.
The CONSTRUCTED state was only briefly set between constructSingleton() and
finishInitializing(). But as no consumer code is executed between setting
CONSTRUCTED and setting INITIALIZING, it was impossible to reach the switch
statement in either getInstance() method in state CONSTRUCTED. So there was no
point in state CONSTRUCTED. Remove it.
With CONSTRUCTED gone, we only ever call finishInitializing() right after
constructSingleton(). Merge finishInitializing() into constructSingleton().
No need to capture a separate list of completed LLEventTimer instances to
delete after the primary loop, since at this point we're looping over a
snapshot and can directly delete each completed timer.
Remove warnings about LLSingleton not being thread-safe because, at this point,
we have devoted considerable effort to trying to make it thread-safe.
Add LLSingleton<T>::Locker, a nested class which both provides a function-
static mutex and a scoped lock that uses it. Instantiating Locker, which has a
nullary constructor, replaces the somewhat cumbersome idiom of declaring a
std::unique_lock<std::recursive_mutex> lk(getMutex);
This eliminates (or rather, absorbs) the typedefs and getMutex() method from
LLParamSingleton. Replace explicit std::unique_lock declarations in
LLParamSingleton methods with Locker declarations.
Remove LLSingleton<T>::SingletonInitializer nested struct. Instead of
getInstance() relying on function-static initialization to protect (only)
constructSingleton() calls, explicitly use a Locker instance to cover its
whole scope, and make the UNINITIALIZED case call constructSingleton().
Rearrange cases so that after constructSingleton(), control falls through to
the CONSTRUCTED case and the finishInitializing() call.
Use a Locker instance in other public-facing methods too: instanceExists(),
wasDeleted(), ~LLSingleton(). Make destructor protected so it can only be called
via deleteSingleton() (but must be accessible to subclasses for overrides).
Remove LLSingletonBase::get_master() and get_initializing(), which permitted
directly manipulating the master list and the initializing stack without any
locking mechanism. Replace with get_initializing_size().
Similarly, replace LLSingleton_manage_master::get_initializing() with
get_initializing_size(). Use in constructSingleton() in place of
get_initializing().size().
Remove LLSingletonBase::capture_dependency()'s list_t parameter, which
accepted the list returned by get_initializing(). Encapsulate that retrieval
within the scope of the new lock in capture_dependency().
Add LLSingleton_manage_master::capture_dependency(LLSingletonBase*, EInitState)
to forward (or not) a call to LLSingletonBase::capture_dependency(). Nullary
LLSingleton<T>::capture_dependency() calls new LLSingleton_manage_master method.
Equip LLSingletonBase::MasterList with a mutex of its own, separate from the
one donated by the LLSingleton machinery, to serialize use of MasterList data
members. Introduce MasterList::Lock nested class to lock the MasterList mutex
while providing a reference to the MasterList instance. Introduce subclasses
LockedMaster, which provides a reference to the actual mMaster master list
while holding the MasterList lock; and LockedInitializing, which does the same
for the initializing list. Make mMaster and get_initializing_() private so
that consuming code can *only* access those lists via LockedInitializing and
LockedMaster.
Make MasterList::cleanup_initializing_() private, with a LockedInitializing
public forwarding method. This avoids another call to MasterList::instance(),
and also mandates that the lock is currently held during every call.
Similarly, move LLSingletonBase::log_initializing() to a LockedInitializing
log() method.
(transplanted from dca0f16266c7bddedb51ae7d7dca468ba87060d5)
The previous implementation went to some effort to crash if anyone attempted
to create or destroy an LLInstanceTracker subclass instance during traversal.
That restriction is manageable within a single thread, but becomes unworkable
if it's possible that a given subclass might be used on more than one thread.
Remove LLInstanceTracker::instance_iter, beginInstances(), endInstances(),
also key_iter, beginKeys() and endKeys(). Instead, introduce key_snapshot()
and instance_snapshot(), the only means of iterating over LLInstanceTracker
instances. (These are intended to resemble functions, but in fact the current
implementation simply presents the classes.) Iterating over a captured
snapshot defends against container modifications during traversal. The term
'snapshot' reminds the coder that a new instance created during traversal will
not be considered. To defend against instance deletion during traversal, a
snapshot stores std::weak_ptrs which it lazily dereferences, skipping on the
fly any that have expired.
Dereferencing instance_snapshot::iterator gets you a reference rather than a
pointer. Because some use cases want to delete all existing instances, add an
instance_snapshot::deleteAll() method that extracts the pointer. Those cases
used to require explicitly copying instance pointers into a separate
container; instance_snapshot() now takes care of that. It remains the caller's
responsibility to ensure that all instances of that LLInstanceTracker subclass
were allocated on the heap.
Replace unkeyed static LLInstanceTracker::getInstance(T*) -- which returned
nullptr if that instance had been destroyed -- with new getWeak() method
returning std::weak_ptr<T>. Caller must detect expiration of that weak_ptr.
Adjust tests accordingly.
Use of std::weak_ptr to detect expired instances requires engaging
std::shared_ptr in the constructor. We now store shared_ptrs in the static
containers (std::map for keyed, std::set for unkeyed).
Make LLInstanceTrackerBase a template parameterized on the type of the static
data it manages. For that reason, hoist static data class declarations out of
the class definitions to an LLInstanceTrackerStuff namespace.
Remove the static atomic sIterationNestDepth and its methods incrementDepth(),
decrementDepth() and getDepth(), since they were used only to forbid creation
and destruction during traversal.
Add a std::mutex to static data. Introduce an internal LockStatic class that
locks the mutex while providing a pointer to static data, making that the only
way to access the static data.
The LLINSTANCETRACKER_DTOR_NOEXCEPT macro goes away because we no longer
expect ~LLInstanceTracker() to throw an exception in test programs.
That affects LLTrace::StatBase as well as LLInstanceTracker itself.
Adapt consumers to the new LLInstanceTracker API.
Moderately often I want to copy the (long) integration test program path from
build output and rerun the test program by hand. But typically we need
environment variables set as well so it can find its dynamic libraries. This
has resulted in my copying parts of several lines of build output, then
pasting to a command prompt, then hand-tweaking the pasted text so it makes
sense as a command.
Streamline run_build_test.py output so less hand-tweaking is needed.
Nat Goodspeed