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.
Until now, the "main coroutine" (the initial context) of each thread left
LLCoros::Current() NULL. The trouble with that is that llcoro::get_id()
returns that CoroData* as an opaque token, and we want distinct values for
every stack in the process. That would not be true if the "main coroutine" on
thread A returned the same value (NULL) as the "main coroutine" on thread B,
and so forth. Give each thread's "main coroutine" a dummy heap CoroData
instance of its own.
Change the module-static thread_specific_ptr to a function-static
thread_specific_ptr so it will be initialized on demand -- since LLSingleton
will need to rely on get_id(). Note that since LLCoros isa LLSingleton, we
must take great care to avoid circularity.
Introduce a private helper class LLCoros::Current to obtain and bind that
thread_specific_ptr. Change all existing internal references from the static
thread_specific_ptr to the new Current helper class.
The original implementation of set_consuming() involved a bool* pointing to a
local bool in VoidListener::operator()()'s stack frame. postAndSuspend() would
set that bool (through the pointer) as soon as it returned from suspension.
The trouble with that is that LLEventMailDrop potentially calls its new
listener (fulfilling the future) immediately in the listen_impl() override --
in other words, way up at the top of postAndSuspend(), well before the code
that sets the relevant bool.
Instead, make the adapter formerly known as VoidListener bind the coroutine's
get_consuming() value at adapter construction time (before listening on the
LLEventPump), so that its operator()() has the coroutine's correct
get_consuming() value to return. Eliminating the bool* makes the code both
simpler AND more correct!
This change makes that adapter very specific to coroutine usage. Rename it
FutureListener and migrate it from lleventcoros.h into the .cpp file. Nobody
else was using it anyway.
Make corresponding changes to postAndSuspend2() and its WaitForEventOnHelper
class -- whose name no longer corresponds to the function as it used to.
Rename that one FutureListener2. The new FutureListener functionality, common
to both these adapters, makes it useful to derive FutureListener2 from
FutureListener.
Introduce llmake(), a generic function to deduce template type arguments from
function parameter types. This allows us to remove the voidlistener() and
wfeoh() helper functions.
Hiding VoidListener broke one of the lleventcoro_test.cpp tests. But that test
was sort of a lame recap of an earlier implementation of postAndSuspend(),
based on LLEventPump events. Recast that test to illustrate how to use a
coroutine future to suspend a coroutine for something other than an LLEventPump.
But that rubbed my nose in the fact that we MUST wrap future's context
switching with proper management of the current coroutine. Introduce
LLCoros::Future<T>, which wraps boost::dcoroutines::future<T>.
Use LLCoros::Future<T> in postAndSuspend() and postAndSuspend2().
Our first cut at tracking the "current" coroutine simply reset the pointer to
NULL every time we context-switched away. But that strategy doesn't handle the
case of coroutine A launching coroutine B.
Introduce LLCoros::CoroData to track, among other things, the previous value
of the current-coroutine pointer each time we switch into a coroutine. Restore
THAT value when we switch back out.
To date, the coroutine helper functions in lleventcoro.h have been in the
global namespace. Migrate them into llcoro namespace, and fix references.
Specifically, LLVoidListener => llcoro::VoidListener, and voidlistener(),
postAndWait(), both waitForEventOn(), postAndWait2(), errorException() and
errorLog() have been moved into llcoro.
Also migrate new LLCoros::get_self() and Suspending to llcoro:: namespace.
While at it, I realized that -- having converted several lleventcoro.h
functions from templates (for arbitrary 'self' parameter type) to ordinary
functions, having moved them from lleventcoro.h to lleventcoro.cpp, we can now
migrate their helpers from lleventcoro.h to lleventcoro.cpp as well. This
eliminates the need for the LLEventDetail namespace; the relevant helpers are
now in an anonymous namespace in the .cpp file: listenerNameForCoro(),
storeToLLSDPath(), WaitForEventOnHelper and wfeoh().
Introduce LLCoros::setStackSize(), with a compile-time default value we hope
we never have to use. Make LLAppViewer call it with the value of the new
settings variable CoroutineStackSize as soon as we've read settings files.
(While we're at it, notify interested parties that we've read settings files.)
Give CoroutineStackSize a default value four times the previous default stack
size. Make LLCoros::launch() pass the saved stack size to each new coroutine
instance.
Re-enable lleventcoro integration test. Use LLSDMap() construct rather than
LLSD::insert(), which used to return the modified object but is now void.
In autobuild.xml, specify today's build of the Boost package that includes the
Boost.Context library, and whose boost::dcoroutines library uses Boost.Context
exclusively instead of its previous context-switching underpinnings (source of
the ucontext.h dependency).
Add BOOST_CONTEXT_LIBRARY to Boost.cmake and Copy3rdPartyLibs.cmake. Link it
with the viewer and with the lllogin.cpp test executable.
Track new Boost package convention that our (early, unofficial) Boost.Coroutine
library is now accessed as boost/dcoroutine/etc.h and boost::dcoroutines::etc.
Remove #include <boost/coroutine/coroutine.hpp> from
llviewerprecompiledheaders.h and lllogin.cpp: old rule that Boost.Coroutine
header must be #included before anything else that might use ucontext.h is
gone now that we no longer depend on ucontext.h. In fact remove
-D_XOPEN_SOURCE in 00-Common.cmake because that was inserted specifically to
work around a known problem with the ucontext.h facilities.
boost::bind() to pass any other coroutine arguments. This allows us to remove
the LLCoroBase and LLCoro constructs, directly storing a coroutine object in
our ptr_map. It also allows us to remove the multiple launch() overloads for
multiple arguments. Finally, it lets us move most launch() functionality into
a non-template method.
instances. LLCoros::launch() intends to address three issues:
- ownership of coroutine instance
- cleanup of coroutine instance when it terminates
- central place to twiddle MSVC optimizations to bypass DEV-32777 crash.
Initially coded on Mac; will address the third bullet on Windows.
Adapt listenerNameForCoro() to consult LLCoros::getName() if applicable.
Change LLLogin::Impl::connect() to use LLCoros::launch().
LLCoros::getName() relies on patch to boost::coroutines::coroutine::self to
introduce get_id().
andreykproductengine