- Jul 30, 2021
-
-
Rye Mutt authored
-
- May 12, 2021
-
-
Nat Goodspeed authored
This is somewhat more expensive for string literals, but switching to std::string_view implies more extensive changes, to be considered separately.
-
Nat Goodspeed authored
-
Nat Goodspeed authored
Introduce 'string_params' typedef for std::initialization_list<std::string>, and make logwarns(), loginfos(), logdebugs() and logerrs() accept const string_params&. Eliminate the central log() function in llsingleton.cpp that used LL_VLOGS(). To cache the result of a (moderately expensive) Log::shouldLog() call, LL_VLOGS() wants its CallSite object to be static -- but of course the shouldLog() result will differ for different ELevel values, so LL_VLOGS() instantiates a static array of CallSite instances. It seems silly to funnel distinct logwarns(), etc., functions through a common log() function only to have LL_VLOGS() tease apart ELevel values again. Instead, make logwarns() directly invoke LL_WARNS(), and similarly for the rest. To reduce boilerplate in these distinct functions, teach std::ostream how to stream a string_params instance by looping over its elements. Then each logwarns(), etc., function can simply stream its string_params argument to LL_WARNS() or whichever. In particular, eliminate the LLERROR_CRASH macro in logerrs(). The fact that it invokes LL_ERRS() ensures that its LL_ENDL macro will crash the viewer.
-
- May 10, 2021
-
-
Nat Goodspeed authored
Instead of accepting a fixed list of (const char* p1="", etc.), accept (std::initializer_list<std::string_view>). Accepting a std::initializer_list<T> in your parameter list allows coding (e.g.) func({T0, T1, T2, ... }); -- in other words, you can pass the initializer_list a brace-enclosed list of an arbitrary number of instances of T. Using std::string_view instead of const char* means we can pass *either* const char* or std::string. string_view is cheaply constructed from either, allowing uniform treatment within the function. Constructing string_view from std::string simply extracts the pointer and length from the std::string. Constructing string_view from const char* (e.g. a "string literal") requires scanning the string for its terminating nul byte -- but that would be necessary even if the scan were deferred until the function body. Since string_view stores the length, the scan still happens only once.
-
- Jan 25, 2021
-
-
Rye Mutt authored
-
- Oct 20, 2020
-
-
Rye Mutt authored
Replace std::recursive_mutex with LLMutex inside LLSingleton as the recursive mutex impl to avoid virtual dispatch overhead of std::mutex on msvc
-
- Aug 24, 2020
-
-
Rye Mutt authored
-
- Jul 27, 2020
-
-
Rye Mutt authored
-
- Apr 03, 2020
-
-
Nat Goodspeed authored
-
- Mar 25, 2020
-
-
Nat Goodspeed authored
-
Nat Goodspeed authored
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().
-
Nat Goodspeed authored
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().
-
Nat Goodspeed authored
-
Nat Goodspeed authored
When calling LLParamSingleton::initParamSingleton() on a secondary thread, use LLMainThreadTask::dispatch() to construct the instance on the main thread -- as with LLSingleton::getInstance().
-
Nat Goodspeed authored
So does LLLockedSingleton<T>::construct().
-
Nat Goodspeed authored
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.
-
Nat Goodspeed authored
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().
-
Nat Goodspeed authored
-
Nat Goodspeed authored
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)
-
- Aug 20, 2019
-
-
Nat Goodspeed authored
An exception in the LLSingleton subclass constructor, or in its initSingleton() method, could leave the LLSingleton machinery in a bad state: the failing instance would remain in the MasterList, also on the stack of initializing LLSingletons. Catch exceptions in either and perform relevant cleanup. This problem is highlighted by test programs, in which LL_ERRS throws an exception rather than crashing the whole process. In the relevant catch clauses, clean up the initializing stack BEFORE logging. Otherwise we get tangled up recording bogus dependencies. Move capture_dependency() out of finishInitializing(): it must be called by every valid getInstance() call, both from LLSingleton and LLParamSingleton. Introduce new CONSTRUCTED EInitState value to distinguish "have called the constructor but not yet the initSingleton() method" from "currently within initSingleton() method." This is transient, but we execute the 'switch' on state within that moment. One could argue that the previous enum used INITIALIZING for current CONSTRUCTED, and INITIALIZED meant INITIALIZING too, but this is clearer. Introduce template LLSingletonBase::classname() helper methods to clarify verbose demangle(typeid(stuff).name()) calls. Similarly, introduce LLSingleton::pop_initializing() shorthand method.
-
- Aug 19, 2019
-
-
Nat Goodspeed authored
Add try/catch clauses to constructSingleton() (to catch exceptions in the subclass constructor) and finishInitializing() (to catch exceptions in the subclass initSingleton() method). Each of these catch clauses rethrows the exception -- they're for cleanup, not for ultimate handling. Introduce LLSingletonBase::reset_initializing(list_t::size_t). The idea is that since we can't know whether the exception happened before or after the push_initializing() call in LLSingletonBase's constructor, we can't just pop the stack. Instead, constructSingleton() captures the stack size before attempting to construct the new LLSingleton subclass. On exception, it calls reset_initializing() to restore the stack to that size. Naturally that requires a corresponding LLSingleton_manage_master method, whose MasterList specialization is a no-op. finishInitializing()'s exception handling is a bit simpler because it has a constructed LLSingleton subclass instance in hand, therefore push_initializing() has definitely been called, therefore it can call pop_initializing(). Break out new static capture_dependency() method from finishInitializing() because, in the previous LLSingleton::getInstance() implementation, the logic now wrapped in capture_dependency() was reached even in the INITIALIZED case. TODO: Add a new EInitState to differentiate "have been constructed, now calling initSingleton()" from "fully initialized, normal case" -- in the latter control path we should not be calling capture_dependency(). The LLSingleton_manage_master<LLSingletonBase::MasterList> specialization's get_initializing() function (which called get_initializing_from()) was potentially dangerous. get_initializing() is called by push_initializing(), which (in the general case) is called by LLSingletonBase's constructor. If somehow the MasterList's LLSingletonBase constructor ended up calling get_initializing(), it would have called get_initializing_from(), passing an LLSingletonBase which had not yet been constructed into the MasterList. In particular, its mInitializing map would not yet have been initialized at all. Since the MasterList must not, by design, depend on any other LLSingletons, LLSingleton_manage_master<LLSingletonBase::MasterList>::get_initializing() need not return a list from the official mInitializing map anyway. It can, and should, and now does, return a static dummy list. That obviates get_initializing_from(), which is removed. That in turn means we no longer need to pass get_initializing() an LLSingletonBase*. Remove that parameter.
-
- Aug 14, 2019
-
-
Nat Goodspeed authored
LLParamSingleton contained a static member mutex. Unfortunately that wasn't guaranteed to be initialized by the time its getInstance() was entered. Use a function-local static instead.
-
- Aug 12, 2019
-
-
Nat Goodspeed authored
from LLParamSingleton::initSingleton().
-
Nat Goodspeed authored
This was forbidden, but AndreyK points out cases in which LLParamSingleton:: initSingleton() should in fact be allowed to circle back to its own instance() method. Use a recursive_mutex instead of plain mutex to permit that; remove LL_ERRS preventing it. Add LLParamSingleton::instance() method that calls LLParamSingleton::getInstance(). Inheriting LLSingleton::instance() called LLSingleton::getInstance() -- not at all what we want. Add LLParamSingleton unit tests.
-
andreykproductengine authored
-
Nat Goodspeed authored
Simplify LLSingleton::SingletonLifetimeManager to SingletonInitializer: that struct has not been responsible for deletion ever since LLSingletonBase acquired dependency-ordered deleteAll(). Move SingletonData::mInitState changes from SingletonLifetimeManager to constructSingleton() method. Similarly, constructSingleton() now sets SingletonData::mInstance instead of making its caller store the pointer. Add variadic arguments to LLSingleton::constructSingleton() so we can reuse it for LLParamSingleton. Add finishInitializing() method to encapsulate logic reused for getInstance()'s INITIALIZING and DELETED cases. Make LLParamSingleton a subclass of LLSingleton, just as LLLockedSingleton is a subclass of LLParamSingleton. Make LLParamSingleton a friend of LLSingleton, so it can access private members of LLSingleton without also granting access to any DERIVED_CLASS subclass. This eliminates the need for protected getInitState(). LLParamSingleton::initParamSingleton() reuses LLSingleton::constructSingleton() and finishInitializing(). Its getInstance() method completely replaces LLSingleton::getInstance(): in most EInitStates, LLParamSingleton::getInstance() is an error. Use a std::mutex to serialize calls to LLParamSingleton::initParamSingleton() and getInstance(). While LLSingleton::getInstance() relies on the "initialized exactly once" guarantee for block-scope static declarations, LLParamSingleton cannot rely on the same mechanism. LLLockedSingleton is now a very succinct subclass of LLParamSingleton -- they have very similar functionality. Giving the LLSINGLETON() macro variadic arguments eliminates the need for a separate LLPARAMSINGLETON() macro, while continuing to support existing usage.
-
- Jul 25, 2019
-
-
andreykproductengine authored
-
- Jan 04, 2019
-
-
Rider Linden authored
-
- Apr 24, 2017
-
-
Nat Goodspeed authored
The recent LLSingleton work added a hook that would run during the C++ runtime's final destruction of static objects. When the LAST LLSingleton in any module was destroyed, its destructor would call LLSingletonBase::deleteAll(). That mechanism was intended to permit an application consuming LLSingletons to skip making an explicit deleteAll() call, knowing that all instantiated LLSingleton instances would eventually be cleaned up anyway. However -- experience proves that kicking off deleteAll() processing during the C++ runtime's final cleanup is too late. Too much has already been destroyed. That call tends to cause more shutdown crashes than it resolves. This commit deletes that whole mechanism. Going forward, if you want to clean up LLSingleton instances, you must explicitly call LLSingletonBase::deleteAll() during the application lifetime. If you don't, LLSingleton instances will simply be leaked -- which might be okay, considering the application is terminating anyway.
-
- Mar 13, 2017
-
-
Nat Goodspeed authored
The logging subsystem depends on two different LLSingletons for some reason. It turns out to be very difficult to completely avoid executing any logging calls after the LLSingletonBase::deleteAll(), but we really don't want to resurrect those LLSingletons so late in the run for a couple stragglers. Introduce LLSingleton::wasDeleted() query method, and use it in logging subsystem to simply bypass last-millisecond logging requests.
-
- Sep 15, 2016
-
-
Nat Goodspeed authored
A shocking number of LLSingleton subclasses had public constructors -- and in several instances, were being explicitly instantiated independently of the LLSingleton machinery. This breaks the new LLSingleton dependency-tracking machinery. It seems only fair that if you say you want an LLSingleton, there should only be ONE INSTANCE! Introduce LLSINGLETON() and LLSINGLETON_EMPTY_CTOR() macros. These handle the friend class LLSingleton<whatevah>; and explicitly declare a private nullary constructor. To try to enforce the LLSINGLETON() convention, introduce a new pure virtual LLSingleton method you_must_use_LLSINGLETON_macro() which is, as you might suspect, defined by the macro. If you declare an LLSingleton subclass without using LLSINGLETON() or LLSINGLETON_EMPTY_CTOR() in the class body, you can't instantiate the subclass for lack of a you_must_use_LLSINGLETON_macro() implementation -- which will hopefully remind the coder. Trawl through ALL LLSingleton subclass definitions, sprinkling in LLSINGLETON() or LLSINGLETON_EMPTY_CTOR() as appropriate. Remove all explicit constructor declarations, public or private, along with relevant 'friend class LLSingleton<myself>' declarations. Where destructors are declared, move them into private section as well. Where the constructor was inline but nontrivial, move out of class body. Fix several LLSingleton abuses revealed by making ctors/dtors private: LLGlobalEconomy was both an LLSingleton and the base class for LLRegionEconomy, a non-LLSingleton. (Therefore every LLRegionEconomy instance contained another instance of the LLGlobalEconomy "singleton.") Extract LLBaseEconomy; LLGlobalEconomy is now a trivial subclass of that. LLRegionEconomy, as you might suspect, now derives from LLBaseEconomy. LLToolGrab, an LLSingleton, was also explicitly instantiated by LLToolCompGun's constructor. Extract LLToolGrabBase, explicitly instantiated, with trivial subclass LLToolGrab, the LLSingleton instance. (WARNING: LLToolGrabBase methods have an unnerving tendency to go after LLToolGrab::getInstance(). I DO NOT KNOW what should be the relationship between the instance in LLToolCompGun and the LLToolGrab singleton instance.) LLGridManager declared a variant constructor accepting (const std::string&), with the comment: // initialize with an explicity grid file for testing. As there is no evidence of this being called from anywhere, delete it. LLChicletBar's constructor accepted an optional (const LLSD&). As the LLSD parameter wasn't used, and as there is no evidence of it being passed from anywhere, delete the parameter. LLViewerWindow::shutdownViews() was checking LLNavigationBar:: instanceExists(), then deleting its getInstance() pointer -- leaving a dangling LLSingleton instance pointer, a land mine if any subsequent code should attempt to reference it. Use deleteSingleton() instead. ~LLAppViewer() was calling LLViewerEventRecorder::instance() and then explicitly calling ~LLViewerEventRecorder() on that instance -- leaving the LLSingleton instance pointer pointing to an allocated-but-destroyed instance. Use deleteSingleton() instead.
-
- Sep 06, 2016
-
-
Nat Goodspeed authored
The stack we maintain of which LLSingletons are currently initializing only makes sense when associated with a particular C++ call stack. But each coroutine introduces another C++ call stack! Move the initializing stack from function-static storage to LLSingletonBase::MasterList. Make it a map keyed by llcoro::id. Each coro then has a stack of its own. This introduces more dependencies on the MasterList singleton, requiring additional LLSingleton_manage_master workarounds.
-
- Sep 03, 2016
-
-
Nat Goodspeed authored
Specifically, add DEBUG logging to the code that maintains the stack of LLSingletons currently being initialized. This involves passing LLSingletonBase's constructor the name of LLSingleton's template parameter subclass, since during that constructor typeid(*this).name() will only produce "LLSingletonBase". Also add logdebugs() and oktolog() helper functions.
-
- Nov 10, 2015
-
-
Oz Linden authored
-
- Jun 30, 2015
-
-
Nat Goodspeed authored
-
- Jun 26, 2015
-
-
Nat Goodspeed authored
-
Nat Goodspeed authored
LLSingleton explicitly supports circular dependencies: initialization performed during an LLSingleton subclass's initSingleton() method may recursively call that same subclass's getInstance() method. On the other hand, circularity from a subclass constructor cannot be permitted, else getInstance() would have to return a partially-constructed object. Our dependency tracking circularity check initially forbade both. Loosen it to permit references from within initSingleton().
-
- Jun 25, 2015
-
-
Nat Goodspeed authored
The forward declaration said it was a 'friend class', whereas the actual definition is a struct. MSVC dislikes that.
-
Nat Goodspeed authored
Part of LLError's logging infrastructure is implemented with an LLSingleton. Therefore, attempts to log from within LLSingleton machinery could potentially go south if LLError's LLSingleton is not yet initialized. Introduce LLError::is_available() in llerrorcontrol.h and llerror.cpp. Make LLSingletonBase::logwarns() and logerrs() consult LLError::is_available() before attempting to use LL_WARNS or LL_ERRS, respectively. Moreover, make all LLSingleton internal logging use logwarns() and logerrs() instead of directly engaging LL_ERRS or LL_WARNS.
-