| Age | Commit message (Collapse) | Author |
|---|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
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).
|
|
|
|
|
|
We used to have to use #if LL_WINDOWS logic to pass std::mem_fun1() to
llbind2nd() instead of std::mem_fun() elsewhere. VS 2017 no longer supports
std::mem_fun1(), which means we can eliminate the special case for Windows.
|
|
The Microsoft _open_osfhandle() opens a HANDLE to produce a C-style int file
descriptor suitable for passing to _fdopen(). We used to cast the HANDLEs
returned by GetStdHandle() to long to pass to _open_osfhandle(). Since HANDLE
is an alias for a pointer, this no longer works.
Fortunately _open_osfhandle() now accepts intptr_t, so we can change the
relevant GetStdHandle() calls. (But why not simply accept HANDLE in the first
place?)
|
|
VS 2017 complains about the same thing that clang does: casting S32 to GLvoid*
can't possibly produce a valid pointer value because S32 can't fit a whole
64-bit pointer. To appease it, not only must we use reinterpret_cast, but we
must first cast S32 to intptr_t and then reinterpret_cast THAT.
|
|
VS 2017 was complaining about truncating the value.
|
|
With VS 2017, these produced fatal warnings.
|
|
llcorehttp's test_allocator.{h,cpp} overrides global operator new(), operator
new[](), operator delete() and operator delete[](). The two operator new()
functions used to be declared with throw(std::bad_alloc). Worse, for VS 2013
and previous, we needed _THROW0() and _THROW1(std::bad_alloc) instead,
requiring #if logic.
But with dynamic throw declarations deprecated, we must actually remove those.
That obviates the THROW_BAD_ALLOC() / THROW_NOTHING() workarounds in
test_allocator.cpp.
|
|
In three different places we use the same pattern: an ssl_thread_id_callback()
function (a static member of LLCrashLogger, in that case) that used to be
passed to CRYPTO_set_id_callback() and therefore returned an unsigned long
representing the ID of the current thread.
But GetCurrentThread() is a HANDLE, an alias for a pointer, and you can't
uniquely cram a 64-bit pointer into an unsigned long.
Fortunately OpenSSL has a more modern API for retrieving thread ID. Pass
each ssl_thread_id_callback() function to CRYPTO_THREADID_set_callback()
instead, converting it to accept CRYPTO_THREADID* and call
CRYPTO_THREADID_set_pointer() or CRYPTO_THREADID_set_numeric() as appropriate().
|
|
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().
|
|
Also, on Windows, put build output into
build-vc$AUTOBUILD_VSVER-$AUTOBUILD_ADDRSIZE instead of hard-coding
build-vc120-$AUTOBUILD_ADDRSIZE.
|
|
Add code to login-fail handler to provide release notes URL from
SLVersionChecker handshake event.
|
|
|
|
Now, when the viewer decides it's appropriate to display release notes on the
login screen, wait for SLVersionChecker to post the release-notes URL before
opening the web floater.
|
|
Make LLAppViewer retrieve release notes from LLVersionInfo, rather than
synthesizing the release-notes URL itself based on the viewer version string.
|
|
getStateTable returns a list of the EStartupState symbolic names, implicitly
mapping each to its index (its enum numeric value).
|
|
Before this change, you had to literally pass LLSD::emptyArray() to get no-op
behavior.
|
|
The default string returned by getReleaseNotes() is empty. It must be set by
posting the relevant release-notes URL string to a new LLEventMailDrop
instance named "relnotes".
Add unique_ptr<LLEventMailDrop> and unique_ptr<LLStoreListener<std::string>>
to LLVersionInfo -- using unique_ptr to leave those classes opaque to
header-file consumers. Introduce an out-of-line destructor to handle the
unique_ptr<opaque> idiom.
Initialize the LLEventMailDrop with the desired name; initialize the
LLStoreListener with that LLEventMailDrop and the data member returned by
getReleaseNotes().
|
|
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.
|
|
This changeset is meant to exemplify how to convert a "namespace" class whose
methods are static -- and whose data are module-static -- to an LLSingleton.
LLVersionInfo has no initClass() or cleanupClass() methods, but the general
idea is the same.
* Derive the class from LLSingleton<T>:
class LLSomeSingleton: public LLSingleton<LLSomeSingleton> { ... };
* Add LLSINGLETON(LLSomeSingleton); in the private section of the class. This
usage implies a separate LLSomeSingleton::LLSomeSingleton() definition, as
described in indra/llcommon/llsingleton.h.
* Move module-scope data in the .cpp file to non-static class members. Change
any sVariableName to mVariableName to avoid being outright misleading.
* Make static class methods non-static. Remove '//static' comments from method
definitions as needed.
* For LLVersionInfo specifically, the 'const std::string&' return type was
replaced with 'std::string'. Returning a reference to a static or a member,
const or otherwise, is an anti-pattern: the interface constrains the
implementation, prohibiting possibly later returning a temporary (an
expression).
* For LLVersionInfo specifically, 'const S32' return type was replaced with
simple 'S32'. 'const' is just noise in that usage.
* Simple member initialization (e.g. the original initializer expressions for
static variables) can be done with member{ value } initializers (no examples
here though).
* Delete initClass() method.
* LLSingleton's forté is of course lazy initialization. It might work to
simply delete any calls to initClass(). But if there are side effects that
must happen at that moment, replace LLSomeSingleton::initClass() with
(void)LLSomeSingleton::instance();
* Most initClass() initialization can be done in the constructor, as would
normally be the case.
* Initialization that might cause a circular LLSingleton reference should be
moved to initSingleton(). Override 'void initSingleton();' should be private.
* For LLVersionInfo specifically, certain initialization that used to be
lazily performed was made unconditional, due to its low cost.
* For LLVersionInfo specifically, certain initialization involved calling
methods that have become non-static. This was moved to initSingleton()
because, in a constructor body, 'this' does not yet point to the enclosing
class.
* Delete cleanupClass() method.
* There is already a generic LLSingletonBase::deleteAll() call in
LLAppViewer::cleanup(). It might work to let this new LLSingleton be cleaned
up with all the rest. But if there are side effects that must happen at that
moment, replace LLSomeSingleton::cleanupClass() with
LLSomeSingleton::deleteSingleton(). That said, much of the benefit of
converting to LLSingleton is deleteAll()'s guarantee that cross-LLSingleton
dependencies will be properly honored: we're trying to migrate the code base
away from the present fragile manual cleanup sequence.
* Most cleanupClass() cleanup can be done in the destructor, as would normally
be the case.
* Cleanup that might throw an exception should be moved to cleanupSingleton().
Override 'void cleanupSingleton();' should be private.
* Within LLSomeSingleton methods, remove any existing
LLSomeSingleton::methodName() qualification: simple methodName() is better.
* In the rest of the code base, convert most LLSomeSingleton::methodName()
references to LLSomeSingleton::instance().methodName(). (Prefer instance() to
getInstance() because a reference does not admit the possibility of NULL.)
* Of course, LLSomeSingleton::ENUM_VALUE can remain unchanged.
In general, for many successive references to an LLSingleton instance, it
can be useful to capture the instance() as in:
auto& versionInfo{LLVersionInfo::instance()};
// ... versionInfo.getVersion() ...
We did not do that here only to simplify the code review.
The STRINGIZE(expression) macro encapsulates:
std::ostringstream out;
out << expression;
return out.str();
We used that in a couple places.
For LLVersionInfo specifically, lllogininstance_test.cpp used to dummy out a
couple specific static methods. It's harder to dummy out
LLSingleton::instance() references, so we add the real class to that test.
|
|
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().
|
|
by tweaking #include order.
|
|
|
|
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.
|
|
|
|
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::thread::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::thread::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::thread::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.
|
