From 66981fab0b3c8dcc3a031d50710a2b24ec6b0603 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 10 May 2018 21:46:07 -0400 Subject: SL-793: Use Boost.Fiber instead of the "dcoroutine" library. 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). --- indra/test/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/test') diff --git a/indra/test/CMakeLists.txt b/indra/test/CMakeLists.txt index 8344cead57..4187076030 100644 --- a/indra/test/CMakeLists.txt +++ b/indra/test/CMakeLists.txt @@ -98,7 +98,7 @@ target_link_libraries(lltest ${WINDOWS_LIBRARIES} ${BOOST_PROGRAM_OPTIONS_LIBRARY} ${BOOST_REGEX_LIBRARY} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY} ${DL_LIBRARY} -- cgit v1.2.3 From 939d35054881d150fe5d191d6965f6a09c5d0223 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 29 Dec 2018 10:19:01 -0500 Subject: SL-793: Add LL_PRETTY_FUNCTION macro wrapping __PRETTY_FUNCTION__ which is, of course, different in Visual Studio (__FUNCSIG__). Use LL_PRETTY_FUNCTION in DEBUG output instead of plain __FUNCTION__. --- indra/test/debug.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/test') diff --git a/indra/test/debug.h b/indra/test/debug.h index d61eba651b..33c3ea2d27 100644 --- a/indra/test/debug.h +++ b/indra/test/debug.h @@ -64,7 +64,7 @@ private: // It's often convenient to use the name of the enclosing function as the name // of the Debug block. -#define DEBUG Debug debug(__FUNCTION__) +#define DEBUG Debug debug(LL_PRETTY_FUNCTION) // These BEGIN/END macros are specifically for debugging output -- please // don't assume you must use such for coroutines in general! They only help to -- cgit v1.2.3 From b5bb0794f0022517c7aeff9a7775864a56488da6 Mon Sep 17 00:00:00 2001 From: Anchor Date: Wed, 8 May 2019 18:57:33 -0600 Subject: [DRTVWR-476] - fix linking --- indra/test/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) (limited to 'indra/test') diff --git a/indra/test/CMakeLists.txt b/indra/test/CMakeLists.txt index 4187076030..0f14862cba 100644 --- a/indra/test/CMakeLists.txt +++ b/indra/test/CMakeLists.txt @@ -83,6 +83,7 @@ list(APPEND test_SOURCE_FILES ${test_HEADER_FILES}) add_executable(lltest ${test_SOURCE_FILES}) target_link_libraries(lltest + ${LEGACY_STDIO_LIBS} ${LLDATABASE_LIBRARIES} ${LLINVENTORY_LIBRARIES} ${LLMESSAGE_LIBRARIES} -- cgit v1.2.3 From d7c2e4a77bed665d7ab626d9955c35db8c318e95 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 11 Sep 2019 09:33:07 -0400 Subject: DRTVWR-476: Add Sync class to help with stepwise coroutine tests. Sync is specifically intended for test programs. It is based on an LLScalarCond. 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. --- indra/test/CMakeLists.txt | 1 + indra/test/sync.h | 85 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 indra/test/sync.h (limited to 'indra/test') diff --git a/indra/test/CMakeLists.txt b/indra/test/CMakeLists.txt index 0f14862cba..87536e146b 100644 --- a/indra/test/CMakeLists.txt +++ b/indra/test/CMakeLists.txt @@ -67,6 +67,7 @@ set(test_HEADER_FILES llpipeutil.h llsdtraits.h lltut.h + sync.h ) if (NOT WINDOWS) diff --git a/indra/test/sync.h b/indra/test/sync.h new file mode 100644 index 0000000000..cafbc034b4 --- /dev/null +++ b/indra/test/sync.h @@ -0,0 +1,85 @@ +/** + * @file sync.h + * @author Nat Goodspeed + * @date 2019-03-13 + * @brief Synchronize coroutines within a test program so we can observe side + * effects. Certain test programs test coroutine synchronization + * mechanisms. Such tests usually want to interleave coroutine + * executions in strictly stepwise fashion. This class supports that + * paradigm. + * + * $LicenseInfo:firstyear=2019&license=viewerlgpl$ + * Copyright (c) 2019, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_SYNC_H) +#define LL_SYNC_H + +#include "llcond.h" +#include "lltut.h" +#include "stringize.h" +#include "llerror.h" +#include "llcoros.h" + +/** + * Instantiate Sync in any test in which we need to suspend one coroutine + * until we're sure that another has had a chance to run. Simply calling + * llcoro::suspend() isn't necessarily enough; that provides a chance for the + * other to run, but doesn't guarantee that it has. If each coroutine is + * consistent about calling Sync::bump() every time it wakes from any + * suspension, Sync::yield() and yield_until() should at least ensure that + * somebody else has had a chance to run. + */ +class Sync +{ + LLScalarCond mCond{0}; + F32Milliseconds mTimeout; + +public: + Sync(F32Milliseconds timeout=F32Milliseconds(10.0f)): + mTimeout(timeout) + {} + + /// Bump mCond by n steps -- ideally, do this every time a participating + /// coroutine wakes up from any suspension. The choice to bump() after + /// resumption rather than just before suspending is worth calling out: + /// this practice relies on the fact that condition_variable::notify_all() + /// merely marks a suspended coroutine ready to run, rather than + /// immediately resuming it. This way, though, even if a coroutine exits + /// before reaching its next suspend point, the other coroutine isn't + /// left waiting forever. + void bump(int n=1) + { + LL_DEBUGS() << llcoro::logname() << " bump(" << n << ") -> " << (mCond.get() + n) << LL_ENDL; + mCond.set_all(mCond.get() + n); + } + + /// suspend until "somebody else" has bumped mCond by n steps + void yield(int n=1) + { + return yield_until(STRINGIZE("Sync::yield_for(" << n << ") timed out after " + << int(mTimeout.value()) << "ms"), + mCond.get() + n); + } + + /// suspend until "somebody else" has bumped mCond to a specific value + void yield_until(int until) + { + return yield_until(STRINGIZE("Sync::yield_until(" << until << ") timed out after " + << int(mTimeout.value()) << "ms"), + until); + } + +private: + void yield_until(const std::string& desc, int until) + { + std::string name(llcoro::logname()); + LL_DEBUGS() << name << " yield_until(" << until << ") suspending" << LL_ENDL; + tut::ensure(name + ' ' + desc, mCond.wait_for_equal(mTimeout, until)); + // each time we wake up, bump mCond + bump(); + } +}; + +#endif /* ! defined(LL_SYNC_H) */ -- cgit v1.2.3 From 6b70493ddb1b95a2d3527e2189f5b94f5a2b606f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 14 Oct 2019 15:41:09 -0400 Subject: DRTVWR-476: Make test program --debug switch work like LOGTEST=DEBUG. 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() and removeRecorder() template functions to locate (or remove) a RecorderPtr to an object of the specified subclass. Both are based on an underlying findRecorderPos() 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() rather than removing mFileRecorder (the only use of mFileRecorder). logToFixedBuffer() uses removeRecorder() 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() 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. --- indra/test/test.cpp | 3 +++ 1 file changed, 3 insertions(+) (limited to 'indra/test') diff --git a/indra/test/test.cpp b/indra/test/test.cpp index b14c2eb255..51f0e80043 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -611,6 +611,9 @@ int main(int argc, char **argv) wait_at_exit = true; break; case 'd': + // this is what LLError::initForApplication() does internally + // when you pass log_to_stderr=true + LLError::logToStderr(); LLError::setDefaultLevel(LLError::LEVEL_DEBUG); break; case 'x': -- cgit v1.2.3 From 18e2b9ca8b5d4be0f1b92b464421693678cca0a0 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 15 Oct 2019 16:16:55 -0400 Subject: DRTVWR-476: Remove llwrap(), LLListenerWrapper[Base] and support. The only usage of any of this was in test code. --- indra/test/llevents_tut.cpp | 30 +----------------------------- 1 file changed, 1 insertion(+), 29 deletions(-) (limited to 'indra/test') diff --git a/indra/test/llevents_tut.cpp b/indra/test/llevents_tut.cpp index 3abae3e43e..9ff0f5086d 100644 --- a/indra/test/llevents_tut.cpp +++ b/indra/test/llevents_tut.cpp @@ -38,7 +38,6 @@ #define testable public #include "llevents.h" #undef testable -#include "lllistenerwrapper.h" // STL headers // std headers #include @@ -633,33 +632,6 @@ ensure("implicit disconnect", ! connection.connected()); heaptest.post(2); } -template<> template<> -void events_object::test<15>() -{ -// This test ensures that using an LLListenerWrapper subclass doesn't -// block Boost.Signals2 from recognizing a bound LLEventTrackable -// subclass. -set_test_name("listen(llwrap(boost::bind(...TempTrackableListener ref...)))"); -bool live = false; -LLEventPump& heaptest(pumps.obtain("heaptest")); -LLBoundListener connection; -{ - TempTrackableListener tempListener("temp", live); - ensure("TempTrackableListener constructed", live); - connection = heaptest.listen(tempListener.getName(), - llwrap( - boost::bind(&TempTrackableListener::call, - boost::ref(tempListener), _1))); - heaptest.post(1); - check_listener("received", tempListener, 1); -} // presumably this will make tempListener go away? -// verify that -ensure("TempTrackableListener destroyed", ! live); -ensure("implicit disconnect", ! connection.connected()); -// now just make sure we don't blow up trying to access a freed object! -heaptest.post(2); -} - class TempSharedListener: public TempListener, public boost::enable_shared_from_this { @@ -670,7 +642,7 @@ TempSharedListener(const std::string& name, bool& liveFlag): }; template<> template<> -void events_object::test<16>() +void events_object::test<15>() { set_test_name("listen(boost::bind(...TempSharedListener ref...))"); #if 0 -- cgit v1.2.3 From afaad3cef749e926cb63b6aed0c14f25d3495d55 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 15 Oct 2019 23:06:04 -0400 Subject: DRTVWR-476: Remove special case for listen(boost::bind(weak_ptr)). 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. --- indra/test/llevents_tut.cpp | 222 +++++++++----------------------------------- 1 file changed, 43 insertions(+), 179 deletions(-) (limited to 'indra/test') diff --git a/indra/test/llevents_tut.cpp b/indra/test/llevents_tut.cpp index 9ff0f5086d..a8a3188249 100644 --- a/indra/test/llevents_tut.cpp +++ b/indra/test/llevents_tut.cpp @@ -492,196 +492,60 @@ void events_object::test<10>() heaptest.stopListening("temp"); } -template<> template<> -void events_object::test<11>() -{ - set_test_name("listen(boost::bind(...weak_ptr...))"); - // listen() detecting weak_ptr in boost::bind() object - bool live = false; - LLEventPump& heaptest(pumps.obtain("heaptest")); - LLBoundListener connection; - ensure("default state", !connection.connected()); - { - boost::shared_ptr newListener(new TempListener("heap", live)); - newListener->reset(); - ensure("TempListener constructed", live); - connection = heaptest.listen(newListener->getName(), - boost::bind(&Listener::call, - weaken(newListener), - _1)); - ensure("new connection", connection.connected()); - heaptest.post(1); - check_listener("received", *newListener, 1); - } // presumably this will make newListener go away? - // verify that - ensure("TempListener destroyed", !live); - ensure("implicit disconnect", !connection.connected()); - // now just make sure we don't blow up trying to access a freed object! - heaptest.post(2); -} - -template<> template<> -void events_object::test<12>() -{ - set_test_name("listen(boost::bind(...shared_ptr...))"); - /*==========================================================================*| - // DISABLED because I've made this case produce a compile error. - // Following the error leads the disappointed dev to a comment - // instructing her to use the weaken() function to bind a weak_ptr - // instead of binding a shared_ptr, and explaining why. I know of - // no way to use TUT to code a repeatable test in which the expected - // outcome is a compile error. The interested reader is invited to - // uncomment this block and build to see for herself. - - // listen() detecting shared_ptr in boost::bind() object - bool live = false; - LLEventPump& heaptest(pumps.obtain("heaptest")); - LLBoundListener connection; - std::string listenerName("heap"); - ensure("default state", !connection.connected()); - { - boost::shared_ptr newListener(new TempListener(listenerName, live)); - ensure_equals("use_count", newListener.use_count(), 1); - newListener->reset(); - ensure("TempListener constructed", live); - connection = heaptest.listen(newListener->getName(), - boost::bind(&Listener::call, newListener, _1)); - ensure("new connection", connection.connected()); - ensure_equals("use_count", newListener.use_count(), 2); - heaptest.post(1); - check_listener("received", *newListener, 1); - } // this should make newListener go away... - // Unfortunately, the fact that we've bound a shared_ptr by value into - // our LLEventPump means that copy will keep the referenced object alive. - ensure("TempListener still alive", live); - ensure("still connected", connection.connected()); - // disconnecting explicitly should delete the TempListener... - heaptest.stopListening(listenerName); -#if 0 // however, in my experience, it does not. I don't know why not. - // Ah: on 2009-02-19, Frank Mori Hess, author of the Boost.Signals2 - // library, stated on the boost-users mailing list: - // http://www.nabble.com/Re%3A--signals2--review--The-review-of-the-signals2-library-(formerly-thread_safe_signals)-begins-today%2C-Nov-1st-p22102367.html - // "It will get destroyed eventually. The signal cleans up its slot - // list little by little during connect/invoke. It doesn't immediately - // remove disconnected slots from the slot list since other threads - // might be using the same slot list concurrently. It might be - // possible to make it immediately reset the shared_ptr owning the - // slot though, leaving an empty shared_ptr in the slot list, since - // that wouldn't invalidate any iterators." - ensure("TempListener destroyed", ! live); - ensure("implicit disconnect", ! connection.connected()); -#endif // 0 - // now just make sure we don't blow up trying to access a freed object! - heaptest.post(2); -|*==========================================================================*/ -} - class TempTrackableListener: public TempListener, public LLEventTrackable { public: -TempTrackableListener(const std::string& name, bool& liveFlag): - TempListener(name, liveFlag) -{} + TempTrackableListener(const std::string& name, bool& liveFlag): + TempListener(name, liveFlag) + {} }; template<> template<> -void events_object::test<13>() -{ -set_test_name("listen(boost::bind(...TempTrackableListener ref...))"); -bool live = false; -LLEventPump& heaptest(pumps.obtain("heaptest")); -LLBoundListener connection; +void events_object::test<11>() { - TempTrackableListener tempListener("temp", live); - ensure("TempTrackableListener constructed", live); - connection = heaptest.listen(tempListener.getName(), - boost::bind(&TempTrackableListener::call, - boost::ref(tempListener), _1)); - heaptest.post(1); - check_listener("received", tempListener, 1); -} // presumably this will make tempListener go away? -// verify that -ensure("TempTrackableListener destroyed", ! live); -ensure("implicit disconnect", ! connection.connected()); -// now just make sure we don't blow up trying to access a freed object! -heaptest.post(2); + set_test_name("listen(boost::bind(...TempTrackableListener ref...))"); + bool live = false; + LLEventPump& heaptest(pumps.obtain("heaptest")); + LLBoundListener connection; + { + TempTrackableListener tempListener("temp", live); + ensure("TempTrackableListener constructed", live); + connection = heaptest.listen(tempListener.getName(), + boost::bind(&TempTrackableListener::call, + boost::ref(tempListener), _1)); + heaptest.post(1); + check_listener("received", tempListener, 1); + } // presumably this will make tempListener go away? + // verify that + ensure("TempTrackableListener destroyed", ! live); + ensure("implicit disconnect", ! connection.connected()); + // now just make sure we don't blow up trying to access a freed object! + heaptest.post(2); } template<> template<> -void events_object::test<14>() -{ -set_test_name("listen(boost::bind(...TempTrackableListener pointer...))"); -bool live = false; -LLEventPump& heaptest(pumps.obtain("heaptest")); -LLBoundListener connection; +void events_object::test<12>() { - TempTrackableListener* newListener(new TempTrackableListener("temp", live)); - ensure("TempTrackableListener constructed", live); - connection = heaptest.listen(newListener->getName(), - boost::bind(&TempTrackableListener::call, - newListener, _1)); - heaptest.post(1); - check_listener("received", *newListener, 1); - // explicitly destroy newListener - delete newListener; -} -// verify that -ensure("TempTrackableListener destroyed", ! live); -ensure("implicit disconnect", ! connection.connected()); -// now just make sure we don't blow up trying to access a freed object! -heaptest.post(2); + set_test_name("listen(boost::bind(...TempTrackableListener pointer...))"); + bool live = false; + LLEventPump& heaptest(pumps.obtain("heaptest")); + LLBoundListener connection; + { + TempTrackableListener* newListener(new TempTrackableListener("temp", live)); + ensure("TempTrackableListener constructed", live); + connection = heaptest.listen(newListener->getName(), + boost::bind(&TempTrackableListener::call, + newListener, _1)); + heaptest.post(1); + check_listener("received", *newListener, 1); + // explicitly destroy newListener + delete newListener; + } + // verify that + ensure("TempTrackableListener destroyed", ! live); + ensure("implicit disconnect", ! connection.connected()); + // now just make sure we don't blow up trying to access a freed object! + heaptest.post(2); } -class TempSharedListener: public TempListener, -public boost::enable_shared_from_this -{ -public: -TempSharedListener(const std::string& name, bool& liveFlag): - TempListener(name, liveFlag) -{} -}; - -template<> template<> -void events_object::test<15>() -{ - set_test_name("listen(boost::bind(...TempSharedListener ref...))"); -#if 0 -bool live = false; -LLEventPump& heaptest(pumps.obtain("heaptest")); -LLBoundListener connection; -{ - // We MUST have at least one shared_ptr to an - // enable_shared_from_this subclass object before - // shared_from_this() can work. - boost::shared_ptr - tempListener(new TempSharedListener("temp", live)); - ensure("TempSharedListener constructed", live); - // However, we're not passing either the shared_ptr or its - // corresponding weak_ptr -- instead, we're passing a reference to - // the TempSharedListener. -/*==========================================================================*| - std::cout << "Capturing const ref" << std::endl; - const boost::enable_shared_from_this& cref(*tempListener); - std::cout << "Capturing const ptr" << std::endl; - const boost::enable_shared_from_this* cp(&cref); - std::cout << "Capturing non-const ptr" << std::endl; - boost::enable_shared_from_this* p(const_cast*>(cp)); - std::cout << "Capturing shared_from_this()" << std::endl; - boost::shared_ptr sp(p->shared_from_this()); - std::cout << "Capturing weak_ptr" << std::endl; - boost::weak_ptr wp(weaken(sp)); - std::cout << "Binding weak_ptr" << std::endl; -|*==========================================================================*/ - connection = heaptest.listen(tempListener->getName(), - boost::bind(&TempSharedListener::call, *tempListener, _1)); - heaptest.post(1); - check_listener("received", *tempListener, 1); -} // presumably this will make tempListener go away? -// verify that -ensure("TempSharedListener destroyed", ! live); -ensure("implicit disconnect", ! connection.connected()); -// now just make sure we don't blow up trying to access a freed object! -heaptest.post(2); -#endif // 0 -} } // namespace tut -- cgit v1.2.3 From 79a3e391d3c992925230ff01551747e7edccb0ca Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 17 Oct 2019 13:26:51 -0400 Subject: DRTVWR-476: Kill LLEventQueue, per-frame LLEventPump::flush() calls. 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. --- indra/test/llevents_tut.cpp | 57 +++++++-------------------------------------- 1 file changed, 9 insertions(+), 48 deletions(-) (limited to 'indra/test') diff --git a/indra/test/llevents_tut.cpp b/indra/test/llevents_tut.cpp index a8a3188249..17f64a4953 100644 --- a/indra/test/llevents_tut.cpp +++ b/indra/test/llevents_tut.cpp @@ -91,9 +91,7 @@ template<> template<> void events_object::test<1>() { set_test_name("basic operations"); - // Now there's a static constructor in llevents.cpp that registers on - // the "mainloop" pump to call LLEventPumps::flush(). - // Actually -- having to modify this to track the statically- + // Having to modify this to track the statically- // constructed pumps in other TUT modules in this giant monolithic test // executable isn't such a hot idea. // ensure_equals("initial pump", pumps.mPumpMap.size(), 1); @@ -209,43 +207,6 @@ bool chainEvents(Listener& someListener, const LLSD& event) template<> template<> void events_object::test<3>() -{ - set_test_name("LLEventQueue delayed action"); - // This access is NOT legal usage: we can do it only because we're - // hacking private for test purposes. Normally we'd either compile in - // a particular name, or (later) edit a config file. - pumps.mQueueNames.insert("login"); - LLEventPump& login(pumps.obtain("login")); - // The "mainloop" pump is special: posting on that implicitly calls - // LLEventPumps::flush(), which in turn should flush our "login" - // LLEventQueue. - LLEventPump& mainloop(pumps.obtain("mainloop")); - ensure("LLEventQueue leaf class", dynamic_cast (&login)); - listener0.listenTo(login); - listener0.reset(0); - login.post(1); - check_listener("waiting for queued event", listener0, 0); - mainloop.post(LLSD()); - check_listener("got queued event", listener0, 1); - login.stopListening(listener0.getName()); - // Verify that when an event handler posts a new event on the same - // LLEventQueue, it doesn't get processed in the same flush() call -- - // it waits until the next flush() call. - listener0.reset(17); - login.listen("chainEvents", boost::bind(chainEvents, boost::ref(listener0), _1)); - login.post(1); - check_listener("chainEvents(1) not yet called", listener0, 17); - mainloop.post(LLSD()); - check_listener("chainEvents(1) called", listener0, 1); - mainloop.post(LLSD()); - check_listener("chainEvents(0) called", listener0, 0); - mainloop.post(LLSD()); - check_listener("chainEvents(-1) not called", listener0, 0); - login.stopListening("chainEvents"); -} - -template<> template<> -void events_object::test<4>() { set_test_name("explicitly-instantiated LLEventStream"); // Explicitly instantiate an LLEventStream, and verify that it @@ -270,7 +231,7 @@ void events_object::test<4>() } template<> template<> -void events_object::test<5>() +void events_object::test<4>() { set_test_name("stopListening()"); LLEventPump& login(pumps.obtain("login")); @@ -284,7 +245,7 @@ void events_object::test<5>() } template<> template<> -void events_object::test<6>() +void events_object::test<5>() { set_test_name("chaining LLEventPump instances"); LLEventPump& upstream(pumps.obtain("upstream")); @@ -309,7 +270,7 @@ void events_object::test<6>() } template<> template<> -void events_object::test<7>() +void events_object::test<6>() { set_test_name("listener dependency order"); typedef LLEventPump::NameList NameList; @@ -391,7 +352,7 @@ void events_object::test<7>() } template<> template<> -void events_object::test<8>() +void events_object::test<7>() { set_test_name("tweaked and untweaked LLEventPump instance names"); { // nested scope @@ -423,7 +384,7 @@ void eventSource(const LLListenerOrPumpName& listener) } template<> template<> -void events_object::test<9>() +void events_object::test<8>() { set_test_name("LLListenerOrPumpName"); // Passing a boost::bind() expression to LLListenerOrPumpName @@ -464,7 +425,7 @@ private: }; template<> template<> -void events_object::test<10>() +void events_object::test<9>() { set_test_name("listen(boost::bind(...TempListener...))"); // listen() can't do anything about a plain TempListener instance: @@ -501,7 +462,7 @@ public: }; template<> template<> -void events_object::test<11>() +void events_object::test<10>() { set_test_name("listen(boost::bind(...TempTrackableListener ref...))"); bool live = false; @@ -524,7 +485,7 @@ void events_object::test<11>() } template<> template<> -void events_object::test<12>() +void events_object::test<11>() { set_test_name("listen(boost::bind(...TempTrackableListener pointer...))"); bool live = false; -- cgit v1.2.3 From 28a54c2f7b25b9fda763c51746701f0cd21c8018 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 22 Oct 2019 16:49:29 -0400 Subject: DRTVWR-476: Infrastructure to help manage long-lived coroutines. Introduce LLCoros::Stop exception, with subclasses Stopping, Stopped and Shutdown. Add LLCoros::checkStop(), intended to be called periodically by any coroutine with nontrivial lifespan. It checks the LLApp status and, unless isRunning(), throws one of these new exceptions. Make LLCoros::toplevel() catch Stop specially and log forcible coroutine termination. Now that LLApp status matters even in a test program, introduce a trivial LLTestApp subclass whose sole function is to make isRunning() true. (LLApp::setStatus() is protected: only a subclass can call it.) Add LLTestApp instances to lleventcoro_test.cpp and lllogin_test.cpp. Make LLCoros::toplevel() accept parameters by value rather than by const reference so we can continue using them even after context switches. Make private LLCoros::get_CoroData() static. Given that we've observed some coroutines living past LLCoros destruction, making the caller call LLCoros::instance() is more dangerous than encapsulating it within a static method -- since the encapsulated call can check LLCoros::wasDeleted() first and do something reasonable instead. This also eliminates the need for both a const and non-const overload. Defend LLCoros::delete_CoroData() (cleanup function for fiber_specific_ptr for CoroData, implicitly called after coroutine termination) against calls after ~LLCoros(). Add a status string to coroutine-local data, with LLCoro::setStatus(), getStatus() and RAII class TempStatus. Add an optional 'when' string argument to LLCoros::printActiveCoroutines(). Make ~LLCoros() print the coroutines still active at destruction. --- indra/test/lltestapp.h | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 indra/test/lltestapp.h (limited to 'indra/test') diff --git a/indra/test/lltestapp.h b/indra/test/lltestapp.h new file mode 100644 index 0000000000..382516cd2b --- /dev/null +++ b/indra/test/lltestapp.h @@ -0,0 +1,34 @@ +/** + * @file lltestapp.h + * @author Nat Goodspeed + * @date 2019-10-21 + * @brief LLApp subclass useful for testing. + * + * $LicenseInfo:firstyear=2019&license=viewerlgpl$ + * Copyright (c) 2019, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_LLTESTAPP_H) +#define LL_LLTESTAPP_H + +#include "llapp.h" + +/** + * LLTestApp is a dummy LLApp that simply sets LLApp::isRunning() for anyone + * who cares. + */ +class LLTestApp: public LLApp +{ +public: + LLTestApp() + { + setStatus(APP_STATUS_RUNNING); + } + + bool init() { return true; } + bool cleanup() { return true; } + bool frame() { return true; } +}; + +#endif /* ! defined(LL_LLTESTAPP_H) */ -- cgit v1.2.3 From 39f4acd92144546d346ecd63224945da8d64c5db Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 15 Nov 2019 08:11:32 -0500 Subject: DRTVWR-476: Conflate LOGFAIL env var empty with completely unset. Sometimes it's useful to be able to temporarily override an existing LOGFAIL setting in the current environment. It's far more convenient to prepend LOGFAIL='' to a command than to 'unset LOGFAIL' as a whole separate command -- and then remember to restore its previous value. --- indra/test/test.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'indra/test') diff --git a/indra/test/test.cpp b/indra/test/test.cpp index 51f0e80043..6b342ffe89 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -631,8 +631,9 @@ int main(int argc, char **argv) const char* LOGFAIL = getenv("LOGFAIL"); boost::shared_ptr replayer; // As described in stream_usage(), LOGFAIL overrides both --debug and - // LOGTEST. - if (LOGFAIL) + // LOGTEST. But allow user to set LOGFAIL empty to revert to LOGTEST + // and/or --debug. + if (LOGFAIL && *LOGFAIL) { LLError::ELevel level = LLError::decodeLevel(LOGFAIL); replayer.reset(new LLReplayLogReal(level, gAPRPoolp)); -- cgit v1.2.3 From 3cd2beb97ef0d368d47b0b7efd242b3c709d01af Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 9 Dec 2019 14:33:56 -0500 Subject: DRTVWR-476: Make Sync::bump() atomic, add set() method. Using Sync with multiple threads is trickier than with coroutines. In particular, Sync::bump() was racy (get() and set() as two different operations), and threads were proceeding when they should have waited. Fortunately LLCond, on which Sync is based, already supports atomic update operations. Use that for bump(). But to nail things down even more specifically, add set(n) to complement yield_until(n). Using those methods, there should be no ambiguity about which call in one thread synchronizes with which call in the other thread. --- indra/test/sync.h | 51 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 10 deletions(-) (limited to 'indra/test') diff --git a/indra/test/sync.h b/indra/test/sync.h index cafbc034b4..0f0b6cece8 100644 --- a/indra/test/sync.h +++ b/indra/test/sync.h @@ -41,18 +41,49 @@ public: mTimeout(timeout) {} - /// Bump mCond by n steps -- ideally, do this every time a participating - /// coroutine wakes up from any suspension. The choice to bump() after - /// resumption rather than just before suspending is worth calling out: - /// this practice relies on the fact that condition_variable::notify_all() - /// merely marks a suspended coroutine ready to run, rather than - /// immediately resuming it. This way, though, even if a coroutine exits - /// before reaching its next suspend point, the other coroutine isn't - /// left waiting forever. + /** + * Bump mCond by n steps -- ideally, do this every time a participating + * coroutine wakes up from any suspension. The choice to bump() after + * resumption rather than just before suspending is worth calling out: + * this practice relies on the fact that condition_variable::notify_all() + * merely marks a suspended coroutine ready to run, rather than + * immediately resuming it. This way, though, even if a coroutine exits + * before reaching its next suspend point, the other coroutine isn't + * left waiting forever. + */ void bump(int n=1) { - LL_DEBUGS() << llcoro::logname() << " bump(" << n << ") -> " << (mCond.get() + n) << LL_ENDL; - mCond.set_all(mCond.get() + n); + // Calling mCond.set_all(mCond.get() + n) would be great for + // coroutines -- but not so good between kernel threads -- it would be + // racy. Make the increment atomic by calling update_all(), which runs + // the passed lambda within a mutex lock. + int updated; + mCond.update_all( + [&n, &updated](int& data) + { + data += n; + // Capture the new value for possible logging purposes. + updated = data; + }); + // In the multi-threaded case, this log message could be a bit + // misleading, as it will be emitted after waiting threads have + // already awakened. But emitting the log message within the lock + // would seem to hold the lock longer than we really ought. + LL_DEBUGS() << llcoro::logname() << " bump(" << n << ") -> " << updated << LL_ENDL; + } + + /** + * Set mCond to a specific n. Use of bump() and yield() is nicely + * maintainable, since you can insert or delete matching operations in a + * test function and have the rest of the Sync operations continue to + * line up as before. But sometimes you need to get very specific, which + * is where set() and yield_until() come in handy: less maintainable, + * more precise. + */ + void set(int n) + { + LL_DEBUGS() << llcoro::logname() << " set(" << n << ")" << LL_ENDL; + mCond.set_all(n); } /// suspend until "somebody else" has bumped mCond by n steps -- cgit v1.2.3 From dc07509f296661cf7a4d4bceb88a1a897757de98 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 3 Apr 2020 10:38:53 -0400 Subject: DRTVWR-476: Cherry-pick debug aids from commit 77b0c53 (fiber-mutex) --- indra/test/chained_callback.h | 105 ++++++++++++++++++++++++++++++++++++++++++ indra/test/debug.h | 40 ++++++++++++---- indra/test/print.h | 42 +++++++++++++++++ indra/test/test.cpp | 105 ++++++++++++++++++++++-------------------- 4 files changed, 233 insertions(+), 59 deletions(-) create mode 100644 indra/test/chained_callback.h create mode 100644 indra/test/print.h (limited to 'indra/test') diff --git a/indra/test/chained_callback.h b/indra/test/chained_callback.h new file mode 100644 index 0000000000..41a7f7c9fa --- /dev/null +++ b/indra/test/chained_callback.h @@ -0,0 +1,105 @@ +/** + * @file chained_callback.h + * @author Nat Goodspeed + * @date 2020-01-03 + * @brief Subclass of tut::callback used for chaining callbacks. + * + * $LicenseInfo:firstyear=2020&license=viewerlgpl$ + * Copyright (c) 2020, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_CHAINED_CALLBACK_H) +#define LL_CHAINED_CALLBACK_H + +/** + * Derive your TUT callback from chained_callback instead of tut::callback to + * ensure that multiple such callbacks can coexist in a given test executable. + * The relevant callback method will be called for each callback instance in + * reverse order of the instance's link() methods being called: the most + * recently link()ed callback will be called first, then the previous, and so + * forth. + * + * Obviously, for this to work, all relevant callbacks must be derived from + * chained_callback instead of tut::callback. Given that, control should reach + * each of them regardless of their construction order. The chain is + * guaranteed to stop because the first link() call will link to test_runner's + * default_callback, which is simply an instance of the callback() base class. + * + * The rule for deriving from chained_callback is that you may override any of + * its virtual methods, but your override must at some point call the + * corresponding chained_callback method. + */ +class chained_callback: public tut::callback +{ +public: + /** + * Instead of calling tut::test_runner::set_callback(&your_callback), call + * your_callback.link(); + * This uses the canonical instance of tut::test_runner. + */ + void link() + { + link(tut::runner.get()); + } + + /** + * If for some reason you have a different instance of test_runner... + */ + void link(tut::test_runner& runner) + { + // Since test_runner's constructor sets a default callback, + // get_callback() will always return a reference to a valid callback + // instance. + mPrev = &runner.get_callback(); + runner.set_callback(this); + } + + /** + * Called when new test run started. + */ + virtual void run_started() + { + mPrev->run_started(); + } + + /** + * Called when a group started + * @param name Name of the group + */ + virtual void group_started(const std::string& name) + { + mPrev->group_started(name); + } + + /** + * Called when a test finished. + * @param tr Test results. + */ + virtual void test_completed(const tut::test_result& tr) + { + mPrev->test_completed(tr); + } + + /** + * Called when a group is completed + * @param name Name of the group + */ + virtual void group_completed(const std::string& name) + { + mPrev->group_completed(name); + } + + /** + * Called when all tests in run completed. + */ + virtual void run_completed() + { + mPrev->run_completed(); + } + +private: + tut::callback* mPrev; +}; + +#endif /* ! defined(LL_CHAINED_CALLBACK_H) */ diff --git a/indra/test/debug.h b/indra/test/debug.h index 33c3ea2d27..76dbb973b2 100644 --- a/indra/test/debug.h +++ b/indra/test/debug.h @@ -29,37 +29,59 @@ #if ! defined(LL_DEBUG_H) #define LL_DEBUG_H -#include +#include "print.h" /***************************************************************************** * Debugging stuff *****************************************************************************/ -// This class is intended to illuminate entry to a given block, exit from the -// same block and checkpoints along the way. It also provides a convenient -// place to turn std::cout output on and off. +/** + * This class is intended to illuminate entry to a given block, exit from the + * same block and checkpoints along the way. It also provides a convenient + * place to turn std::cerr output on and off. + * + * If the environment variable LOGTEST is non-empty, each Debug instance will + * announce its construction and destruction, presumably at entry and exit to + * the block in which it's declared. Moreover, any arguments passed to its + * operator()() will be streamed to std::cerr, prefixed by the block + * description. + * + * The variable LOGTEST is used because that's the environment variable + * checked by test.cpp, our TUT main() program, to turn on LLError logging. It + * is expected that Debug is solely for use in test programs. + */ class Debug { public: Debug(const std::string& block): - mBlock(block) + mBlock(block), + mLOGTEST(getenv("LOGTEST")), + // debug output enabled when LOGTEST is set AND non-empty + mEnabled(mLOGTEST && *mLOGTEST) { (*this)("entry"); } + // non-copyable + Debug(const Debug&) = delete; + ~Debug() { (*this)("exit"); } - void operator()(const std::string& status) + template + void operator()(ARGS&&... args) { -#if defined(DEBUG_ON) - std::cout << mBlock << ' ' << status << std::endl; -#endif + if (mEnabled) + { + print(mBlock, ' ', std::forward(args)...); + } } private: const std::string mBlock; + const char* mLOGTEST; + bool mEnabled; }; // It's often convenient to use the name of the enclosing function as the name diff --git a/indra/test/print.h b/indra/test/print.h new file mode 100644 index 0000000000..08e36caddf --- /dev/null +++ b/indra/test/print.h @@ -0,0 +1,42 @@ +/** + * @file print.h + * @author Nat Goodspeed + * @date 2020-01-02 + * @brief print() function for debugging + * + * $LicenseInfo:firstyear=2020&license=viewerlgpl$ + * Copyright (c) 2020, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_PRINT_H) +#define LL_PRINT_H + +#include + +// print(..., NONL); +// leaves the output dangling, suppressing the normally appended std::endl +struct NONL_t {}; +#define NONL (NONL_t()) + +// normal recursion end +inline +void print() +{ + std::cerr << std::endl; +} + +// print(NONL) is a no-op +inline +void print(NONL_t) +{ +} + +template +void print(T&& first, ARGS&&... rest) +{ + std::cerr << first; + print(std::forward(rest)...); +} + +#endif /* ! defined(LL_PRINT_H) */ diff --git a/indra/test/test.cpp b/indra/test/test.cpp index 6b342ffe89..ea54ba658e 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -37,6 +37,7 @@ #include "linden_common.h" #include "llerrorcontrol.h" #include "lltut.h" +#include "chained_callback.h" #include "stringize.h" #include "namedtempfile.h" #include "lltrace.h" @@ -71,7 +72,6 @@ #include #include #include -#include #include @@ -172,8 +172,10 @@ private: LLError::RecorderPtr mRecorder; }; -class LLTestCallback : public tut::callback +class LLTestCallback : public chained_callback { + typedef chained_callback super; + public: LLTestCallback(bool verbose_mode, std::ostream *stream, boost::shared_ptr replayer) : @@ -184,7 +186,7 @@ public: mSkippedTests(0), // By default, capture a shared_ptr to std::cout, with a no-op "deleter" // so that destroying the shared_ptr makes no attempt to delete std::cout. - mStream(boost::shared_ptr(&std::cout, boost::lambda::_1)), + mStream(boost::shared_ptr(&std::cout, [](std::ostream*){})), mReplayer(replayer) { if (stream) @@ -205,22 +207,25 @@ public: ~LLTestCallback() { - } + } virtual void run_started() { //std::cout << "run_started" << std::endl; LL_INFOS("TestRunner")<<"Test Started"<< LL_ENDL; + super::run_started(); } virtual void group_started(const std::string& name) { LL_INFOS("TestRunner")<<"Unit test group_started name=" << name << LL_ENDL; *mStream << "Unit test group_started name=" << name << std::endl; + super::group_started(name); } virtual void group_completed(const std::string& name) { LL_INFOS("TestRunner")<<"Unit test group_completed name=" << name << LL_ENDL; *mStream << "Unit test group_completed name=" << name << std::endl; + super::group_completed(name); } virtual void test_completed(const tut::test_result& tr) @@ -282,6 +287,7 @@ public: *mStream << std::endl; } LL_INFOS("TestRunner")< replayer; - // As described in stream_usage(), LOGFAIL overrides both --debug and - // LOGTEST. But allow user to set LOGFAIL empty to revert to LOGTEST - // and/or --debug. - if (LOGFAIL && *LOGFAIL) + boost::shared_ptr replayer{boost::make_shared()}; + + // Testing environment variables for both 'set' and 'not empty' allows a + // user to suppress a pre-existing environment variable by forcing empty. + if (LOGTEST && *LOGTEST) { - LLError::ELevel level = LLError::decodeLevel(LOGFAIL); - replayer.reset(new LLReplayLogReal(level, gAPRPoolp)); + LLError::initForApplication(".", ".", true /* log to stderr */); + LLError::setDefaultLevel(LLError::decodeLevel(LOGTEST)); } else { - replayer.reset(new LLReplayLog()); + LLError::initForApplication(".", ".", false /* do not log to stderr */); + LLError::setDefaultLevel(LLError::LEVEL_DEBUG); + if (LOGFAIL && *LOGFAIL) + { + LLError::ELevel level = LLError::decodeLevel(LOGFAIL); + replayer.reset(new LLReplayLogReal(level, gAPRPoolp)); + } } + LLError::setFatalFunction(wouldHaveCrashed); + std::string test_app_name(argv[0]); + std::string test_log = test_app_name + ".log"; + LLFile::remove(test_log); + LLError::logToFile(test_log); + +#ifdef CTYPE_WORKAROUND + ctype_workaround(); +#endif + + if (!sMasterThreadRecorder) + { + sMasterThreadRecorder = new LLTrace::ThreadRecorder(); + LLTrace::set_master_thread_recorder(sMasterThreadRecorder); + } + + // run the tests LLTestCallback* mycallback; if (getenv("TEAMCITY_PROJECT_NAME")) @@ -653,7 +657,8 @@ int main(int argc, char **argv) mycallback = new LLTestCallback(verbose_mode, output.get(), replayer); } - tut::runner.get().set_callback(mycallback); + // a chained_callback subclass must be linked with previous + mycallback->link(); if(test_group.empty()) { -- cgit v1.2.3 From 962ccb4f01f220850fea35e32c3d92a718d35631 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 2 Apr 2020 13:14:31 -0400 Subject: DRTVWR-476: Facilitate debugging test programs with logging. On Mac, even if you run a test program with --debug or set LOGTEST=DEBUG, it won't log to stderr if you're filtering build output or running the build in an emacs compile buffer. This is because, on Mac, a viewer launched by mouse rather than from the command line is passed a stderr stream that ultimately gets logged to the system Console. The shouldLogToStderr() function is intended to avoid spamming the Console with the (voluminous) viewer log output. It tests whether stderr isatty() and, if not, suppresses calling LLError::logToStderr(). This makes debugging test programs using log output trickier than necessary. Change shouldLogToStderr() to permit logging when either stderr isatty() or is a pipe. The original intention is preserved in that empirically, a viewer launched by mouse is passed a stderr stream identified as a character device rather than as a pipe. Also introduce SetEnv, a class that facilitates setting (e.g.) LOGTEST=DEBUG for specific test programs without setting it for all test programs in the build. Using the constructor for a static object means you can set environment variables before main() is entered, which is important because it's the main() function in test.cpp that acts on the LOGTEST and LOGFAIL environment variables. These changes make it unnecessary to retain the temporary change in test.cpp to force LOGTEST to DEBUG. --- indra/test/setenv.h | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++ indra/test/test.cpp | 3 --- 2 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 indra/test/setenv.h (limited to 'indra/test') diff --git a/indra/test/setenv.h b/indra/test/setenv.h new file mode 100644 index 0000000000..ed2de9ccca --- /dev/null +++ b/indra/test/setenv.h @@ -0,0 +1,66 @@ +/** + * @file setenv.h + * @author Nat Goodspeed + * @date 2020-04-01 + * @brief Provide a way for a particular test program to alter the + * environment before entry to main(). + * + * $LicenseInfo:firstyear=2020&license=viewerlgpl$ + * Copyright (c) 2020, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_SETENV_H) +#define LL_SETENV_H + +#include // setenv() + +/** + * Our test.cpp main program responds to environment variables LOGTEST and + * LOGFAIL. But if you set (e.g.) LOGTEST=DEBUG before a viewer build, @em + * every test program in the build emits debug log output. This can be so + * voluminous as to slow down the build. + * + * With an integration test program, you can specifically build (e.g.) the + * INTEGRATION_TEST_llstring target, and set any environment variables you + * want for that. But with a unit test program, since executing the program is + * a side effect rather than an explicit target, specifically building (e.g.) + * PROJECT_lllogin_TEST_lllogin only builds the executable without running it. + * + * To set an environment variable for a particular test program, declare a + * static instance of SetEnv in its .cpp file. SetEnv's constructor takes + * pairs of strings, e.g. + * + * @code + * static SetEnv sLOGGING("LOGTEST", "INFO"); + * @endcode + * + * Declaring a static instance of SetEnv is important because that ensures + * that the environment variables are set before main() is entered, since it + * is main() that examines LOGTEST and LOGFAIL. + */ +struct SetEnv +{ + // degenerate constructor, terminate recursion + SetEnv() {} + + /** + * SetEnv() accepts an arbitrary number of pairs of strings: variable + * name, value, variable name, value ... Entering the constructor sets + * those variables in the process environment using Posix setenv(), + * overriding any previous value. If static SetEnv declarations in + * different translation units specify overlapping sets of variable names, + * it is indeterminate which instance will "win." + */ + template + SetEnv(VAR&& var, VAL&& val, ARGS&&... rest): + // constructor forwarding handles the tail of the list + SetEnv(std::forward(rest)...) + { + // set just the first (variable, value) pair + // 1 means override previous value if any + setenv(std::forward(var), std::forward(val), 1); + } +}; + +#endif /* ! defined(LL_SETENV_H) */ diff --git a/indra/test/test.cpp b/indra/test/test.cpp index ea54ba658e..87c4a8d8a3 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -544,9 +544,6 @@ int main(int argc, char **argv) // LOGTEST overrides default, but can be overridden by --debug. const char* LOGTEST = getenv("LOGTEST"); - // DELETE - LOGTEST = "DEBUG"; - // values used for options parsing apr_status_t apr_err; const char* opt_arg = NULL; -- cgit v1.2.3 From 5ced20b1a9b76f361e92088d1464615fba1b1697 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 2 Apr 2020 14:51:48 -0400 Subject: DRTVWR-476: chained_callback.h depends on lltut.h. #include it. --- indra/test/chained_callback.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'indra/test') diff --git a/indra/test/chained_callback.h b/indra/test/chained_callback.h index 41a7f7c9fa..05929e33ad 100644 --- a/indra/test/chained_callback.h +++ b/indra/test/chained_callback.h @@ -12,6 +12,8 @@ #if ! defined(LL_CHAINED_CALLBACK_H) #define LL_CHAINED_CALLBACK_H +#include "lltut.h" + /** * Derive your TUT callback from chained_callback instead of tut::callback to * ensure that multiple such callbacks can coexist in a given test executable. -- cgit v1.2.3 From d979ba68ee91404d5d3037701e49eb26286109f5 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 3 Apr 2020 10:54:37 -0400 Subject: DRTVWR-476: Use a longer default timeout for Sync class. The timeout is meant to prevent a deadlocked test program from hanging a build. It's not intended to ensure some sort of SLA for the operations under test. Empirically, using a longer timeout helps some test programs. The only downside of increasing the timeout is that if some test does hang, it takes longer to notice. But changes on the order of a few seconds are negligible. --- indra/test/sync.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/test') diff --git a/indra/test/sync.h b/indra/test/sync.h index 0f0b6cece8..ca8b7262d6 100644 --- a/indra/test/sync.h +++ b/indra/test/sync.h @@ -37,7 +37,7 @@ class Sync F32Milliseconds mTimeout; public: - Sync(F32Milliseconds timeout=F32Milliseconds(10.0f)): + Sync(F32Milliseconds timeout=F32Milliseconds(10000.0f)): mTimeout(timeout) {} -- cgit v1.2.3