| Age | Commit message (Collapse) | Author |
|---|
|
No one uses LLEventQueue to defer posted events until the next mainloop tick
-- and with LLCoros moving to Boost.Fiber, cross-coroutine event posting works
that way anyway, making LLEventQueue pretty unnecessary.
The static RegisterFlush instance in llevents.cpp was used to call
LLEventPumps::flush() once per mainloop tick, which in turn called flush() on
every registered LLEventPump. But the only reason for that mechanism was to
support LLEventQueue. In fact, when LLEventMailDrop overrode its flush()
method for something quite different, it was startling to find that the new
flush() override was being called once per frame -- which caused at least one
fairly mysterious bug. Remove RegisterFlush. Both LLEventPumps::flush() and
LLEventPump::flush() remain for now, though intended usage is unclear.
Eliminating LLEventQueue means we must at least repurpose
LLEventPumps::mQueueNames, a map intended to make LLEventPumps::obtain()
instantiate an LLEventQueue rather than the default LLEventPump. Replace it
with mFactories, a map from desired instance name to a callable returning
LLEventPump*. New map initialization syntax plus lambda support allows us to
populate that map at compile time with little lambdas returning the correct
subclass instance.
Similarly, LLLeapListener::newpump() used to check the ["type"] entry in the
LLSD request specifically for "LLEventQueue". Introduce another such map in
llleaplistener.cpp for potential future extensibility.
Eliminate the LLEventQueue-specific test.
|
|
Overriding virtual LLEventPump::flush() for the semantic of discarding
LLEventMailDrop's queued events turns out not to be such a great idea, because
LLEventPumps::flush(), which calls every registered LLEventPump's flush()
method, is called every mainloop tick. The first time we hit a use case in
which we expected LLEventMailDrop to hold queued events across a mainloop tick,
we were baffled that they were never delivered.
Moving that logic to a separate method specific to LLEventMailDrop resolves
that problem. Naming it discard() clarifies its intended functionality.
|
|
LLEventLogProxy can be introduced to serve as a logging proxy for an existing
LLEventPump subclass instance. Access through the LLEventLogProxy will be
logged; access directly to the underlying LLEventPump will not.
LLEventLogProxyFor<LLEventPumpSubclass> functions as a drop-in replacement for
the original LLEventPumpSubclass instance. It internally instantiates
LLEventPumpSubclass and serves as a proxy for that instance.
Add unit tests for LLEventMailDrop and LLEventLogProxyFor<LLEventMailDrop>,
both "plain" (events only) and via lleventcoro.h synchronization.
|
|
passed to postAndSuspendsetup().
The requestPump is optional, and the function varies its behavior depending on
whether that parameter is empty or meaningful. But it unconditionally uses the
replyPump. Passing an empty LLEventPumpOrPumpName caused mysterious crashes.
Add llassert_always_msg() to make the coding error explicit in such a case.
Also streamline access to meaningful requestPump and replyPump by temporarily
caching the bound LLEventPump reference.
|
|
LLEventDetail::visit_and_connect() promised special treatment for the
specific case when an LLEventPump::listen() listener was composed of (possibly
nested) boost::bind() objects storing boost::weak_ptr values -- specifically
boost::bind() rather than std::bind or lambdas, specifically boost::weak_ptr
rather than std::weak_ptr.
Outside of self-tests, it does not appear that anyone actually uses that
support.
There is good reason not to: it's a silent side effect of a complicated
compile-time inspection that could be silently derailed by use of std::bind()
or a lambda or a std::weak_ptr. Can you be sure you've engaged that promise?
How?
A more robust guarantee can be achieved by storing an LLTempBoundConnection in
the transient object itself. When the object is destroyed, the listener is
disconnected. Normal C++ rules around object destruction guarantee it. This
idiom is widely used.
There are a couple good reasons to remove the visit_and_connect() machinery:
* boost::bind() and boost::weak_ptr do not constitute the wave of the future.
Preferring those constructs to lambdas and std::weak_ptr penalizes new code,
whether by silently failing or by discouraging use of modern idioms.
* The visit_and_connect() machinery was always complicated, and apparently
never very robust. Most of its promised features have been commented out
over the years. Making the code base simpler, clearer and more maintainable
is always a useful effect.
LLEventDetail::visit_and_connect() was also used by the four
LLNotificationChannelBase::connectMumble() methods. Streamline those as well.
Of course, remove related test code.
|
|
The only usage of any of this was in test code.
|
|
llsd::array(), as one might suspect, takes an arbitrary number of arguments of
arbitrary convertible types and returns an LLSD::Array constructed from those
elements. This supercedes the older LLSDArray class.
llsd::map() takes an even number of arguments paired as (LLSD::String,
arbitrary convertible type) and returns an LLSD::Map constructed from those
(key, value) pairs. This supercedes the older LLSDMap class.
These two functions not only have a simpler API -- arbitrary function
arguments rather than an (arg list)(arg list) sequence -- but also
specifically return a final LLSD object, rather than needing conversion to
LLSD from the LLSDArray or LLSDMap object.
Also support LLSD == LLSD and LLSD != LLSD comparisons, using llsd_equals()
with default exact-float-equality semantics.
|
|
|
|
The comments within indra/test/test.cpp promise that --debug is, in fact, like
LOGTEST=DEBUG. Until now, that was a lie. LOGTEST=level displayed log output
on stderr as well as in testprogram.log, while --debug did not.
Add LLError::logToStderr() function, and make initForApplication() (i.e.
commonInit()) call that instead of instantiating RecordToStderr inline. Also
call it when test.cpp recognizes --debug switch.
Remove the mFileRecorder, mFixedBufferRecorder and mFileRecorderFileName
members from SettingsConfig. That tactic doesn't scale.
Instead, add findRecorder<RECORDER>() and removeRecorder<RECORDER>() template
functions to locate (or remove) a RecorderPtr to an object of the specified
subclass. Both are based on an underlying findRecorderPos<RECORDER>() template
function. Since we never expect to manage more than a handful of RecorderPtrs,
and since access to the deleted members is very much application setup rather
than any kind of ongoing access, a search loop suffices.
logToFile() uses removeRecorder<RecordToFile>() rather than removing
mFileRecorder (the only use of mFileRecorder).
logToFixedBuffer() uses removeRecorder<RecordToFixedBuffer>() rather than
removing mFixedBufferRecorder (the only use of mFixedBufferRecorder).
Make RecordToFile store the filename with which it was instantiated. Add a
getFilename() method to retrieve it. logFileName() is now based on
findRecorder<RecordToFile>() instead of mFileRecorderFileName (the only use of
mFileRecorderFileName).
Make RecordToStderr::mUseANSI a simple bool rather than a three-state enum,
and set it immediately on construction. Apparently the reason it was set
lazily was because it consults its own checkANSI() method, and of course
'this' doesn't acquire the leaf class type until the constructor has completed
successfully. But since nothing in checkANSI() depends on anything else in
RecordToStderr, making it static solves that problem.
|
|
Actually the fix is in postAndSuspendSetup(), which affects postAndSuspend(),
postAndSuspendWithTimeout(), suspendUntilEventOnWithTimeout() and
suspendUntilEventOn().
By "overflow case" we mean the special circumstance in which:
* the LLEventPump in question is an LLEventMailDrop, meaning its listeners
eventually expect to see every post()ed value
* one of the listeners is supposed to consume those values (has called
LLCoros::set_consuming(true))
* post() is called more than once before that listener is resumed.
The magic of postAndSuspend() (et al.) is a temporary LLCoros::Promise. The
waiting coroutine calls get() on the corresponding Future, causing it to
suspend (as promised) until the Promise is fulfilled.
With the Boost.Fiber implementation of coroutines, fulfilling the Promise
doesn't immediately resume the suspended coroutine -- it merely marks it ready
to resume, next time the scheduler gets control.
A second post() call before the suspended coroutine is resumed results in a
second call to Promise::set_value(). But Promise is a one-shot entity. This
results in a promise_already_satisfied exception. Because a second post() call
during that time window is perfectly reasonable, we catch that exception and
carry on.
The tricky part is: when that exception is thrown, what should the listener
return? Previously we were returning the listener's current consuming setting,
just as when the set_value() call succeeds.
But when the LLEventPump is an LLEventMailDrop, and the listener's consuming
flag is true, that told LLEventMailDrop::post() that the value got through,
and that it needn't bother to save it in its history queue. The net effect was
to discard the value.
Instead, return the listener's consuming flag only when Promise::set_value()
succeeds. When it throws promise_already_satisfied, unconditionally return
false. That directs LLEventMailDrop::post() to enqueue the undelivered value
so that the *next* suspendUntilEventOn() call can pick it up.
|
|
|
|
Sync is specifically intended for test programs. It is based on an
LLScalarCond<int>. The idea is that each of two coroutines can watch for the
other to get a chance to run, indicated by incrementing the wrapped int and
notifying the wrapped condition_variable. This is less hand-wavy than calling
llcoro::suspend() and hoping that the other routine will have had a chance to
run.
Use Sync in lleventcoro_test.cpp.
Also refactor lleventcoro_test.cpp so that instead of a collection of static
data requiring a clear() call at start of each individual test function, the
relevant data is all part of the test_data struct common to all test
functions. Make the helper coroutine functions members of test_data too.
Introduce llcoro::logname(), a convenience function to log the name of the
currently executing coroutine or "main" if in the thread's main coroutine.
|
|
If the test<1>() child process terminates with nonzero rc, also report any
stdout/stderr it might have emitted first.
|
|
|
|
|
|
|
|
|
|
Also introduce value_type typedef.
|
|
LLCond encapsulates the usage patterns required to properly use
condition_variable. We also provide LLScalarCond, LLBoolCond and LLOneShotCond.
|
|
The global replace in changeset bd80903cf987 was a bit too sweeping: a comment
mentioning the OS function wait() (which exists) was inadvertently changed to
talk about an OS function suspend() (which does not).
|
|
|
|
|
|
build working
|
|
|
|
|
|
times by multiple events.
|
|
LLError::shouldLogToStderr() behavior under xcode.
|
|
been fulfilled.
|
|
useful for debugging;
|
|
|
|
|
|
Delete the test for SRV timeout: lllogin no longer issues an SRV query. That
test only confuses the test program without exercising any useful paths in
production code.
As with other tests dating from the previous LLCoros implementation, we need a
few llcoro::suspend() calls sprinkled in so that a fiber marked ready -- by
fulfilling the future for which it is waiting -- gets a chance to run.
Clear LLEventPumps between test functions.
|
|
which is, of course, different in Visual Studio (__FUNCSIG__).
Use LL_PRETTY_FUNCTION in DEBUG output instead of plain __FUNCTION__.
|
|
This is like the existing reset() method, except that reset() is specifically
intended for shutdown: it disables every existing LLEventPump in such a way
that it cannot be subsequently reused. (The original idea was to disconnect
listeners in DLLs unloaded at shutdown.)
clear() forcibly disconnects all existing listeners, but leaves LLEventPumps
ready for reuse. This is useful (e.g.) for test programs to reset the state of
LLEventPumps between individual test functions.
|
|
Longtime fans will remember that the "dcoroutine" library is a Google Summer
of Code project by Giovanni P. Deretta. He originally called it
"Boost.Coroutine," and we originally added it to our 3p-boost autobuild
package as such. But when the official Boost.Coroutine library came along
(with a very different API), and we still needed the API of the GSoC project,
we renamed the unofficial one "dcoroutine" to allow coexistence.
The "dcoroutine" library had an internal low-level API more or less analogous
to Boost.Context. We later introduced an implementation of that internal API
based on Boost.Context, a step towards eliminating the GSoC code in favor of
official, supported Boost code.
However, recent versions of Boost.Context no longer support the API on which
we built the shim for "dcoroutine." We started down the path of reimplementing
that shim using the current Boost.Context API -- then realized that it's time
to bite the bullet and replace the "dcoroutine" API with the Boost.Fiber API,
which we've been itching to do for literally years now.
Naturally, most of the heavy lifting is in llcoros.{h,cpp} and
lleventcoro.{h,cpp} -- which is good: the LLCoros layer abstracts away most of
the differences between "dcoroutine" and Boost.Fiber.
The one feature Boost.Fiber does not provide is the ability to forcibly
terminate some other fiber. Accordingly, disable LLCoros::kill() and
LLCoprocedureManager::shutdown(). The only known shutdown() call was in
LLCoprocedurePool's destructor.
We also took the opportunity to remove postAndSuspend2() and its associated
machinery: FutureListener2, LLErrorEvent, errorException(), errorLog(),
LLCoroEventPumps. All that dual-LLEventPump stuff was introduced at a time
when the Responder pattern was king, and we assumed we'd want to listen on one
LLEventPump with the success handler and on another with the error handler. We
have never actually used that in practice. Remove associated tests, of course.
There is one other semantic difference that necessitates patching a number of
tests: with "dcoroutine," fulfilling a future IMMEDIATELY resumes the waiting
coroutine. With Boost.Fiber, fulfilling a future merely marks the fiber as
ready to resume next time the scheduler gets around to it. To observe the test
side effects, we've inserted a number of llcoro::suspend() calls -- also in
the main loop.
For a long time we retained a single unit test exercising the raw "dcoroutine"
API. Remove that.
Eliminate llcoro_get_id.{h,cpp}, which provided llcoro::get_id(), which was a
hack to emulate fiber-local variables. Since Boost.Fiber has an actual API for
that, remove the hack.
In fact, use (new alias) LLCoros::local_ptr for LLSingleton's dependency
tracking in place of llcoro::get_id().
In CMake land, replace BOOST_COROUTINE_LIBRARY with BOOST_FIBER_LIBRARY. We
don't actually use the Boost.Coroutine for anything (though there exist
plausible use cases).
|
|
VS 2017 was complaining about truncating the value.
|
|
With VS 2017, these produced fatal warnings.
|
|
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/snprintf-snprintf-snprintf-l-snwprintf-snwprintf-l?view=vs-2017
"Beginning with the UCRT in Visual Studio 2015 and Windows 10, snprintf is no
longer identical to _snprintf. The snprintf function behavior is now C99
standard compliant."
In other words, VS 2015 et ff. snprintf() now promises to nul-terminate the
buffer even in the overflow case, which is what snprintf_hack::snprintf() was
for.
This removal was motivated by ambiguous-call errors generated by VS 2017 for
library snprintf() vs. snprintf_hack::snprintf().
|
|
Before this change, you had to literally pass LLSD::emptyArray() to get no-op
behavior.
|
|
LLStoreListener is an adapter initialized with a reference to an LLEventPump
on which to listen, a reference to a variable into which to store received
data, and an optional llsd::drill() path to extract desired data from each
event received on the subject LLEventPump.
In effect, LLStoreListener is like a miniature LLEventAPI whose only operation
is to store to its destination variable.
|
|
We include both const and non-const overloads. The latter returns LLSD&, so
you can assign to the located element.
In fact we already implemented the non-const logic in a less public form as
storeToLLSDPath() in lleventcoro.cpp. Reimplement the latter to use the new
llsd::drill() function.
|
|
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().
|
|
So does LLLockedSingleton<T>::construct().
|
|
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.
|
|
Monty's code review reveals that conflating dispatch() with [un]lock
functionality is inconsistent and unnecessary.
|
