From b754eba78b318eed3bfd620d90e76e624ec4e6c2 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 4 Oct 2019 08:45:00 -0400 Subject: [PATCH 01/38] DRTVWR-476: Fix Windows line endings --- indra/llcommon/StackWalker.cpp | 4 ++-- indra/llcommon/llstacktrace.cpp | 2 +- indra/newview/llwindebug.h | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/indra/llcommon/StackWalker.cpp b/indra/llcommon/StackWalker.cpp index 7a7230db4b..56defc6465 100644 --- a/indra/llcommon/StackWalker.cpp +++ b/indra/llcommon/StackWalker.cpp @@ -98,7 +98,7 @@ // If VC7 and later, then use the shipped 'dbghelp.h'-file #pragma pack(push,8) #if _MSC_VER >= 1300 -#pragma warning (push) +#pragma warning (push) #pragma warning (disable:4091) // a microsoft header has warnings. Very nice. #include #pragma warning (pop) @@ -660,7 +660,7 @@ private: pGMI = (tGMI) GetProcAddress( hPsapi, "GetModuleInformation" ); if ( (pEPM == NULL) || (pGMFNE == NULL) || (pGMBN == NULL) || (pGMI == NULL) ) { - // we couldnīt find all functions + // we couldn't find all functions FreeLibrary(hPsapi); return FALSE; } diff --git a/indra/llcommon/llstacktrace.cpp b/indra/llcommon/llstacktrace.cpp index 7084fe6f60..80057bf0f2 100644 --- a/indra/llcommon/llstacktrace.cpp +++ b/indra/llcommon/llstacktrace.cpp @@ -33,7 +33,7 @@ #include #include "llwin32headerslean.h" -#pragma warning (push) +#pragma warning (push) #pragma warning (disable:4091) // a microsoft header has warnings. Very nice. #include #pragma warning (pop) diff --git a/indra/newview/llwindebug.h b/indra/newview/llwindebug.h index 05f245b311..524adba652 100644 --- a/indra/newview/llwindebug.h +++ b/indra/newview/llwindebug.h @@ -29,8 +29,8 @@ #include "stdtypes.h" #include "llwin32headerslean.h" - -#pragma warning (push) + +#pragma warning (push) #pragma warning (disable:4091) // a microsoft header has warnings. Very nice. #include #pragma warning (pop) From 4aa4a52c08d6180c58beb93991388789c91328de Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 7 Oct 2019 17:18:29 -0400 Subject: [PATCH 02/38] DRTVWR-476: Fix overflow case in llcoro::postAndSuspend(). 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. --- indra/llcommon/lleventcoro.cpp | 35 ++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/indra/llcommon/lleventcoro.cpp b/indra/llcommon/lleventcoro.cpp index 96e1125622..7558e54e88 100644 --- a/indra/llcommon/lleventcoro.cpp +++ b/indra/llcommon/lleventcoro.cpp @@ -177,20 +177,27 @@ LLBoundListener postAndSuspendSetup(const std::string& callerName, // make a callback that will assign a value to the future, and listen on // the specified LLEventPump with that callback LLBoundListener connection( - replyPump.getPump().listen(listenerName, - [&promise, consuming, listenerName](const LLSD& result) - { - try - { - promise.set_value(result); - } - catch(boost::fibers::promise_already_satisfied & ex) - { - LL_WARNS("lleventcoro") << "promise already satisfied in '" - << listenerName << "' " << ex.what() << LL_ENDL; - } - return consuming; - })); + replyPump.getPump().listen( + listenerName, + [&promise, consuming, listenerName](const LLSD& result) + { + try + { + promise.set_value(result); + // We did manage to propagate the result value to the + // (real) listener. If we're supposed to indicate that + // we've consumed it, do so. + return consuming; + } + catch(boost::fibers::promise_already_satisfied & ex) + { + LL_WARNS("lleventcoro") << "promise already satisfied in '" + << listenerName << "' " << ex.what() << LL_ENDL; + // We could not propagate the result value to the + // listener. + return false; + } + })); // skip the "post" part if requestPump is default-constructed if (requestPump) { From 18bb6d5d496689a4d04c301340018b160e46b825 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 14 Oct 2019 15:41:09 -0400 Subject: [PATCH 03/38] 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/llcommon/llerror.cpp | 200 ++++++++++++++++++++------------ indra/llcommon/llerrorcontrol.h | 1 + indra/test/test.cpp | 3 + 3 files changed, 129 insertions(+), 75 deletions(-) diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 1c3a9d6d67..2e8e18b450 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -118,27 +118,28 @@ namespace { class RecordToFile : public LLError::Recorder { public: - RecordToFile(const std::string& filename) + RecordToFile(const std::string& filename): + mName(filename) { mFile.open(filename.c_str(), std::ios_base::out | std::ios_base::app); if (!mFile) { LL_INFOS() << "Error setting log file to " << filename << LL_ENDL; } - else - { - if (!LLError::getAlwaysFlush()) - { - mFile.sync_with_stdio(false); - } - } + else + { + if (!LLError::getAlwaysFlush()) + { + mFile.sync_with_stdio(false); + } + } } - + ~RecordToFile() { mFile.close(); } - + virtual bool enabled() override { #ifdef LL_RELEASE_FOR_DOWNLOAD @@ -148,11 +149,13 @@ namespace { #endif } - bool okay() { return mFile.good(); } - - virtual void recordMessage(LLError::ELevel level, - const std::string& message) override - { + bool okay() const { return mFile.good(); } + + std::string getFilename() const { return mName; } + + virtual void recordMessage(LLError::ELevel level, + const std::string& message) override + { if (LLError::getAlwaysFlush()) { mFile << message << std::endl; @@ -161,9 +164,10 @@ namespace { { mFile << message << "\n"; } - } - + } + private: + const std::string mName; llofstream mFile; }; @@ -171,7 +175,7 @@ namespace { class RecordToStderr : public LLError::Recorder { public: - RecordToStderr(bool timestamp) : mUseANSI(ANSI_PROBE) + RecordToStderr(bool timestamp) : mUseANSI(checkANSI()) { this->showMultiline(true); } @@ -184,10 +188,7 @@ namespace { virtual void recordMessage(LLError::ELevel level, const std::string& message) override { - if (ANSI_PROBE == mUseANSI) - mUseANSI = (checkANSI() ? ANSI_YES : ANSI_NO); - - if (ANSI_YES == mUseANSI) + if (mUseANSI) { // Default all message levels to bold so we can distinguish our own messages from those dumped by subprocesses and libraries. colorANSI("1"); // bold @@ -206,16 +207,11 @@ namespace { } } fprintf(stderr, "%s\n", message.c_str()); - if (ANSI_YES == mUseANSI) colorANSI("0"); // reset + if (mUseANSI) colorANSI("0"); // reset } private: - enum ANSIState - { - ANSI_PROBE, - ANSI_YES, - ANSI_NO - } mUseANSI; + bool mUseANSI; void colorANSI(const std::string color) { @@ -223,7 +219,7 @@ namespace { fprintf(stderr, "\033[%sm", color.c_str() ); }; - bool checkANSI(void) + static bool checkANSI(void) { #if LL_LINUX || LL_DARWIN // Check whether it's okay to use ANSI; if stderr is @@ -484,14 +480,11 @@ namespace LLError LLError::FatalFunction mCrashFunction; LLError::TimeFunction mTimeFunction; - + Recorders mRecorders; - RecorderPtr mFileRecorder; - RecorderPtr mFixedBufferRecorder; - std::string mFileRecorderFileName; - - int mShouldLogCallCounter; - + + int mShouldLogCallCounter; + private: SettingsConfig(); }; @@ -525,9 +518,6 @@ namespace LLError mCrashFunction(NULL), mTimeFunction(NULL), mRecorders(), - mFileRecorder(), - mFixedBufferRecorder(), - mFileRecorderFileName(), mShouldLogCallCounter(0) { } @@ -679,20 +669,19 @@ namespace void commonInit(const std::string& user_dir, const std::string& app_dir, bool log_to_stderr = true) { LLError::Settings::getInstance()->reset(); - + LLError::setDefaultLevel(LLError::LEVEL_INFO); - LLError::setAlwaysFlush(true); - LLError::setEnabledLogTypesMask(0xFFFFFFFF); + LLError::setAlwaysFlush(true); + LLError::setEnabledLogTypesMask(0xFFFFFFFF); LLError::setFatalFunction(LLError::crashAndLoop); LLError::setTimeFunction(LLError::utcTime); // log_to_stderr is only false in the unit and integration tests to keep builds quieter if (log_to_stderr && shouldLogToStderr()) { - LLError::RecorderPtr recordToStdErr(new RecordToStderr(stderrLogWantsTime())); - LLError::addRecorder(recordToStdErr); + LLError::logToStderr(); } - + #if LL_WINDOWS LLError::RecorderPtr recordToWinDebug(new RecordToWinDebug()); LLError::addRecorder(recordToWinDebug); @@ -990,49 +979,110 @@ namespace LLError s->mRecorders.erase(std::remove(s->mRecorders.begin(), s->mRecorders.end(), recorder), s->mRecorders.end()); } + + // Find an entry in SettingsConfig::mRecorders whose RecorderPtr points to + // a Recorder subclass of type RECORDER. Return, not a RecorderPtr (which + // points to the Recorder base class), but a shared_ptr which + // specifically points to the concrete RECORDER subclass instance, along + // with a Recorders::iterator indicating the position of that entry in + // mRecorders. The shared_ptr might be empty (operator!() returns true) if + // there was no such RECORDER subclass instance in mRecorders. + template + std::pair, Recorders::iterator> + findRecorderPos() + { + SettingsConfigPtr s = Settings::instance().getSettingsConfig(); + // Since we promise to return an iterator, use a classic iterator + // loop. + auto end{s->mRecorders.end()}; + for (Recorders::iterator it{s->mRecorders.begin()}; it != end; ++it) + { + // *it is a RecorderPtr, a shared_ptr. Use a + // dynamic_pointer_cast to try to downcast to test if it's also a + // shared_ptr. + auto ptr = boost::dynamic_pointer_cast(*it); + if (ptr) + { + // found the entry we want + return { ptr, it }; + } + } + // dropped out of the loop without finding any such entry -- instead + // of default-constructing Recorders::iterator (which might or might + // not be valid), return a value that is valid but not dereferenceable. + return { {}, end }; + } + + // Find an entry in SettingsConfig::mRecorders whose RecorderPtr points to + // a Recorder subclass of type RECORDER. Return, not a RecorderPtr (which + // points to the Recorder base class), but a shared_ptr which + // specifically points to the concrete RECORDER subclass instance. The + // shared_ptr might be empty (operator!() returns true) if there was no + // such RECORDER subclass instance in mRecorders. + template + boost::shared_ptr findRecorder() + { + return findRecorderPos().first; + } + + // Remove an entry from SettingsConfig::mRecorders whose RecorderPtr + // points to a Recorder subclass of type RECORDER. Return true if there + // was one and we removed it, false if there wasn't one to start with. + template + bool removeRecorder() + { + auto found = findRecorderPos(); + if (found.first) + { + SettingsConfigPtr s = Settings::instance().getSettingsConfig(); + s->mRecorders.erase(found.second); + } + return bool(found.first); + } } namespace LLError { void logToFile(const std::string& file_name) { - SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); + // remove any previous Recorder filling this role + removeRecorder(); - removeRecorder(s->mFileRecorder); - s->mFileRecorder.reset(); - s->mFileRecorderFileName.clear(); - if (!file_name.empty()) { - RecorderPtr recordToFile(new RecordToFile(file_name)); - if (boost::dynamic_pointer_cast(recordToFile)->okay()) - { - s->mFileRecorderFileName = file_name; - s->mFileRecorder = recordToFile; - addRecorder(recordToFile); - } + boost::shared_ptr recordToFile(new RecordToFile(file_name)); + if (recordToFile->okay()) + { + addRecorder(recordToFile); + } } } - - void logToFixedBuffer(LLLineBuffer* fixedBuffer) - { - SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); - - removeRecorder(s->mFixedBufferRecorder); - s->mFixedBufferRecorder.reset(); - - if (fixedBuffer) - { - RecorderPtr recordToFixedBuffer(new RecordToFixedBuffer(fixedBuffer)); - s->mFixedBufferRecorder = recordToFixedBuffer; - addRecorder(recordToFixedBuffer); - } - } std::string logFileName() { - SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); - return s->mFileRecorderFileName; + auto found = findRecorder(); + return found? found->getFilename() : std::string(); + } + + void logToStderr() + { + if (! findRecorder()) + { + RecorderPtr recordToStdErr(new RecordToStderr(stderrLogWantsTime())); + addRecorder(recordToStdErr); + } + } + + void logToFixedBuffer(LLLineBuffer* fixedBuffer) + { + // remove any previous Recorder filling this role + removeRecorder(); + + if (fixedBuffer) + { + RecorderPtr recordToFixedBuffer(new RecordToFixedBuffer(fixedBuffer)); + addRecorder(recordToFixedBuffer); + } } } diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h index 276d22fc36..bfa2269025 100644 --- a/indra/llcommon/llerrorcontrol.h +++ b/indra/llcommon/llerrorcontrol.h @@ -183,6 +183,7 @@ namespace LLError // each error message is passed to each recorder via recordMessage() LL_COMMON_API void logToFile(const std::string& filename); + LL_COMMON_API void logToStderr(); LL_COMMON_API void logToFixedBuffer(LLLineBuffer*); // Utilities to add recorders for logging to a file or a fixed buffer // A second call to the same function will remove the logger added diff --git a/indra/test/test.cpp b/indra/test/test.cpp index 861ec1d942..d7a7e4df21 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -612,6 +612,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': From 64ac480afefb0f87e76a43238da8b8a42ec3017c Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 14 Oct 2019 15:43:06 -0400 Subject: [PATCH 04/38] DRTVWR-476: Engage variadic llmake() implementation. --- indra/llcommon/llmake.h | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/indra/llcommon/llmake.h b/indra/llcommon/llmake.h index 08744f90fb..d3538aa693 100644 --- a/indra/llcommon/llmake.h +++ b/indra/llcommon/llmake.h @@ -25,37 +25,10 @@ #if ! defined(LL_LLMAKE_H) #define LL_LLMAKE_H -/*==========================================================================*| -// When we allow ourselves to compile with C++11 features enabled, this form -// should generically handle an arbitrary number of arguments. - template class CLASS_TEMPLATE, typename... ARGS> CLASS_TEMPLATE llmake(ARGS && ... args) { return CLASS_TEMPLATE(std::forward(args)...); } -|*==========================================================================*/ - -// As of 2015-12-18, this is what we'll use instead. Add explicit overloads -// for different numbers of template parameters as use cases arise. - -/** - * Usage: llmake(arg) - * - * Deduces the type T of 'arg' and returns an instance of SomeTemplate - * initialized with 'arg'. Assumes a constructor accepting T (by value, - * reference or whatever). - */ -template class CLASS_TEMPLATE, typename ARG1> -CLASS_TEMPLATE llmake(const ARG1& arg1) -{ - return CLASS_TEMPLATE(arg1); -} - -template class CLASS_TEMPLATE, typename ARG1, typename ARG2> -CLASS_TEMPLATE llmake(const ARG1& arg1, const ARG2& arg2) -{ - return CLASS_TEMPLATE(arg1, arg2); -} #endif /* ! defined(LL_LLMAKE_H) */ From e29b9ce3b26d554bfc34fd4631d60f7c61f9f8ac Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 14 Oct 2019 15:49:37 -0400 Subject: [PATCH 05/38] DRTVWR-476: Add llsd::array() and llsd::map() variadic functions. 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. --- indra/llcommon/llsdutil.h | 70 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/indra/llcommon/llsdutil.h b/indra/llcommon/llsdutil.h index 01ab6bcb8d..8e7ce2fd60 100644 --- a/indra/llcommon/llsdutil.h +++ b/indra/llcommon/llsdutil.h @@ -129,6 +129,16 @@ LL_COMMON_API std::string llsd_matches(const LLSD& prototype, const LLSD& data, /// equality rather than bitwise equality, pass @a bits as for /// is_approx_equal_fraction(). LL_COMMON_API bool llsd_equals(const LLSD& lhs, const LLSD& rhs, int bits=-1); +/// If you don't care about LLSD::Real equality +inline bool operator==(const LLSD& lhs, const LLSD& rhs) +{ + return llsd_equals(lhs, rhs); +} +inline bool operator!=(const LLSD& lhs, const LLSD& rhs) +{ + // operator!=() should always be the negation of operator==() + return ! (lhs == rhs); +} // Simple function to copy data out of input & output iterators if // there is no need for casting. @@ -211,6 +221,36 @@ private: LLSD _data; }; +namespace llsd +{ + +/** + * Construct an LLSD::Array inline, using modern C++ variadic arguments. + */ + +// recursion tail +inline +void array_(LLSD&) {} + +// recursive call +template +void array_(LLSD& data, T0&& v0, Ts&&... vs) +{ + data.append(std::forward(v0)); + array_(data, std::forward(vs)...); +} + +// public interface +template +LLSD array(Ts&&... vs) +{ + LLSD data; + array_(data, std::forward(vs)...); + return data; +} + +} // namespace llsd + /***************************************************************************** * LLSDMap *****************************************************************************/ @@ -255,6 +295,36 @@ private: LLSD _data; }; +namespace llsd +{ + +/** + * Construct an LLSD::Map inline, using modern C++ variadic arguments. + */ + +// recursion tail +inline +void map_(LLSD&) {} + +// recursive call +template +void map_(LLSD& data, const LLSD::String& k0, T0&& v0, Ts&&... vs) +{ + data[k0] = v0; + map_(data, std::forward(vs)...); +} + +// public interface +template +LLSD map(Ts&&... vs) +{ + LLSD data; + map_(data, std::forward(vs)...); + return data; +} + +} // namespace llsd + /***************************************************************************** * LLSDParam *****************************************************************************/ From bdb87556ff07f916e8ce3c108d40b1af2e53ec7b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 15 Oct 2019 14:30:19 -0400 Subject: [PATCH 06/38] DRTVWR-476: Update to viewer-manager build 531762 --- autobuild.xml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/autobuild.xml b/autobuild.xml index f820ec3db3..2d294a61a8 100644 --- a/autobuild.xml +++ b/autobuild.xml @@ -3150,9 +3150,9 @@ Copyright (c) 2012, 2014, 2015, 2016 nghttp2 contributors archive hash - 46ce1788585c2d0eac48491531cd3c8a + 32fa3efa3529f91fd483c88bc6d64efd url - http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/44691/394879/viewer_manager-2.0.531568-darwin64-531568.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/44990/397778/viewer_manager-2.0.531762-darwin64-531762.tar.bz2 name darwin64 @@ -3174,9 +3174,9 @@ Copyright (c) 2012, 2014, 2015, 2016 nghttp2 contributors archive hash - b82b5d39fd36b4b4ecb8dca6a18a9d39 + 49de4f0490b3a3d7b078af0efc8d0ae8 url - http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/44695/394889/viewer_manager-2.0.531568-windows-531568.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/44991/397785/viewer_manager-2.0.531762-windows-531762.tar.bz2 name windows @@ -3187,7 +3187,7 @@ Copyright (c) 2012, 2014, 2015, 2016 nghttp2 contributors source_type hg version - 2.0.531568 + 2.0.531762 vlc-bin From 401023ec7aa84a4b45935f7900918e151c4f2ac4 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 15 Oct 2019 16:16:55 -0400 Subject: [PATCH 07/38] DRTVWR-476: Remove llwrap(), LLListenerWrapper[Base] and support. The only usage of any of this was in test code. --- indra/llcommon/CMakeLists.txt | 2 - indra/llcommon/llevents.h | 70 +--------- indra/llcommon/lllistenerwrapper.h | 198 ----------------------------- indra/test/llevents_tut.cpp | 30 +---- 4 files changed, 2 insertions(+), 298 deletions(-) delete mode 100644 indra/llcommon/lllistenerwrapper.h diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 8df5afd0e9..4c9bfe4443 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -1,4 +1,3 @@ - # -*- cmake -*- project(llcommon) @@ -185,7 +184,6 @@ set(llcommon_HEADER_FILES llkeythrottle.h llleap.h llleaplistener.h - lllistenerwrapper.h llliveappconfig.h lllivefile.h llmd5.h diff --git a/indra/llcommon/llevents.h b/indra/llcommon/llevents.h index 18525a8fa5..253592256d 100644 --- a/indra/llcommon/llevents.h +++ b/indra/llcommon/llevents.h @@ -814,62 +814,6 @@ private: LL_COMMON_API bool sendReply(const LLSD& reply, const LLSD& request, const std::string& replyKey="reply"); -/** - * Base class for LLListenerWrapper. See visit_and_connect() and llwrap(). We - * provide virtual @c accept_xxx() methods, customization points allowing a - * subclass access to certain data visible at LLEventPump::listen() time. - * Example subclass usage: - * - * @code - * myEventPump.listen("somename", - * llwrap(boost::bind(&MyClass::method, instance, _1))); - * @endcode - * - * Because of the anticipated usage (note the anonymous temporary - * MyListenerWrapper instance in the example above), the @c accept_xxx() - * methods must be @c const. - */ -class LL_COMMON_API LLListenerWrapperBase -{ -public: - /// New instance. The accept_xxx() machinery makes it important to use - /// shared_ptrs for our data. Many copies of this object are made before - /// the instance that actually ends up in the signal, yet accept_xxx() - /// will later be called on the @em original instance. All copies of the - /// same original instance must share the same data. - LLListenerWrapperBase(): - mName(new std::string), - mConnection(new LLBoundListener) - { - } - - /// Copy constructor. Copy shared_ptrs to original instance data. - LLListenerWrapperBase(const LLListenerWrapperBase& that): - mName(that.mName), - mConnection(that.mConnection) - { - } - virtual ~LLListenerWrapperBase() {} - - /// Ask LLEventPump::listen() for the listener name - virtual void accept_name(const std::string& name) const - { - *mName = name; - } - - /// Ask LLEventPump::listen() for the new connection - virtual void accept_connection(const LLBoundListener& connection) const - { - *mConnection = connection; - } - -protected: - /// Listener name. - boost::shared_ptr mName; - /// Connection. - boost::shared_ptr mConnection; -}; - /***************************************************************************** * Underpinnings *****************************************************************************/ @@ -1121,19 +1065,7 @@ namespace LLEventDetail // Boost.Signals, in case we were passed a boost::ref(). visit_each(visitor, LLEventDetail::unwrap(raw_listener)); // Make the connection using passed function. - LLBoundListener connection(connect_func(listener)); - // If the LISTENER is an LLListenerWrapperBase subclass, pass it the - // desired information. It's important that we pass the raw_listener - // so the compiler can make decisions based on its original type. - const LLListenerWrapperBase* lwb = - ll_template_cast(&raw_listener); - if (lwb) - { - lwb->accept_name(name); - lwb->accept_connection(connection); - } - // In any case, show new connection to caller. - return connection; + return connect_func(listener); } } // namespace LLEventDetail diff --git a/indra/llcommon/lllistenerwrapper.h b/indra/llcommon/lllistenerwrapper.h deleted file mode 100644 index 09d074abca..0000000000 --- a/indra/llcommon/lllistenerwrapper.h +++ /dev/null @@ -1,198 +0,0 @@ -/** - * @file lllistenerwrapper.h - * @author Nat Goodspeed - * @date 2009-11-30 - * @brief Introduce LLListenerWrapper template - * - * $LicenseInfo:firstyear=2009&license=viewerlgpl$ - * Second Life Viewer Source Code - * Copyright (C) 2010, Linden Research, Inc. - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; - * version 2.1 of the License only. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA - * - * Linden Research, Inc., 945 Battery Street, San Francisco, CA 94111 USA - * $/LicenseInfo$ - */ - -#if ! defined(LL_LLLISTENERWRAPPER_H) -#define LL_LLLISTENERWRAPPER_H - -#include "llevents.h" // LLListenerWrapperBase -#include - -/** - * Template base class for coding wrappers for LLEventPump listeners. - * - * Derive your listener wrapper from LLListenerWrapper. You must use - * LLLISTENER_WRAPPER_SUBCLASS() so your subclass will play nicely with - * boost::visit_each (q.v.). That way boost::signals2 can still detect - * derivation from LLEventTrackable, and so forth. - */ -template -class LLListenerWrapper: public LLListenerWrapperBase -{ -public: - /// Wrap an arbitrary listener object - LLListenerWrapper(const LISTENER& listener): - mListener(listener) - {} - - /// call - virtual bool operator()(const LLSD& event) - { - return mListener(event); - } - - /// Allow boost::visit_each() to peek at our mListener. - template - void accept_visitor(V& visitor) const - { - using boost::visit_each; - visit_each(visitor, mListener, 0); - } - -private: - LISTENER mListener; -}; - -/** - * Specialize boost::visit_each() (leveraging ADL) to peek inside an - * LLListenerWrapper to traverse its LISTENER. We borrow the - * accept_visitor() pattern from boost::bind(), avoiding the need to make - * mListener public. - */ -template -void visit_each(V& visitor, const LLListenerWrapper& wrapper, int) -{ - wrapper.accept_visitor(visitor); -} - -/// use this (sigh!) for each subclass of LLListenerWrapper you write -#define LLLISTENER_WRAPPER_SUBCLASS(CLASS) \ -template \ -void visit_each(V& visitor, const CLASS& wrapper, int) \ -{ \ - visit_each(visitor, static_cast&>(wrapper), 0); \ -} \ - \ -/* Have to state this explicitly, rather than using LL_TEMPLATE_CONVERTIBLE, */ \ -/* because the source type is itself a template. */ \ -template \ -struct ll_template_cast_impl*> \ -{ \ - const LLListenerWrapperBase* operator()(const CLASS* wrapper) \ - { \ - return wrapper; \ - } \ -} - -/** - * Make an instance of a listener wrapper. Every wrapper class must be a - * template accepting a listener object of arbitrary type. In particular, the - * type of a boost::bind() expression is deliberately undocumented. So we - * can't just write Wrapper(boost::bind(...)). Instead we must - * write llwrap(boost::bind(...)). - */ -template class WRAPPER, typename T> -WRAPPER llwrap(const T& listener) -{ - return WRAPPER(listener); -} - -/** - * This LLListenerWrapper template subclass is used to report entry/exit to an - * event listener, by changing this: - * @code - * someEventPump.listen("MyClass", - * boost::bind(&MyClass::method, ptr, _1)); - * @endcode - * to this: - * @code - * someEventPump.listen("MyClass", - * llwrap( - * boost::bind(&MyClass::method, ptr, _1))); - * @endcode - */ -template -class LLCoutListener: public LLListenerWrapper -{ - typedef LLListenerWrapper super; - -public: - /// Wrap an arbitrary listener object - LLCoutListener(const LISTENER& listener): - super(listener) - {} - - /// call - virtual bool operator()(const LLSD& event) - { - std::cout << "Entering listener " << *super::mName << " with " << event << std::endl; - bool handled = super::operator()(event); - std::cout << "Leaving listener " << *super::mName; - if (handled) - { - std::cout << " (handled)"; - } - std::cout << std::endl; - return handled; - } -}; - -LLLISTENER_WRAPPER_SUBCLASS(LLCoutListener); - -/** - * This LLListenerWrapper template subclass is used to log entry/exit to an - * event listener, by changing this: - * @code - * someEventPump.listen("MyClass", - * boost::bind(&MyClass::method, ptr, _1)); - * @endcode - * to this: - * @code - * someEventPump.listen("MyClass", - * llwrap( - * boost::bind(&MyClass::method, ptr, _1))); - * @endcode - */ -template -class LLLogListener: public LLListenerWrapper -{ - typedef LLListenerWrapper super; - -public: - /// Wrap an arbitrary listener object - LLLogListener(const LISTENER& listener): - super(listener) - {} - - /// call - virtual bool operator()(const LLSD& event) - { - LL_DEBUGS("LLLogListener") << "Entering listener " << *super::mName << " with " << event << LL_ENDL; - bool handled = super::operator()(event); - LL_DEBUGS("LLLogListener") << "Leaving listener " << *super::mName; - if (handled) - { - LL_CONT << " (handled)"; - } - LL_CONT << LL_ENDL; - return handled; - } -}; - -LLLISTENER_WRAPPER_SUBCLASS(LLLogListener); - -#endif /* ! defined(LL_LLLISTENERWRAPPER_H) */ diff --git a/indra/test/llevents_tut.cpp b/indra/test/llevents_tut.cpp index 16edab6282..1d7d246e71 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 @@ -643,33 +642,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 { @@ -680,7 +652,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 From 94a50320fb1d3c8af72beb4e58ff17d99c7c9eb1 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 15 Oct 2019 23:06:04 -0400 Subject: [PATCH 08/38] 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/llcommon/llevents.h | 343 +---------------------------------- indra/llui/llnotifications.h | 34 +--- indra/test/llevents_tut.cpp | 228 +++++------------------ 3 files changed, 57 insertions(+), 548 deletions(-) diff --git a/indra/llcommon/llevents.h b/indra/llcommon/llevents.h index 253592256d..3c388bf176 100644 --- a/indra/llcommon/llevents.h +++ b/indra/llcommon/llevents.h @@ -309,37 +309,6 @@ testable: PumpNames mQueueNames; }; -/***************************************************************************** -* details -*****************************************************************************/ -namespace LLEventDetail -{ - /// Any callable capable of connecting an LLEventListener to an - /// LLStandardSignal to produce an LLBoundListener can be mapped to this - /// signature. - typedef boost::function ConnectFunc; - - /// overload of visit_and_connect() when we have a string identifier available - template - LLBoundListener visit_and_connect(const std::string& name, - const LISTENER& listener, - const ConnectFunc& connect_func); - /** - * Utility template function to use Visitor appropriately - * - * @param listener Callable to connect, typically a boost::bind() - * expression. This will be visited by Visitor using boost::visit_each(). - * @param connect_func Callable that will connect() @a listener to an - * LLStandardSignal, returning LLBoundListener. - */ - template - LLBoundListener visit_and_connect(const LISTENER& listener, - const ConnectFunc& connect_func) - { - return visit_and_connect("", listener, connect_func); - } -} // namespace LLEventDetail - /***************************************************************************** * LLEventTrackable *****************************************************************************/ @@ -374,11 +343,6 @@ namespace LLEventDetail * instance, it attempts to dereference the Foo* pointer that was * deleted but not zeroed.) * - Undefined behavior results. - * If you suspect you may encounter any such scenario, you're better off - * managing the lifespan of your object with boost::shared_ptr. - * Passing LLEventPump::listen() a boost::bind() expression - * involving a boost::weak_ptr is recognized specially, engaging - * thread-safe Boost.Signals2 machinery. */ typedef boost::signals2::trackable LLEventTrackable; @@ -517,44 +481,13 @@ public: * the result be assigned to a LLTempBoundListener or the listener is * manually disconnected when no longer needed since there will be no * way to later find and disconnect this listener manually. - * - * If (as is typical) you pass a boost::bind() expression as @a - * listener, listen() will inspect the components of that expression. If a - * bound object matches any of several cases, the connection will - * automatically be disconnected when that object is destroyed. - * - * * You bind a boost::weak_ptr. - * * Binding a boost::shared_ptr that way would ensure that the - * referenced object would @em never be destroyed, since the @c - * shared_ptr stored in the LLEventPump would remain an outstanding - * reference. Use the weaken() function to convert your @c shared_ptr to - * @c weak_ptr. Because this is easy to forget, binding a @c shared_ptr - * will produce a compile error (@c BOOST_STATIC_ASSERT failure). - * * You bind a simple pointer or reference to an object derived from - * boost::enable_shared_from_this. (UNDER CONSTRUCTION) - * * You bind a simple pointer or reference to an object derived from - * LLEventTrackable. Unlike the cases described above, though, this is - * vulnerable to a couple of cross-thread race conditions, as described - * in the LLEventTrackable documentation. */ - template - LLBoundListener listen(const std::string& name, const LISTENER& listener, + LLBoundListener listen(const std::string& name, + const LLEventListener& listener, const NameList& after=NameList(), const NameList& before=NameList()) { - // Examine listener, using our listen_impl() method to make the - // actual connection. - // This is why listen() is a template. Conversion from boost::bind() - // to LLEventListener performs type erasure, so it's important to look - // at the boost::bind object itself before that happens. - return LLEventDetail::visit_and_connect(name, - listener, - boost::bind(&LLEventPump::listen_invoke, - this, - name, - _1, - after, - before)); + return listen_impl(name, listener, after, before); } /// Get the LLBoundListener associated with the passed name (dummy @@ -598,13 +531,6 @@ private: private: - LLBoundListener listen_invoke(const std::string& name, const LLEventListener& listener, - const NameList& after, - const NameList& before) - { - return this->listen_impl(name, listener, after, before); - } - // must precede mName; see LLEventPump::LLEventPump() LLHandle mRegistry; @@ -814,261 +740,6 @@ private: LL_COMMON_API bool sendReply(const LLSD& reply, const LLSD& request, const std::string& replyKey="reply"); -/***************************************************************************** -* Underpinnings -*****************************************************************************/ -/** - * We originally provided a suite of overloaded - * LLEventTrackable::listenTo(LLEventPump&, ...) methods that would call - * LLEventPump::listen(...) and then pass the returned LLBoundListener to - * LLEventTrackable::track(). This was workable but error-prone: the coder - * must remember to call listenTo() rather than the more straightforward - * listen() method. - * - * Now we publish only the single canonical listen() method, so there's a - * uniform mechanism. Having a single way to do this is good, in that there's - * no question in the coder's mind which of several alternatives to choose. - * - * To support automatic connection management, we use boost::visit_each - * (http://www.boost.org/doc/libs/1_37_0/doc/html/boost/visit_each.html) to - * inspect each argument of a boost::bind expression. (Although the visit_each - * mechanism was first introduced with the original Boost.Signals library, it - * was only later documented.) - * - * Cases: - * * At least one of the function's arguments is a boost::weak_ptr. Pass - * the corresponding shared_ptr to slot_type::track(). Ideally that would be - * the object whose method we want to call, but in fact we do the same for - * any weak_ptr we might find among the bound arguments. If we're passing - * our bound method a weak_ptr to some object, wouldn't the destruction of - * that object invalidate the call? So we disconnect automatically when any - * such object is destroyed. This is the mechanism preferred by boost:: - * signals2. - * * One of the functions's arguments is a boost::shared_ptr. This produces - * a compile error: the bound copy of the shared_ptr stored in the - * boost_bind object stored in the signal object would make the referenced - * T object immortal. We provide a weaken() function. Pass - * weaken(your_shared_ptr) instead. (We can inspect, but not modify, the - * boost::bind object. Otherwise we'd replace the shared_ptr with weak_ptr - * implicitly and just proceed.) - * * One of the function's arguments is a plain pointer/reference to an object - * derived from boost::enable_shared_from_this. We assume that this object - * is managed using boost::shared_ptr, so we implicitly extract a shared_ptr - * and track that. (UNDER CONSTRUCTION) - * * One of the function's arguments is derived from LLEventTrackable. Pass - * the LLBoundListener to its LLEventTrackable::track(). This is vulnerable - * to a couple different race conditions, as described in LLEventTrackable - * documentation. (NOTE: Now that LLEventTrackable is a typedef for - * boost::signals2::trackable, the Signals2 library handles this itself, so - * our visitor needs no special logic for this case.) - * * Any other argument type is irrelevant to automatic connection management. - */ - -namespace LLEventDetail -{ - template - const F& unwrap(const F& f) { return f; } - - template - const F& unwrap(const boost::reference_wrapper& f) { return f.get(); } - - // Most of the following is lifted from the Boost.Signals use of - // visit_each. - template struct truth {}; - - /** - * boost::visit_each() Visitor, used on a template argument const F& - * f as follows (see visit_and_connect()): - * @code - * LLEventListener listener(f); - * Visitor visitor(listener); // bind listener so it can track() shared_ptrs - * using boost::visit_each; // allow unqualified visit_each() call for ADL - * visit_each(visitor, unwrap(f)); - * @endcode - */ - class Visitor - { - public: - /** - * Visitor binds a reference to LLEventListener so we can track() any - * shared_ptrs we find in the argument list. - */ - Visitor(LLEventListener& listener): - mListener(listener) - { - } - - /** - * boost::visit_each() calls this method for each component of a - * boost::bind() expression. - */ - template - void operator()(const T& t) const - { - decode(t, 0); - } - - private: - // decode() decides between a reference wrapper and anything else - // boost::ref() variant - template - void decode(const boost::reference_wrapper& t, int) const - { -// add_if_trackable(t.get_pointer()); - } - - // decode() anything else - template - void decode(const T& t, long) const - { - typedef truth<(boost::is_pointer::value)> is_a_pointer; - maybe_get_pointer(t, is_a_pointer()); - } - - // maybe_get_pointer() decides between a pointer and a non-pointer - // plain pointer variant - template - void maybe_get_pointer(const T& t, truth) const - { -// add_if_trackable(t); - } - - // shared_ptr variant - template - void maybe_get_pointer(const boost::shared_ptr& t, truth) const - { - // If we have a shared_ptr to this object, it doesn't matter - // whether the object is derived from LLEventTrackable, so no - // further analysis of T is needed. -// mListener.track(t); - - // Make this case illegal. Passing a bound shared_ptr to - // slot_type::track() is useless, since the bound shared_ptr will - // keep the object alive anyway! Force the coder to cast to weak_ptr. - - // Trivial as it is, make the BOOST_STATIC_ASSERT() condition - // dependent on template param so the macro is only evaluated if - // this method is in fact instantiated, as described here: - // http://www.boost.org/doc/libs/1_34_1/doc/html/boost_staticassert.html - - // ATTENTION: Don't bind a shared_ptr using - // LLEventPump::listen(boost::bind()). Doing so captures a copy of - // the shared_ptr, making the referenced object effectively - // immortal. Use the weaken() function, e.g.: - // somepump.listen(boost::bind(...weaken(my_shared_ptr)...)); - // This lets us automatically disconnect when the referenced - // object is destroyed. - BOOST_STATIC_ASSERT(sizeof(T) == 0); - } - - // weak_ptr variant - template - void maybe_get_pointer(const boost::weak_ptr& t, truth) const - { - // If we have a weak_ptr to this object, it doesn't matter - // whether the object is derived from LLEventTrackable, so no - // further analysis of T is needed. - mListener.track(t); -// std::cout << "Found weak_ptr<" << typeid(T).name() << ">!\n"; - } - -#if 0 - // reference to anything derived from boost::enable_shared_from_this - template - inline void maybe_get_pointer(const boost::enable_shared_from_this& ct, - truth) const - { - // Use the slot_type::track(shared_ptr) mechanism. Cast away - // const-ness because (in our code base anyway) it's unusual - // to find shared_ptr. - boost::enable_shared_from_this& - t(const_cast&>(ct)); - std::cout << "Capturing shared_from_this()" << std::endl; - boost::shared_ptr sp(t.shared_from_this()); -/*==========================================================================*| - std::cout << "Capturing weak_ptr" << std::endl; - boost::weak_ptr wp(sp); -|*==========================================================================*/ - std::cout << "Tracking shared__ptr" << std::endl; - mListener.track(sp); - } -#endif - - // non-pointer variant - template - void maybe_get_pointer(const T& t, truth) const - { - // Take the address of this object, because the object itself may be - // trackable -// add_if_trackable(boost::addressof(t)); - } - -/*==========================================================================*| - // add_if_trackable() adds LLEventTrackable objects to mTrackables - inline void add_if_trackable(const LLEventTrackable* t) const - { - if (t) - { - } - } - - // pointer to anything not an LLEventTrackable subclass - inline void add_if_trackable(const void*) const - { - } - - // pointer to free function - // The following construct uses the preprocessor to generate - // add_if_trackable() overloads accepting pointer-to-function taking - // 0, 1, ..., LLEVENTS_LISTENER_ARITY parameters of arbitrary type. -#define BOOST_PP_LOCAL_MACRO(n) \ - template \ - inline void \ - add_if_trackable(R (*)(BOOST_PP_ENUM_PARAMS(n, T))) const \ - { \ - } -#define BOOST_PP_LOCAL_LIMITS (0, LLEVENTS_LISTENER_ARITY) -#include BOOST_PP_LOCAL_ITERATE() -#undef BOOST_PP_LOCAL_MACRO -#undef BOOST_PP_LOCAL_LIMITS -|*==========================================================================*/ - - /// Bind a reference to the LLEventListener to call its track() method. - LLEventListener& mListener; - }; - - /** - * Utility template function to use Visitor appropriately - * - * @param raw_listener Callable to connect, typically a boost::bind() - * expression. This will be visited by Visitor using boost::visit_each(). - * @param connect_funct Callable that will connect() @a raw_listener to an - * LLStandardSignal, returning LLBoundListener. - */ - template - LLBoundListener visit_and_connect(const std::string& name, - const LISTENER& raw_listener, - const ConnectFunc& connect_func) - { - // Capture the listener - LLEventListener listener(raw_listener); - // Define our Visitor, binding the listener so we can call - // listener.track() if we discover any shared_ptr. - LLEventDetail::Visitor visitor(listener); - // Allow unqualified visit_each() call for ADL - using boost::visit_each; - // Visit each component of a boost::bind() expression. Pass - // 'raw_listener', our template argument, rather than 'listener' from - // which type details have been erased. unwrap() comes from - // Boost.Signals, in case we were passed a boost::ref(). - visit_each(visitor, LLEventDetail::unwrap(raw_listener)); - // Make the connection using passed function. - return connect_func(listener); - } -} // namespace LLEventDetail - // Somewhat to my surprise, passing boost::bind(...boost::weak_ptr...) to // listen() fails in Boost code trying to instantiate LLEventListener (i.e. // LLStandardSignal::slot_type) because the boost::get_pointer() utility function isn't @@ -1079,12 +750,4 @@ namespace boost T* get_pointer(const weak_ptr& ptr) { return shared_ptr(ptr).get(); } } -/// Since we forbid use of listen(boost::bind(...shared_ptr...)), provide an -/// easy way to cast to the corresponding weak_ptr. -template -boost::weak_ptr weaken(const boost::shared_ptr& ptr) -{ - return boost::weak_ptr(ptr); -} - #endif /* ! defined(LL_LLEVENTS_H) */ diff --git a/indra/llui/llnotifications.h b/indra/llui/llnotifications.h index 1509446920..e3cb4ba74e 100644 --- a/indra/llui/llnotifications.h +++ b/indra/llui/llnotifications.h @@ -744,42 +744,24 @@ public: virtual ~LLNotificationChannelBase() {} // you can also connect to a Channel, so you can be notified of // changes to this channel - template - LLBoundListener connectChanged(const LISTENER& slot) + LLBoundListener connectChanged(const LLEventListener& slot) { - // Examine slot to see if it binds an LLEventTrackable subclass, or a - // boost::shared_ptr to something, or a boost::weak_ptr to something. // Call this->connectChangedImpl() to actually connect it. - return LLEventDetail::visit_and_connect(slot, - boost::bind(&LLNotificationChannelBase::connectChangedImpl, - this, - _1)); + return connectChangedImpl(slot); } - template - LLBoundListener connectAtFrontChanged(const LISTENER& slot) + LLBoundListener connectAtFrontChanged(const LLEventListener& slot) { - return LLEventDetail::visit_and_connect(slot, - boost::bind(&LLNotificationChannelBase::connectAtFrontChangedImpl, - this, - _1)); + return connectAtFrontChangedImpl(slot); } - template - LLBoundListener connectPassedFilter(const LISTENER& slot) + LLBoundListener connectPassedFilter(const LLEventListener& slot) { // see comments in connectChanged() - return LLEventDetail::visit_and_connect(slot, - boost::bind(&LLNotificationChannelBase::connectPassedFilterImpl, - this, - _1)); + return connectPassedFilterImpl(slot); } - template - LLBoundListener connectFailedFilter(const LISTENER& slot) + LLBoundListener connectFailedFilter(const LLEventListener& slot) { // see comments in connectChanged() - return LLEventDetail::visit_and_connect(slot, - boost::bind(&LLNotificationChannelBase::connectFailedFilterImpl, - this, - _1)); + return connectFailedFilterImpl(slot); } // use this when items change or to add a new one diff --git a/indra/test/llevents_tut.cpp b/indra/test/llevents_tut.cpp index 1d7d246e71..3dd802b84d 100644 --- a/indra/test/llevents_tut.cpp +++ b/indra/test/llevents_tut.cpp @@ -502,196 +502,60 @@ void events_object::test<10>() heaptest.stopListening("temp"); } +class TempTrackableListener: public TempListener, public LLEventTrackable +{ +public: + TempTrackableListener(const std::string& name, bool& liveFlag): + TempListener(name, liveFlag) + {} +}; + 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); + 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<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); -|*==========================================================================*/ + 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 TempTrackableListener: public TempListener, public LLEventTrackable -{ -public: -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; -{ - 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; -{ - 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 From d57186d84fc459ad6919dcffdf297ab4a22760db Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 16 Oct 2019 08:52:51 -0400 Subject: [PATCH 09/38] DRTVWR-476: Validate LLEventPumpOrPumpName replyPump 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. --- indra/llcommon/lleventcoro.cpp | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/indra/llcommon/lleventcoro.cpp b/indra/llcommon/lleventcoro.cpp index 7558e54e88..aa0a8b0cc4 100644 --- a/indra/llcommon/lleventcoro.cpp +++ b/indra/llcommon/lleventcoro.cpp @@ -165,8 +165,8 @@ LLBoundListener postAndSuspendSetup(const std::string& callerName, const std::string& listenerName, LLCoros::Promise& promise, const LLSD& event, - const LLEventPumpOrPumpName& requestPump, - const LLEventPumpOrPumpName& replyPump, + const LLEventPumpOrPumpName& requestPumpP, + const LLEventPumpOrPumpName& replyPumpP, const LLSD& replyPumpNamePath) { // Get the consuming attribute for THIS coroutine, the one that's about to @@ -174,10 +174,12 @@ LLBoundListener postAndSuspendSetup(const std::string& callerName, // return the consuming attribute for some other coroutine, most likely // the main routine. bool consuming(LLCoros::get_consuming()); - // make a callback that will assign a value to the future, and listen on - // the specified LLEventPump with that callback + // listen on the specified LLEventPump with a lambda that will assign a + // value to the promise, thus fulfilling its future + llassert_always_msg(replyPumpP, ("replyPump required for " + callerName)); + LLEventPump& replyPump{replyPumpP.getPump()}; LLBoundListener connection( - replyPump.getPump().listen( + replyPump.listen( listenerName, [&promise, consuming, listenerName](const LLSD& result) { @@ -191,30 +193,31 @@ LLBoundListener postAndSuspendSetup(const std::string& callerName, } catch(boost::fibers::promise_already_satisfied & ex) { - LL_WARNS("lleventcoro") << "promise already satisfied in '" - << listenerName << "' " << ex.what() << LL_ENDL; + LL_DEBUGS("lleventcoro") << "promise already satisfied in '" + << listenerName << "': " << ex.what() << LL_ENDL; // We could not propagate the result value to the // listener. return false; } })); // skip the "post" part if requestPump is default-constructed - if (requestPump) + if (requestPumpP) { + LLEventPump& requestPump{requestPumpP.getPump()}; // If replyPumpNamePath is non-empty, store the replyPump name in the // request event. LLSD modevent(event); - storeToLLSDPath(modevent, replyPumpNamePath, replyPump.getPump().getName()); + storeToLLSDPath(modevent, replyPumpNamePath, replyPump.getName()); LL_DEBUGS("lleventcoro") << callerName << ": coroutine " << listenerName - << " posting to " << requestPump.getPump().getName() + << " posting to " << requestPump.getName() << LL_ENDL; // *NOTE:Mani - Removed because modevent could contain user's hashed passwd. // << ": " << modevent << LL_ENDL; - requestPump.getPump().post(modevent); + requestPump.post(modevent); } LL_DEBUGS("lleventcoro") << callerName << ": coroutine " << listenerName - << " about to wait on LLEventPump " << replyPump.getPump().getName() + << " about to wait on LLEventPump " << replyPump.getName() << LL_ENDL; return connection; } From 97f834df2206d4251150a7e7889097684f223167 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 16 Oct 2019 09:15:47 -0400 Subject: [PATCH 10/38] DRTVWR-476: Add LLEventLogProxy, LLEventLogProxyFor. 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 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, both "plain" (events only) and via lleventcoro.h synchronization. --- indra/llcommon/lleventfilter.cpp | 59 +++++++++++++ indra/llcommon/lleventfilter.h | 95 +++++++++++++++++++++ indra/llcommon/tests/lleventcoro_test.cpp | 78 +++++++++++++++++ indra/llcommon/tests/lleventfilter_test.cpp | 75 ++++++++++++++++ 4 files changed, 307 insertions(+) diff --git a/indra/llcommon/lleventfilter.cpp b/indra/llcommon/lleventfilter.cpp index 9fb18dc67d..06b3cb769e 100644 --- a/indra/llcommon/lleventfilter.cpp +++ b/indra/llcommon/lleventfilter.cpp @@ -37,6 +37,7 @@ // other Linden headers #include "llerror.h" // LL_ERRS #include "llsdutil.h" // llsd_matches() +#include "stringize.h" /***************************************************************************** * LLEventFilter @@ -409,3 +410,61 @@ void LLEventBatchThrottle::setSize(std::size_t size) flush(); } } + +/***************************************************************************** +* LLEventLogProxy +*****************************************************************************/ +LLEventLogProxy::LLEventLogProxy(LLEventPump& source, const std::string& name, bool tweak): + // note: we are NOT using the constructor that implicitly connects! + LLEventFilter(name, tweak), + // instead we simply capture a reference to the subject LLEventPump + mPump(source) +{ +} + +bool LLEventLogProxy::post(const LLSD& event) /* override */ +{ + auto counter = mCounter++; + auto eventplus = event; + if (eventplus.type() == LLSD::TypeMap) + { + eventplus["_cnt"] = counter; + } + std::string hdr{STRINGIZE(getName() << ": post " << counter)}; + LL_INFOS("LogProxy") << hdr << ": " << event << LL_ENDL; + bool result = mPump.post(eventplus); + LL_INFOS("LogProxy") << hdr << " => " << result << LL_ENDL; + return result; +} + +LLBoundListener LLEventLogProxy::listen_impl(const std::string& name, + const LLEventListener& target, + const NameList& after, + const NameList& before) +{ + LL_DEBUGS("LogProxy") << "LLEventLogProxy('" << getName() << "').listen('" + << name << "')" << LL_ENDL; + return mPump.listen(name, + [this, name, target](const LLSD& event)->bool + { return listener(name, target, event); }, + after, + before); +} + +bool LLEventLogProxy::listener(const std::string& name, + const LLEventListener& target, + const LLSD& event) const +{ + auto eventminus = event; + std::string counter{"**"}; + if (eventminus.has("_cnt")) + { + counter = stringize(eventminus["_cnt"].asInteger()); + eventminus.erase("_cnt"); + } + std::string hdr{STRINGIZE(getName() << " to " << name << " " << counter)}; + LL_INFOS("LogProxy") << hdr << ": " << eventminus << LL_ENDL; + bool result = target(eventminus); + LL_INFOS("LogProxy") << hdr << " => " << result << LL_ENDL; + return result; +} diff --git a/indra/llcommon/lleventfilter.h b/indra/llcommon/lleventfilter.h index ff8fc9bc7f..cbc0b18901 100644 --- a/indra/llcommon/lleventfilter.h +++ b/indra/llcommon/lleventfilter.h @@ -376,4 +376,99 @@ private: std::size_t mBatchSize; }; +/***************************************************************************** +* LLEventLogProxy +*****************************************************************************/ +/** + * LLEventLogProxy is a little different than the other LLEventFilter + * subclasses declared in this header file, in that it completely wraps the + * passed LLEventPump (both input and output) instead of simply processing its + * output. Of course, if someone directly posts to the wrapped LLEventPump by + * looking up its string name in LLEventPumps, LLEventLogProxy can't intercept + * that post() call. But as long as consuming code is willing to access the + * LLEventLogProxy instance instead of the wrapped LLEventPump, all event data + * both post()ed and received is logged. + * + * The proxy role means that LLEventLogProxy intercepts more of LLEventPump's + * API than a typical LLEventFilter subclass. + */ +class LLEventLogProxy: public LLEventFilter +{ + typedef LLEventFilter super; +public: + /** + * Construct LLEventLogProxy, wrapping the specified LLEventPump. + * Unlike a typical LLEventFilter subclass, the name parameter is @emph + * not optional because typically you want LLEventLogProxy to completely + * replace the wrapped LLEventPump. So you give the subject LLEventPump + * some other name and give the LLEventLogProxy the name that would have + * been used for the subject LLEventPump. + */ + LLEventLogProxy(LLEventPump& source, const std::string& name, bool tweak=false); + + /// register a new listener + LLBoundListener listen_impl(const std::string& name, const LLEventListener& target, + const NameList& after, const NameList& before); + + /// Post an event to all listeners + virtual bool post(const LLSD& event) /* override */; + +private: + /// This method intercepts each call to any target listener. We pass it + /// the listener name and the caller's intended target listener plus the + /// posted LLSD event. + bool listener(const std::string& name, + const LLEventListener& target, + const LLSD& event) const; + + LLEventPump& mPump; + LLSD::Integer mCounter{0}; +}; + +/** + * LLEventPumpHolder is a helper for LLEventLogProxyFor. It simply + * stores an instance of T, presumably a subclass of LLEventPump. We derive + * LLEventLogProxyFor from LLEventPumpHolder, ensuring that + * LLEventPumpHolder's contained mWrappedPump is fully constructed before + * passing it to LLEventLogProxyFor's LLEventLogProxy base class constructor. + * But since LLEventPumpHolder presents none of the LLEventPump API, + * LLEventLogProxyFor inherits its methods unambiguously from + * LLEventLogProxy. + */ +template +class LLEventPumpHolder +{ +protected: + LLEventPumpHolder(const std::string& name, bool tweak=false): + mWrappedPump(name, tweak) + {} + T mWrappedPump; +}; + +/** + * LLEventLogProxyFor is a wrapper around any of the LLEventPump subclasses. + * Instantiating an LLEventLogProxy instantiates an internal T. Otherwise + * it behaves like LLEventLogProxy. + */ +template +class LLEventLogProxyFor: private LLEventPumpHolder, public LLEventLogProxy +{ + // We derive privately from LLEventPumpHolder because it's an + // implementation detail of LLEventLogProxyFor. The only reason it's a + // base class at all is to guarantee that it's constructed first so we can + // pass it to our LLEventLogProxy base class constructor. + typedef LLEventPumpHolder holder; + typedef LLEventLogProxy super; + +public: + LLEventLogProxyFor(const std::string& name, bool tweak=false): + // our wrapped LLEventPump subclass instance gets a name suffix + // because that's not the LLEventPump we want consumers to obtain when + // they ask LLEventPumps for this name + holder(name + "-", tweak), + // it's our LLEventLogProxy that gets the passed name + super(holder::mWrappedPump, name, tweak) + {} +}; + #endif /* ! defined(LL_LLEVENTFILTER_H) */ diff --git a/indra/llcommon/tests/lleventcoro_test.cpp b/indra/llcommon/tests/lleventcoro_test.cpp index 4e774b27d9..c13920eefd 100644 --- a/indra/llcommon/tests/lleventcoro_test.cpp +++ b/indra/llcommon/tests/lleventcoro_test.cpp @@ -37,12 +37,14 @@ #include #include +#include #include "../test/lltut.h" #include "llsd.h" #include "llsdutil.h" #include "llevents.h" #include "llcoros.h" +#include "lleventfilter.h" #include "lleventcoro.h" #include "../test/debug.h" #include "../test/sync.h" @@ -255,4 +257,80 @@ namespace tut LLCoros::instance().launch("test<5>", [this](){ coroPumpPost(); }); ensure_equals(result.asInteger(), 18); } + + template + void test() + { + PUMP pump(typeid(PUMP).name()); + bool running{false}; + LLSD data{LLSD::emptyArray()}; + // start things off by posting once before even starting the listener + // coro + LL_DEBUGS() << "test() posting first" << LL_ENDL; + LLSD first{LLSDMap("desc", "first")("value", 0)}; + bool consumed = pump.post(first); + ensure("should not have consumed first", ! consumed); + // now launch the coro + LL_DEBUGS() << "test() launching listener coro" << LL_ENDL; + running = true; + LLCoros::instance().launch( + "listener", + [&pump, &running, &data](){ + // important for this test that we consume posted values + LLCoros::instance().set_consuming(true); + // should immediately retrieve 'first' without waiting + LL_DEBUGS() << "listener coro waiting for first" << LL_ENDL; + data.append(llcoro::suspendUntilEventOnWithTimeout(pump, 0.1, LLSD())); + // Don't use ensure() from within the coro -- ensure() failure + // throws tut::fail, which won't propagate out to the main + // test driver, which will result in an odd failure. + // Wait for 'second' because it's not already pending. + LL_DEBUGS() << "listener coro waiting for second" << LL_ENDL; + data.append(llcoro::suspendUntilEventOnWithTimeout(pump, 0.1, LLSD())); + // and wait for 'third', which should involve no further waiting + LL_DEBUGS() << "listener coro waiting for third" << LL_ENDL; + data.append(llcoro::suspendUntilEventOnWithTimeout(pump, 0.1, LLSD())); + LL_DEBUGS() << "listener coro done" << LL_ENDL; + running = false; + }); + // back from coro at the point where it's waiting for 'second' + LL_DEBUGS() << "test() posting second" << LL_ENDL; + LLSD second{llsd::map("desc", "second", "value", 1)}; + consumed = pump.post(second); + ensure("should have consumed second", consumed); + // This is a key point: even though we've post()ed the value for which + // the coroutine is waiting, it's actually still suspended until we + // pause for some other reason. The coroutine will only pick up one + // value at a time from our 'pump'. It's important to exercise the + // case when we post() two values before it picks up either. + LL_DEBUGS() << "test() posting third" << LL_ENDL; + LLSD third{llsd::map("desc", "third", "value", 2)}; + consumed = pump.post(third); + ensure("should NOT yet have consumed third", ! consumed); + // now just wait for coro to finish -- which it eventually will, given + // that all its suspend calls have short timeouts. + while (running) + { + LL_DEBUGS() << "test() waiting for coro done" << LL_ENDL; + llcoro::suspendUntilTimeout(0.1); + } + // okay, verify expected results + ensure_equals("should have received three values", data, + llsd::array(first, second, third)); + LL_DEBUGS() << "test() done" << LL_ENDL; + } + + template<> template<> + void object::test<6>() + { + set_test_name("LLEventMailDrop"); + tut::test(); + } + + template<> template<> + void object::test<7>() + { + set_test_name("LLEventLogProxyFor"); + tut::test< LLEventLogProxyFor >(); + } } diff --git a/indra/llcommon/tests/lleventfilter_test.cpp b/indra/llcommon/tests/lleventfilter_test.cpp index eb98b12ef5..683bdcc167 100644 --- a/indra/llcommon/tests/lleventfilter_test.cpp +++ b/indra/llcommon/tests/lleventfilter_test.cpp @@ -36,9 +36,12 @@ // other Linden headers #include "../test/lltut.h" #include "stringize.h" +#include "llsdutil.h" #include "listener.h" #include "tests/wrapllerrs.h" +#include + /***************************************************************************** * Test classes *****************************************************************************/ @@ -407,6 +410,78 @@ namespace tut throttle.post(";17"); ensure_equals("17", cat.result, "136;12;17"); // "17" delivered } + + template + void test() + { + PUMP pump(typeid(PUMP).name()); + LLSD data{LLSD::emptyArray()}; + bool consumed{true}; + // listener that appends to 'data' + // but that also returns the current value of 'consumed' + // Instantiate this separately because we're going to listen() + // multiple times with the same lambda: LLEventMailDrop only replays + // queued events on a new listen() call. + auto lambda = + [&data, &consumed](const LLSD& event)->bool + { + data.append(event); + return consumed; + }; + { + LLTempBoundListener conn = pump.listen("lambda", lambda); + pump.post("first"); + } + // first post() should certainly be received by listener + ensure_equals("first", data, llsd::array("first")); + // the question is, since consumed was true, did it queue the value? + data = LLSD::emptyArray(); + { + // if it queued the value, it would be delivered on subsequent + // listen() call + LLTempBoundListener conn = pump.listen("lambda", lambda); + } + ensure_equals("empty1", data, LLSD::emptyArray()); + data = LLSD::emptyArray(); + // now let's NOT consume the posted data + consumed = false; + { + LLTempBoundListener conn = pump.listen("lambda", lambda); + pump.post("second"); + pump.post("third"); + } + // the two events still arrive + ensure_equals("second,third1", data, llsd::array("second", "third")); + data = LLSD::emptyArray(); + { + // when we reconnect, these should be delivered again + // but this time they should be consumed + consumed = true; + LLTempBoundListener conn = pump.listen("lambda", lambda); + } + // unconsumed events were delivered again + ensure_equals("second,third2", data, llsd::array("second", "third")); + data = LLSD::emptyArray(); + { + // when we reconnect this time, no more unconsumed events + LLTempBoundListener conn = pump.listen("lambda", lambda); + } + ensure_equals("empty2", data, LLSD::emptyArray()); + } + + template<> template<> + void filter_object::test<6>() + { + set_test_name("LLEventMailDrop"); + tut::test(); + } + + template<> template<> + void filter_object::test<7>() + { + set_test_name("LLEventLogProxyFor"); + tut::test< LLEventLogProxyFor >(); + } } // namespace tut /***************************************************************************** From 7ccd902b892cc644980aa4fdba226bcd765b59fe Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 16 Oct 2019 11:19:29 -0400 Subject: [PATCH 11/38] DRTVWR-476: Directly reference LLVivoxVoiceClient::mVivoxPump. The LLEventMailDrop used to communicate with the Vivox coroutine is a member of LLVivoxVoiceClient. We don't need to keep looking it up by its string name in LLEventPumps. --- indra/newview/llvoicevivox.cpp | 80 ++++++++++++++-------------------- 1 file changed, 33 insertions(+), 47 deletions(-) diff --git a/indra/newview/llvoicevivox.cpp b/indra/newview/llvoicevivox.cpp index e10ba77e16..6c51b09a5a 100644 --- a/indra/newview/llvoicevivox.cpp +++ b/indra/newview/llvoicevivox.cpp @@ -1024,8 +1024,6 @@ bool LLVivoxVoiceClient::provisionVoiceAccount() bool LLVivoxVoiceClient::establishVoiceConnection() { - LLEventPump &voiceConnectPump = LLEventPumps::instance().obtain("vivoxClientPump"); - if (!mVoiceEnabled && mIsInitialized) { LL_WARNS("Voice") << "cannot establish connection; enabled "<mHandle)) ("session", "joined")); - LLEventPumps::instance().post("vivoxClientPump", vivoxevent); + mVivoxPump.post(vivoxevent); // Add the current user as a participant here. participantStatePtr_t participant(session->addParticipant(sipURIFromName(mAccountName))); @@ -3630,7 +3616,7 @@ void LLVivoxVoiceClient::leftAudioSession(const sessionStatePtr_t &session) LLSD vivoxevent(LLSDMap("handle", LLSD::String(session->mHandle)) ("session", "removed")); - LLEventPumps::instance().post("vivoxClientPump", vivoxevent); + mVivoxPump.post(vivoxevent); } } @@ -3658,7 +3644,7 @@ void LLVivoxVoiceClient::accountLoginStateChangeEvent( case 1: levent["login"] = LLSD::String("account_login"); - LLEventPumps::instance().post("vivoxClientPump", levent); + mVivoxPump.post(levent); break; case 2: break; @@ -3666,7 +3652,7 @@ void LLVivoxVoiceClient::accountLoginStateChangeEvent( case 3: levent["login"] = LLSD::String("account_loggingOut"); - LLEventPumps::instance().post("vivoxClientPump", levent); + mVivoxPump.post(levent); break; case 4: @@ -3679,7 +3665,7 @@ void LLVivoxVoiceClient::accountLoginStateChangeEvent( case 0: levent["login"] = LLSD::String("account_logout"); - LLEventPumps::instance().post("vivoxClientPump", levent); + mVivoxPump.post(levent); break; default: @@ -3714,7 +3700,7 @@ void LLVivoxVoiceClient::mediaCompletionEvent(std::string &sessionGroupHandle, s } if (!result.isUndefined()) - LLEventPumps::instance().post("vivoxClientPump", result); + mVivoxPump.post(result); } void LLVivoxVoiceClient::mediaStreamUpdatedEvent( @@ -6526,7 +6512,7 @@ void LLVivoxVoiceClient::accountGetSessionFontsResponse(int statusCode, const st // receiving the last one. LLSD result(LLSDMap("voice_fonts", LLSD::Boolean(true))); - LLEventPumps::instance().post("vivoxClientPump", result); + mVivoxPump.post(result); } notifyVoiceFontObservers(); mVoiceFontsReceived = true; @@ -6677,7 +6663,7 @@ void LLVivoxVoiceClient::enablePreviewBuffer(bool enable) else result["recplay"] = "quit"; - LLEventPumps::instance().post("vivoxClientPump", result); + mVivoxPump.post(result); if(mCaptureBufferMode && mIsInChannel) { @@ -6698,7 +6684,7 @@ void LLVivoxVoiceClient::recordPreviewBuffer() mCaptureBufferRecording = true; LLSD result(LLSDMap("recplay", "record")); - LLEventPumps::instance().post("vivoxClientPump", result); + mVivoxPump.post(result); } void LLVivoxVoiceClient::playPreviewBuffer(const LLUUID& effect_id) @@ -6721,7 +6707,7 @@ void LLVivoxVoiceClient::playPreviewBuffer(const LLUUID& effect_id) mCaptureBufferPlaying = true; LLSD result(LLSDMap("recplay", "playback")); - LLEventPumps::instance().post("vivoxClientPump", result); + mVivoxPump.post(result); } void LLVivoxVoiceClient::stopPreviewBuffer() @@ -6730,7 +6716,7 @@ void LLVivoxVoiceClient::stopPreviewBuffer() mCaptureBufferPlaying = false; LLSD result(LLSDMap("recplay", "quit")); - LLEventPumps::instance().post("vivoxClientPump", result); + mVivoxPump.post(result); } bool LLVivoxVoiceClient::isPreviewRecording() From 7acc7a5b3ca28967737ea65fbf5564a08953c58e Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 17 Oct 2019 12:04:02 -0400 Subject: [PATCH 12/38] DRTVWR-476: Introduce LLEventMailDrop::discard() (instead of flush()). 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. --- indra/llcommon/llevents.cpp | 9 +++++++-- indra/llcommon/llevents.h | 3 ++- indra/newview/llvoicevivox.cpp | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/indra/llcommon/llevents.cpp b/indra/llcommon/llevents.cpp index 99abb333bb..186e710c43 100644 --- a/indra/llcommon/llevents.cpp +++ b/indra/llcommon/llevents.cpp @@ -602,6 +602,11 @@ LLBoundListener LLEventMailDrop::listen_impl(const std::string& name, return LLEventStream::listen_impl(name, listener, after, before); } +void LLEventMailDrop::discard() +{ + mEventHistory.clear(); + LLEventStream::flush(); +} /***************************************************************************** * LLEventQueue @@ -621,8 +626,8 @@ bool LLEventQueue::post(const LLSD& event) void LLEventQueue::flush() { - if(!mSignal) return; - + if(!mSignal) return; + // Consider the case when a given listener on this LLEventQueue posts yet // another event on the same queue. If we loop over mEventQueue directly, // we'll end up processing all those events during the same flush() call diff --git a/indra/llcommon/llevents.h b/indra/llcommon/llevents.h index 3c388bf176..ce2aa2f3c9 100644 --- a/indra/llcommon/llevents.h +++ b/indra/llcommon/llevents.h @@ -610,7 +610,8 @@ public: virtual bool post(const LLSD& event) override; /// Remove any history stored in the mail drop. - virtual void flush() override { mEventHistory.clear(); LLEventStream::flush(); }; + void discard(); + protected: virtual LLBoundListener listen_impl(const std::string& name, const LLEventListener&, const NameList& after, diff --git a/indra/newview/llvoicevivox.cpp b/indra/newview/llvoicevivox.cpp index 6c51b09a5a..2ec7d256e3 100644 --- a/indra/newview/llvoicevivox.cpp +++ b/indra/newview/llvoicevivox.cpp @@ -1474,7 +1474,7 @@ bool LLVivoxVoiceClient::addAndJoinSession(const sessionStatePtr_t &nextSession) // We are about to start a whole new session. Anything that MIGHT still be in our // maildrop is going to be stale and cause us much wailing and gnashing of teeth. // Just flush it all out and start new. - mVivoxPump.flush(); + mVivoxPump.discard(); // It appears that I need to wait for BOTH the SessionGroup.AddSession response and the SessionStateChangeEvent with state 4 // before continuing from this state. They can happen in either order, and if I don't wait for both, things can get stuck. From 5da30ce4a492c913bc4e187a0cc60ebe4b3f503b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 17 Oct 2019 13:26:51 -0400 Subject: [PATCH 13/38] 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/llcommon/llevents.cpp | 104 ++++-------------------------- indra/llcommon/llevents.h | 39 ++--------- indra/llcommon/llleaplistener.cpp | 21 +++++- indra/test/llevents_tut.cpp | 57 +++------------- 4 files changed, 47 insertions(+), 174 deletions(-) diff --git a/indra/llcommon/llevents.cpp b/indra/llcommon/llevents.cpp index 186e710c43..d31f5f2d32 100644 --- a/indra/llcommon/llevents.cpp +++ b/indra/llcommon/llevents.cpp @@ -63,52 +63,17 @@ #pragma warning (disable : 4702) #endif -/***************************************************************************** -* queue_names: specify LLEventPump names that should be instantiated as -* LLEventQueue -*****************************************************************************/ -/** - * At present, we recognize particular requested LLEventPump names as needing - * LLEventQueues. Later on we'll migrate this information to an external - * configuration file. - */ -const char* queue_names[] = -{ - "placeholder - replace with first real name string" -}; - -/***************************************************************************** -* If there's a "mainloop" pump, listen on that to flush all LLEventQueues -*****************************************************************************/ -struct RegisterFlush : public LLEventTrackable -{ - RegisterFlush(): - pumps(LLEventPumps::instance()) - { - pumps.obtain("mainloop").listen("flushLLEventQueues", boost::bind(&RegisterFlush::flush, this, _1)); - } - bool flush(const LLSD&) - { - pumps.flush(); - return false; - } - ~RegisterFlush() - { - // LLEventTrackable handles stopListening for us. - } - LLEventPumps& pumps; -}; -static RegisterFlush registerFlush; - /***************************************************************************** * LLEventPumps *****************************************************************************/ -LLEventPumps::LLEventPumps(): - // Until we migrate this information to an external config file, - // initialize mQueueNames from the static queue_names array. - mQueueNames(boost::begin(queue_names), boost::end(queue_names)) +LLEventPumps::PumpFactories LLEventPumps::mFactories { -} + // LLEventStream is the default for obtain(), so even if somebody DOES + // call obtain("placeholder"), this sample entry won't break anything. + { "placeholder", [](const std::string& name) { return new LLEventStream(name); } } +}; + +LLEventPumps::LLEventPumps() {} LLEventPump& LLEventPumps::obtain(const std::string& name) { @@ -121,10 +86,10 @@ LLEventPump& LLEventPumps::obtain(const std::string& name) } // Here we must instantiate an LLEventPump subclass. LLEventPump* newInstance; - // Should this name be an LLEventQueue? - PumpNames::const_iterator nfound = mQueueNames.find(name); - if (nfound != mQueueNames.end()) - newInstance = new LLEventQueue(name); + // Do we have a predefined factory for this instance name? + PumpFactories::const_iterator nfound = mFactories.find(name); + if (nfound != mFactories.end()) + newInstance = (nfound->second)(name); else newInstance = new LLEventStream(name); // LLEventPump's constructor implicitly registers each new instance in @@ -144,14 +109,13 @@ bool LLEventPumps::post(const std::string&name, const LLSD&message) return (*found).second->post(message); } - void LLEventPumps::flush() { // Flush every known LLEventPump instance. Leave it up to each instance to // decide what to do with the flush() call. - for (PumpMap::iterator pmi = mPumpMap.begin(), pmend = mPumpMap.end(); pmi != pmend; ++pmi) + for (PumpMap::value_type& pair : mPumpMap) { - pmi->second->flush(); + pair.second->flush(); } } @@ -605,48 +569,6 @@ LLBoundListener LLEventMailDrop::listen_impl(const std::string& name, void LLEventMailDrop::discard() { mEventHistory.clear(); - LLEventStream::flush(); -} - -/***************************************************************************** -* LLEventQueue -*****************************************************************************/ -bool LLEventQueue::post(const LLSD& event) -{ - if (mEnabled) - { - // Defer sending this event by queueing it until flush() - mEventQueue.push_back(event); - } - // Unconditionally return false. We won't know until flush() whether a - // listener claims to have handled the event -- meanwhile, don't block - // other listeners. - return false; -} - -void LLEventQueue::flush() -{ - if(!mSignal) return; - - // Consider the case when a given listener on this LLEventQueue posts yet - // another event on the same queue. If we loop over mEventQueue directly, - // we'll end up processing all those events during the same flush() call - // -- rather like an EventStream. Instead, copy mEventQueue and clear it, - // so that any new events posted to this LLEventQueue during flush() will - // be processed in the *next* flush() call. - EventQueue queue(mEventQueue); - mEventQueue.clear(); - // NOTE NOTE NOTE: Any new access to member data beyond this point should - // cause us to move our LLStandardSignal object to a pimpl class along - // with said member data. Then the local shared_ptr will preserve both. - - // DEV-43463: capture a local copy of mSignal. See LLEventStream::post() - // for detailed comments. - boost::shared_ptr signal(mSignal); - for ( ; ! queue.empty(); queue.pop_front()) - { - (*signal)(queue.front()); - } } /***************************************************************************** diff --git a/indra/llcommon/llevents.h b/indra/llcommon/llevents.h index ce2aa2f3c9..c55351919e 100644 --- a/indra/llcommon/llevents.h +++ b/indra/llcommon/llevents.h @@ -37,6 +37,7 @@ #include #include #include +#include #if LL_WINDOWS #pragma warning (push) #pragma warning (disable : 4263) // boost::signals2::expired_slot::what() has const mismatch @@ -55,7 +56,6 @@ #include #include // reference_wrapper #include -#include #include #include "llsd.h" #include "llsingleton.h" @@ -303,10 +303,10 @@ testable: // destroyed. typedef std::set PumpSet; PumpSet mOurPumps; - // LLEventPump names that should be instantiated as LLEventQueue rather - // than as LLEventStream - typedef std::set PumpNames; - PumpNames mQueueNames; + // LLEventPump subclasses that should be instantiated for particular + // instance names + typedef std::map> PumpFactories; + static PumpFactories mFactories; }; /***************************************************************************** @@ -351,7 +351,7 @@ typedef boost::signals2::trackable LLEventTrackable; *****************************************************************************/ /** * LLEventPump is the base class interface through which we access the - * concrete subclasses LLEventStream and LLEventQueue. + * concrete subclasses such as LLEventStream. * * @NOTE * LLEventPump derives from LLEventTrackable so that when you "chain" @@ -594,11 +594,10 @@ public: * event *must* eventually reach a listener that will consume it, else the * queue will grow to arbitrary length. * - * @NOTE: When using an LLEventMailDrop (or LLEventQueue) with a LLEventTimeout or + * @NOTE: When using an LLEventMailDrop with an LLEventTimeout or * LLEventFilter attaching the filter downstream, using Timeout's constructor will * cause the MailDrop to discharge any of its stored events. The timeout should * instead be connected upstream using its listen() method. - * See llcoro::suspendUntilEventOnWithTimeout() for an example. */ class LL_COMMON_API LLEventMailDrop : public LLEventStream { @@ -622,30 +621,6 @@ private: EventList mEventHistory; }; -/***************************************************************************** -* LLEventQueue -*****************************************************************************/ -/** - * LLEventQueue is a LLEventPump whose post() method defers calling registered - * listeners until flush() is called. - */ -class LL_COMMON_API LLEventQueue: public LLEventPump -{ -public: - LLEventQueue(const std::string& name, bool tweak=false): LLEventPump(name, tweak) {} - virtual ~LLEventQueue() {} - - /// Post an event to all listeners - virtual bool post(const LLSD& event); - - /// flush queued events - virtual void flush(); - -private: - typedef std::deque EventQueue; - EventQueue mEventQueue; -}; - /***************************************************************************** * LLReqID *****************************************************************************/ diff --git a/indra/llcommon/llleaplistener.cpp b/indra/llcommon/llleaplistener.cpp index fa5730f112..6b4340a416 100644 --- a/indra/llcommon/llleaplistener.cpp +++ b/indra/llcommon/llleaplistener.cpp @@ -14,6 +14,8 @@ // associated header #include "llleaplistener.h" // STL headers +#include +#include // std headers // external library headers #include @@ -119,6 +121,18 @@ LLLeapListener::~LLLeapListener() } } +namespace +{ + +static std::map> factories +{ + // tweak name for uniqueness + { "LLEventStream", [](const std::string& name){ return new LLEventStream(name, true); } }, + { "LLEventMailDrop", [](const std::string& name){ return new LLEventMailDrop(name, true); } } +}; + +} // anonymous namespace + void LLLeapListener::newpump(const LLSD& request) { Response reply(LLSD(), request); @@ -127,13 +141,14 @@ void LLLeapListener::newpump(const LLSD& request) LLSD const & type = request["type"]; LLEventPump * new_pump = NULL; - if (type.asString() == "LLEventQueue") + auto found = factories.find(type.asString()); + if (found != factories.end()) { - new_pump = new LLEventQueue(name, true); // tweak name for uniqueness + new_pump = (found->second)(name); } else { - if (! (type.isUndefined() || type.asString() == "LLEventStream")) + if (! type.isUndefined()) { reply.warn(STRINGIZE("unknown 'type' " << type << ", using LLEventStream")); } diff --git a/indra/test/llevents_tut.cpp b/indra/test/llevents_tut.cpp index 3dd802b84d..f15fc1d984 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); @@ -211,43 +209,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 @@ -272,7 +233,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")); @@ -286,7 +247,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")); @@ -311,7 +272,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; @@ -397,7 +358,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 @@ -431,7 +392,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 @@ -474,7 +435,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: @@ -511,7 +472,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; @@ -534,7 +495,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; From a47653478d5cb504136df0031d16d9bd54da79e9 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 17 Oct 2019 13:28:33 -0400 Subject: [PATCH 14/38] DRTVWR-476: We've observed a couple different values for VS 2017. --- indra/cmake/Copy3rdPartyLibs.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/indra/cmake/Copy3rdPartyLibs.cmake b/indra/cmake/Copy3rdPartyLibs.cmake index d929608fab..8460d703d1 100644 --- a/indra/cmake/Copy3rdPartyLibs.cmake +++ b/indra/cmake/Copy3rdPartyLibs.cmake @@ -82,7 +82,7 @@ if(WINDOWS) elseif (MSVC_VERSION EQUAL 1800) # VisualStudio 2013, which is (sigh) VS 12 list(APPEND LMSVC_VER 120) list(APPEND LMSVC_VERDOT 12.0) - elseif (MSVC_VERSION EQUAL 1916) # Visual Studio 2017 + elseif (MSVC_VERSION EQUAL 1915 OR MSVC_VERSION EQUAL 1916) # Visual Studio 2017 list(APPEND LMSVC_VER 150) list(APPEND LMSVC_VERDOT 15.0) else (MSVC80) From 64686c49297d235d40bb178b261d977e9008920d Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 18 Oct 2019 11:54:05 -0400 Subject: [PATCH 15/38] DRTVWR-476: Seems gcc 4.8 forbids brace-init of reference vars. Although this is legal, apparently there was a bug in the C++11 standard (to which gcc 4.8 conforms) that was subsequently fixed in the standard (and thus in gcc 4.9). Thanks Henri Beauchamp. --- indra/llcommon/lleventcoro.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/indra/llcommon/lleventcoro.cpp b/indra/llcommon/lleventcoro.cpp index aa0a8b0cc4..bcc35955cc 100644 --- a/indra/llcommon/lleventcoro.cpp +++ b/indra/llcommon/lleventcoro.cpp @@ -177,7 +177,7 @@ LLBoundListener postAndSuspendSetup(const std::string& callerName, // listen on the specified LLEventPump with a lambda that will assign a // value to the promise, thus fulfilling its future llassert_always_msg(replyPumpP, ("replyPump required for " + callerName)); - LLEventPump& replyPump{replyPumpP.getPump()}; + LLEventPump& replyPump(replyPumpP.getPump()); LLBoundListener connection( replyPump.listen( listenerName, @@ -203,7 +203,7 @@ LLBoundListener postAndSuspendSetup(const std::string& callerName, // skip the "post" part if requestPump is default-constructed if (requestPumpP) { - LLEventPump& requestPump{requestPumpP.getPump()}; + LLEventPump& requestPump(requestPumpP.getPump()); // If replyPumpNamePath is non-empty, store the replyPump name in the // request event. LLSD modevent(event); From 5d123b41abdeb112d1ae2b5e85cc25b6f1b2f99d Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 18 Oct 2019 14:46:05 -0400 Subject: [PATCH 16/38] DRTVWR-476: Eliminate static LLEventPump factory maps. Having a map from std::string to a factory function returning LLEventPump* is a cool idea, especially since you could statically populate such a map with string literals and little lambdas. Unfortunately, static initialization of any data is a bad idea when control can reach consuming code before that module's static data are constructed. Since LLEventPumps is already an LLSingleton, it's simple enough to make its map non-static and initialize it in the constructor. But another recent static factory-function map was introduced in llleaplistener.cpp to support the LLLeapListener::newpump() operation. That involves no LLSingletons. Introduce LLEventPumps::make(name, tweak, type) to instantiate an LLEventPump subclass of the specified type with specified (name, tweak) parameters. Instances returned by make() are owned by LLEventPumps, as with obtain(). Introduce LLEventPumps::BadType exception for when the type string isn't recognized. LLEventPumps::obtain() can then simply call make() when the specified instance name doesn't already exist. The configuration data used internally by obtain() becomes { string instance name, string subclass name }. Although this too is currently initialized in the LLEventPumps constructor, migrating it to a configuration file would now be far more straightforward than before. LLLeapListener::newpump(), too, can call LLEventPumps::make() with the caller-specified type string. This eliminates that static factory map. newpump() must catch BadType and report the error back to its invoker. Given that the LLEventPump subclass instances returned by make() are owned by LLEventPumps rather than LLLeapListener, there is no further need for the LLLeapListener::mEventPumps ptr_map, which was used only to manage lifetime. Also remove LLLeapListener's "killpump" operation since LLEventPumps provides no corresponding functionality. --- indra/llcommon/llevents.cpp | 56 ++++++++++++++++++++-------- indra/llcommon/llevents.h | 55 +++++++++++++++++++++------- indra/llcommon/llleaplistener.cpp | 61 +++++++------------------------ indra/llcommon/llleaplistener.h | 5 --- 4 files changed, 95 insertions(+), 82 deletions(-) diff --git a/indra/llcommon/llevents.cpp b/indra/llcommon/llevents.cpp index d31f5f2d32..281a1121bd 100644 --- a/indra/llcommon/llevents.cpp +++ b/indra/llcommon/llevents.cpp @@ -66,14 +66,21 @@ /***************************************************************************** * LLEventPumps *****************************************************************************/ -LLEventPumps::PumpFactories LLEventPumps::mFactories -{ - // LLEventStream is the default for obtain(), so even if somebody DOES - // call obtain("placeholder"), this sample entry won't break anything. - { "placeholder", [](const std::string& name) { return new LLEventStream(name); } } -}; - -LLEventPumps::LLEventPumps() {} +LLEventPumps::LLEventPumps(): + mFactories + { + { "LLEventStream", [](const std::string& name, bool tweak) + { return new LLEventStream(name, tweak); } }, + { "LLEventMailDrop", [](const std::string& name, bool tweak) + { return new LLEventMailDrop(name, tweak); } } + }, + mTypes + { + // LLEventStream is the default for obtain(), so even if somebody DOES + // call obtain("placeholder"), this sample entry won't break anything. + { "placeholder", "LLEventStream" } + } +{} LLEventPump& LLEventPumps::obtain(const std::string& name) { @@ -84,14 +91,31 @@ LLEventPump& LLEventPumps::obtain(const std::string& name) // name. return *found->second; } - // Here we must instantiate an LLEventPump subclass. - LLEventPump* newInstance; - // Do we have a predefined factory for this instance name? - PumpFactories::const_iterator nfound = mFactories.find(name); - if (nfound != mFactories.end()) - newInstance = (nfound->second)(name); - else - newInstance = new LLEventStream(name); + + // Here we must instantiate an LLEventPump subclass. Is there a + // preregistered class name override for this specific instance name? + auto nfound = mTypes.find(name); + std::string type; + if (nfound != mTypes.end()) + { + type = nfound->second; + } + // pass tweak=false: we already know there's no existing instance with + // this name + return make(name, false, type); +} + +LLEventPump& LLEventPumps::make(const std::string& name, bool tweak, + const std::string& type) +{ + // find the relevant factory for this (or default) type + auto found = mFactories.find(type.empty()? "LLEventStream" : type); + if (found == mFactories.end()) + { + // Passing an unrecognized type name is a no-no + LLTHROW(BadType(type)); + } + auto newInstance = (found->second)(name, tweak); // LLEventPump's constructor implicitly registers each new instance in // mPumpMap. But remember that we instantiated it (in mOurPumps) so we'll // delete it later. diff --git a/indra/llcommon/llevents.h b/indra/llcommon/llevents.h index c55351919e..e380c108f4 100644 --- a/indra/llcommon/llevents.h +++ b/indra/llcommon/llevents.h @@ -211,8 +211,7 @@ public: /// exception if you try to call when empty struct Empty: public LLException { - Empty(const std::string& what): - LLException(std::string("LLListenerOrPumpName::Empty: ") + what) {} + Empty(const std::string& what): LLException("LLListenerOrPumpName::Empty: " + what) {} }; private: @@ -247,6 +246,30 @@ public: */ LLEventPump& obtain(const std::string& name); + /// exception potentially thrown by make() + struct BadType: public LLException + { + BadType(const std::string& what): LLException("BadType: " + what) {} + }; + + /** + * Create an LLEventPump with suggested name (optionally of specified + * LLEventPump subclass type). As with obtain(), LLEventPumps owns the new + * instance. + * + * As with LLEventPump's constructor, make() could throw + * LLEventPump::DupPumpName unless you pass tweak=true. + * + * As with a hand-constructed LLEventPump subclass, if you pass + * tweak=true, the tweaked name can be obtained by LLEventPump::getName(). + * + * Pass empty type to get the default LLEventStream. + * + * If you pass an unrecognized type string, make() throws BadType. + */ + LLEventPump& make(const std::string& name, bool tweak=false, + const std::string& type=std::string()); + /** * Find the named LLEventPump instance. If it exists post the message to it. * If the pump does not exist, do nothing. @@ -303,10 +326,19 @@ testable: // destroyed. typedef std::set PumpSet; PumpSet mOurPumps; - // LLEventPump subclasses that should be instantiated for particular - // instance names - typedef std::map> PumpFactories; - static PumpFactories mFactories; + // for make(), map string type name to LLEventPump subclass factory function + typedef std::map> PumpFactories; + // Data used by make(). + // One might think mFactories and mTypes could reasonably be static. So + // they could -- if not for the fact that make() or obtain() might be + // called before this module's static variables have been initialized. + // This is why we use singletons in the first place. + PumpFactories mFactories; + + // for obtain(), map desired string instance name to string type when + // obtain() must create the instance + typedef std::map InstanceTypes; + InstanceTypes mTypes; }; /***************************************************************************** @@ -372,8 +404,7 @@ public: */ struct DupPumpName: public LLException { - DupPumpName(const std::string& what): - LLException(std::string("DupPumpName: ") + what) {} + DupPumpName(const std::string& what): LLException("DupPumpName: " + what) {} }; /** @@ -409,9 +440,7 @@ public: */ struct DupListenerName: public ListenError { - DupListenerName(const std::string& what): - ListenError(std::string("DupListenerName: ") + what) - {} + DupListenerName(const std::string& what): ListenError("DupListenerName: " + what) {} }; /** * exception thrown by listen(). The order dependencies specified for your @@ -423,7 +452,7 @@ public: */ struct Cycle: public ListenError { - Cycle(const std::string& what): ListenError(std::string("Cycle: ") + what) {} + Cycle(const std::string& what): ListenError("Cycle: " + what) {} }; /** * exception thrown by listen(). This one means that your new listener @@ -444,7 +473,7 @@ public: */ struct OrderChange: public ListenError { - OrderChange(const std::string& what): ListenError(std::string("OrderChange: ") + what) {} + OrderChange(const std::string& what): ListenError("OrderChange: " + what) {} }; /// used by listen() diff --git a/indra/llcommon/llleaplistener.cpp b/indra/llcommon/llleaplistener.cpp index 6b4340a416..ed7d7b47e3 100644 --- a/indra/llcommon/llleaplistener.cpp +++ b/indra/llcommon/llleaplistener.cpp @@ -62,16 +62,11 @@ LLLeapListener::LLLeapListener(const ConnectFunc& connect): LLSD need_name(LLSDMap("name", LLSD())); add("newpump", "Instantiate a new LLEventPump named like [\"name\"] and listen to it.\n" - "If [\"type\"] == \"LLEventQueue\", make LLEventQueue, else LLEventStream.\n" + "[\"type\"] == \"LLEventStream\", \"LLEventMailDrop\" et al.\n" "Events sent through new LLEventPump will be decorated with [\"pump\"]=name.\n" "Returns actual name in [\"name\"] (may be different if collision).", &LLLeapListener::newpump, need_name); - add("killpump", - "Delete LLEventPump [\"name\"] created by \"newpump\".\n" - "Returns [\"status\"] boolean indicating whether such a pump existed.", - &LLLeapListener::killpump, - need_name); LLSD need_source_listener(LLSDMap("source", LLSD())("listener", LLSD())); add("listen", "Listen to an existing LLEventPump named [\"source\"], with listener name\n" @@ -121,58 +116,28 @@ LLLeapListener::~LLLeapListener() } } -namespace -{ - -static std::map> factories -{ - // tweak name for uniqueness - { "LLEventStream", [](const std::string& name){ return new LLEventStream(name, true); } }, - { "LLEventMailDrop", [](const std::string& name){ return new LLEventMailDrop(name, true); } } -}; - -} // anonymous namespace - void LLLeapListener::newpump(const LLSD& request) { Response reply(LLSD(), request); std::string name = request["name"]; - LLSD const & type = request["type"]; + std::string type = request["type"]; - LLEventPump * new_pump = NULL; - auto found = factories.find(type.asString()); - if (found != factories.end()) + try { - new_pump = (found->second)(name); + // tweak name for uniqueness + LLEventPump& new_pump(LLEventPumps::instance().make(name, true, type)); + name = new_pump.getName(); + reply["name"] = name; + + // Now listen on this new pump with our plugin listener + std::string myname("llleap"); + saveListener(name, myname, mConnect(new_pump, myname)); } - else + catch (const LLEventPumps::BadType& error) { - if (! type.isUndefined()) - { - reply.warn(STRINGIZE("unknown 'type' " << type << ", using LLEventStream")); - } - new_pump = new LLEventStream(name, true); // tweak name for uniqueness + reply.error(error.what()); } - - name = new_pump->getName(); - - mEventPumps.insert(name, new_pump); - - // Now listen on this new pump with our plugin listener - std::string myname("llleap"); - saveListener(name, myname, mConnect(*new_pump, myname)); - - reply["name"] = name; -} - -void LLLeapListener::killpump(const LLSD& request) -{ - Response reply(LLSD(), request); - - std::string name = request["name"]; - // success == (nonzero number of entries were erased) - reply["status"] = bool(mEventPumps.erase(name)); } void LLLeapListener::listen(const LLSD& request) diff --git a/indra/llcommon/llleaplistener.h b/indra/llcommon/llleaplistener.h index 2193d81b9e..0ca5893657 100644 --- a/indra/llcommon/llleaplistener.h +++ b/indra/llcommon/llleaplistener.h @@ -40,7 +40,6 @@ public: private: void newpump(const LLSD&); - void killpump(const LLSD&); void listen(const LLSD&); void stoplistening(const LLSD&); void ping(const LLSD&) const; @@ -64,10 +63,6 @@ private: // and listener name. typedef std::map, LLBoundListener> ListenersMap; ListenersMap mListeners; - // Similar lifespan reasoning applies to LLEventPumps instantiated by - // newpump() operations. - typedef boost::ptr_map EventPumpsMap; - EventPumpsMap mEventPumps; }; #endif /* ! defined(LL_LLLEAPLISTENER_H) */ From 2c68e27d4d994c1d3c4e4a0828ab8d8831068e4f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 21 Oct 2019 13:10:58 -0400 Subject: [PATCH 17/38] DRTVWR-476: Add LLEventTimer::run_every(), run_at(), run_after(). Also add corresponding LLEventTimeout::post_every(), post_at(), post_after() methods. --- indra/llcommon/lleventfilter.cpp | 22 ++++++++++ indra/llcommon/lleventfilter.h | 16 ++++++++ indra/llcommon/lleventtimer.h | 69 +++++++++++++++++++++++++++++++- 3 files changed, 106 insertions(+), 1 deletion(-) diff --git a/indra/llcommon/lleventfilter.cpp b/indra/llcommon/lleventfilter.cpp index 06b3cb769e..ae35fb9c8e 100644 --- a/indra/llcommon/lleventfilter.cpp +++ b/indra/llcommon/lleventfilter.cpp @@ -38,6 +38,7 @@ #include "llerror.h" // LL_ERRS #include "llsdutil.h" // llsd_matches() #include "stringize.h" +#include "lldate.h" /***************************************************************************** * LLEventFilter @@ -183,6 +184,27 @@ bool LLEventTimeout::countdownElapsed() const return mTimer.hasExpired(); } +LLEventTimer* LLEventTimeout::post_every(F32 period, const std::string& pump, const LLSD& data) +{ + return LLEventTimer::run_every( + period, + [pump, data](){ LLEventPumps::instance().obtain(pump).post(data); }); +} + +LLEventTimer* LLEventTimeout::post_at(const LLDate& time, const std::string& pump, const LLSD& data) +{ + return LLEventTimer::run_at( + time, + [pump, data](){ LLEventPumps::instance().obtain(pump).post(data); }); +} + +LLEventTimer* LLEventTimeout::post_after(F32 interval, const std::string& pump, const LLSD& data) +{ + return LLEventTimer::run_after( + interval, + [pump, data](){ LLEventPumps::instance().obtain(pump).post(data); }); +} + /***************************************************************************** * LLEventBatch *****************************************************************************/ diff --git a/indra/llcommon/lleventfilter.h b/indra/llcommon/lleventfilter.h index cbc0b18901..2e5d7bc825 100644 --- a/indra/llcommon/lleventfilter.h +++ b/indra/llcommon/lleventfilter.h @@ -32,8 +32,11 @@ #include "llevents.h" #include "stdtypes.h" #include "lltimer.h" +#include "lleventtimer.h" #include +class LLDate; + /** * Generic base class */ @@ -210,6 +213,19 @@ public: LLEventTimeout(); LLEventTimeout(LLEventPump& source); + /// using LLEventTimeout as namespace for free functions + /// Post event to specified LLEventPump every period seconds. Delete + /// returned LLEventTimer* to cancel. + static LLEventTimer* post_every(F32 period, const std::string& pump, const LLSD& data); + /// Post event to specified LLEventPump at specified future time. Call + /// LLEventTimer::getInstance(returned pointer) to check whether it's still + /// pending; if so, delete the pointer to cancel. + static LLEventTimer* post_at(const LLDate& time, const std::string& pump, const LLSD& data); + /// Post event to specified LLEventPump after specified interval. Call + /// LLEventTimer::getInstance(returned pointer) to check whether it's still + /// pending; if so, delete the pointer to cancel. + static LLEventTimer* post_after(F32 interval, const std::string& pump, const LLSD& data); + protected: virtual void setCountdown(F32 seconds); virtual bool countdownElapsed() const; diff --git a/indra/llcommon/lleventtimer.h b/indra/llcommon/lleventtimer.h index dc918121e1..dbbfe0c6e6 100644 --- a/indra/llcommon/lleventtimer.h +++ b/indra/llcommon/lleventtimer.h @@ -40,16 +40,83 @@ public: LLEventTimer(F32 period); // period is the amount of time between each call to tick() in seconds LLEventTimer(const LLDate& time); virtual ~LLEventTimer(); - + //function to be called at the supplied frequency // Normally return FALSE; TRUE will delete the timer after the function returns. virtual BOOL tick() = 0; static void updateClass(); + /// Schedule recurring calls to generic callable every period seconds. + /// Returns a pointer; if you delete it, cancels the recurring calls. + template + static LLEventTimer* run_every(F32 period, const CALLABLE& callable); + + /// Schedule a future call to generic callable. Returns a pointer. + /// CAUTION: The object referenced by that pointer WILL BE DELETED once + /// the callback has been called! LLEventTimer::getInstance(pointer) (NOT + /// pointer->getInstance(pointer)!) can be used to test whether the + /// pointer is still valid. If it is, deleting it will cancel the + /// callback. + template + static LLEventTimer* run_at(const LLDate& time, const CALLABLE& callable); + + /// Like run_at(), but after a time delta rather than at a timestamp. + /// Same CAUTION. + template + static LLEventTimer* run_after(F32 interval, const CALLABLE& callable); + protected: LLTimer mEventTimer; F32 mPeriod; + +private: + template + class Generic; }; +template +class LLEventTimer::Generic: public LLEventTimer +{ +public: + // making TIME generic allows engaging either LLEventTimer constructor + template + Generic(const TIME& time, bool once, const CALLABLE& callable): + LLEventTimer(time), + mOnce(once), + mCallable(callable) + {} + BOOL tick() override + { + mCallable(); + // true tells updateClass() to delete this instance + return mOnce; + } + +private: + bool mOnce; + CALLABLE mCallable; +}; + +template +LLEventTimer* LLEventTimer::run_every(F32 period, const CALLABLE& callable) +{ + // return false to schedule recurring calls + return new Generic(period, false, callable); +} + +template +LLEventTimer* LLEventTimer::run_at(const LLDate& time, const CALLABLE& callable) +{ + // return true for one-shot callback + return new Generic(time, true, callable); +} + +template +LLEventTimer* LLEventTimer::run_after(F32 interval, const CALLABLE& callable) +{ + // one-shot callback after specified interval + return new Generic(interval, true, callable); +} + #endif //LL_EVENTTIMER_H From d5a7ba6ce7c6f969e1c10e3e7ecb910af10bd9c9 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 22 Oct 2019 12:20:21 -0400 Subject: [PATCH 18/38] DRTVWR-476: Update to bugsplat build 532004 --- autobuild.xml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/autobuild.xml b/autobuild.xml index 2d294a61a8..5189560509 100644 --- a/autobuild.xml +++ b/autobuild.xml @@ -244,9 +244,9 @@ archive hash - 40e8f6c5f56f411db75c19b95db29de9 + d453d6200607972493e1797a30fc805f url - http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/44287/391455/bugsplat-1.0.7.531352-darwin64-531352.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/45455/401027/bugsplat-1.0.7.532004-darwin64-532004.tar.bz2 name darwin64 @@ -256,9 +256,9 @@ archive hash - 33a52911164bc6b810b44cdf57be723e + 372e1d677ee570f947183bae81ca0852 url - http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/44289/391466/bugsplat-3.6.0.4.531352-windows-531352.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/45457/401043/bugsplat-3.6.0.4.532004-windows-532004.tar.bz2 name windows @@ -268,16 +268,16 @@ archive hash - 8ed50aebd8df656be512fb68be63dc13 + 261d460a67e3e2f52bdad1391bb02ec3 url - http://s3-proxy.lindenlab.com/private-builds-secondlife-com/ct2/44288/391459/bugsplat-.531352-windows64-531352.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/45456/401036/bugsplat-.532004-windows64-532004.tar.bz2 name windows64 version - .531352 + 1.0.7.532004 colladadom From b610bdef78a8cda24ff55e06e7f3dfd2ab15a281 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 22 Oct 2019 16:16:45 -0400 Subject: [PATCH 19/38] DRTVWR-476: Defer #include "lleventtimer.h" until lleventfilter.cpp. --- indra/llcommon/lleventfilter.cpp | 1 + indra/llcommon/lleventfilter.h | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/indra/llcommon/lleventfilter.cpp b/indra/llcommon/lleventfilter.cpp index ae35fb9c8e..4cded7f88e 100644 --- a/indra/llcommon/lleventfilter.cpp +++ b/indra/llcommon/lleventfilter.cpp @@ -38,6 +38,7 @@ #include "llerror.h" // LL_ERRS #include "llsdutil.h" // llsd_matches() #include "stringize.h" +#include "lleventtimer.h" #include "lldate.h" /***************************************************************************** diff --git a/indra/llcommon/lleventfilter.h b/indra/llcommon/lleventfilter.h index 2e5d7bc825..f8df9dd63e 100644 --- a/indra/llcommon/lleventfilter.h +++ b/indra/llcommon/lleventfilter.h @@ -32,9 +32,9 @@ #include "llevents.h" #include "stdtypes.h" #include "lltimer.h" -#include "lleventtimer.h" #include +class LLEventTimer; class LLDate; /** From eea46a88da4bdc05a871487411a5e4f48fea8fca Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 22 Oct 2019 16:27:59 -0400 Subject: [PATCH 20/38] DRTVWR-476: Make ~LLEventPumps() call reset() on its way out. ~LLEventPumps() deletes every LLEventPump instance it created itself. However, many classes themselves contain LLEventPump subclass instances. These are registered with LLEventPumps without it managing their lifespan. But LLEventPump::reset() frees the LLStandardSignal aka boost::signals2::signal instance owned by the LLEventPump, perforce disconnecting all current listeners and disabling the LLEventPump. Even though the instance still exists, if someone subsequently calls post(), nothing will happen -- which is better than control trying to reach a method of a deleted object. --- indra/llcommon/llevents.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/indra/llcommon/llevents.cpp b/indra/llcommon/llevents.cpp index 281a1121bd..64fb985951 100644 --- a/indra/llcommon/llevents.cpp +++ b/indra/llcommon/llevents.cpp @@ -266,6 +266,9 @@ LLEventPumps::~LLEventPumps() { delete *mOurPumps.begin(); } + // Reset every remaining registered LLEventPump subclass instance: those + // we DIDN'T instantiate using either make() or obtain(). + reset(); } /***************************************************************************** From 20383ecf8eb667b950eb15a925fc61aacf7f6127 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 22 Oct 2019 16:36:16 -0400 Subject: [PATCH 21/38] DRTVWR-476: Mention LLApp::stepFrame() in LLAppViewer::idle() which performs "by hand" the same sequence of calls found in stepFrame(). Why not simply call stepFrame()? Hysterical reasons? --- indra/newview/llappviewer.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index 4decbbd296..f9feba9ce3 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -4641,6 +4641,9 @@ void LLAppViewer::idle() LLFrameTimer::updateFrameTime(); LLFrameTimer::updateFrameCount(); LLEventTimer::updateClass(); + // LLApp::stepFrame() performs the above three calls plus mRunner.run(). + // Not sure why we don't call stepFrame() here, except that LLRunner seems + // completely redundant with LLEventTimer. LLNotificationsUI::LLToast::updateClass(); LLSmoothInterpolation::updateInterpolants(); LLMortician::updateClass(); From 71ec081dbf52448a4655c52e6f5e52a68682621e Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 22 Oct 2019 16:49:29 -0400 Subject: [PATCH 22/38] 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/llcommon/llcoros.cpp | 101 +++++++++++++----- indra/llcommon/llcoros.h | 66 +++++++++++- indra/llcommon/tests/lleventcoro_test.cpp | 2 + indra/test/lltestapp.h | 34 ++++++ .../login/tests/lllogin_test.cpp | 2 + 5 files changed, 176 insertions(+), 29 deletions(-) create mode 100644 indra/test/lltestapp.h diff --git a/indra/llcommon/llcoros.cpp b/indra/llcommon/llcoros.cpp index ffdc558056..9e2ea79f89 100644 --- a/indra/llcommon/llcoros.cpp +++ b/indra/llcommon/llcoros.cpp @@ -48,6 +48,7 @@ #undef BOOST_DISABLE_ASSERTS #endif // other Linden headers +#include "llapp.h" #include "lltimer.h" #include "llevents.h" #include "llerror.h" @@ -58,10 +59,15 @@ #include #endif - -const LLCoros::CoroData& LLCoros::get_CoroData(const std::string& caller) const +// static +LLCoros::CoroData& LLCoros::get_CoroData(const std::string& caller) { - CoroData* current = mCurrent.get(); + CoroData* current{ nullptr }; + // be careful about attempted accesses in the final throes of app shutdown + if (! wasDeleted()) + { + current = instance().mCurrent.get(); + } // For the main() coroutine, the one NOT explicitly launched by launch(), // we never explicitly set mCurrent. Use a static CoroData instance with // canonical values. @@ -78,12 +84,6 @@ const LLCoros::CoroData& LLCoros::get_CoroData(const std::string& caller) const return *current; } -LLCoros::CoroData& LLCoros::get_CoroData(const std::string& caller) -{ - // reuse const implementation, just cast away const-ness of result - return const_cast(const_cast(this)->get_CoroData(caller)); -} - //static LLCoros::coro::id LLCoros::get_self() { @@ -93,7 +93,7 @@ LLCoros::coro::id LLCoros::get_self() //static void LLCoros::set_consuming(bool consuming) { - CoroData& data(LLCoros::instance().get_CoroData("set_consuming()")); + CoroData& data(get_CoroData("set_consuming()")); // DO NOT call this on the main() coroutine. llassert_always(! data.mName.empty()); data.mConsuming = consuming; @@ -102,7 +102,19 @@ void LLCoros::set_consuming(bool consuming) //static bool LLCoros::get_consuming() { - return LLCoros::instance().get_CoroData("get_consuming()").mConsuming; + return get_CoroData("get_consuming()").mConsuming; +} + +// static +void LLCoros::setStatus(const std::string& status) +{ + get_CoroData("setStatus()").mStatus = status; +} + +// static +std::string LLCoros::getStatus() +{ + return get_CoroData("getStatus()").mStatus; } LLCoros::LLCoros(): @@ -118,6 +130,11 @@ LLCoros::LLCoros(): { } +LLCoros::~LLCoros() +{ + printActiveCoroutines("at LLCoros destruction"); +} + std::string LLCoros::generateDistinctName(const std::string& prefix) const { static int unique = 0; @@ -166,19 +183,19 @@ void LLCoros::setStackSize(S32 stacksize) mStackSize = stacksize; } -void LLCoros::printActiveCoroutines() +void LLCoros::printActiveCoroutines(const std::string& when) { - LL_INFOS("LLCoros") << "Number of active coroutines: " << (S32)mCoros.size() << LL_ENDL; + LL_INFOS("LLCoros") << "Number of active coroutines " << when + << ": " << (S32)mCoros.size() << LL_ENDL; if (mCoros.size() > 0) { LL_INFOS("LLCoros") << "-------------- List of active coroutines ------------"; - CoroMap::iterator iter; - CoroMap::iterator end = mCoros.end(); F64 time = LLTimer::getTotalSeconds(); - for (iter = mCoros.begin(); iter != end; iter++) + for (const auto& pair : mCoros) { - F64 life_time = time - iter->second->mCreationTime; - LL_CONT << LL_NEWLINE << "Name: " << iter->first << " life: " << life_time; + F64 life_time = time - pair.second->mCreationTime; + LL_CONT << LL_NEWLINE << pair.first << ' ' << pair.second->mStatus + << " life: " << life_time; } LL_CONT << LL_ENDL; LL_INFOS("LLCoros") << "-----------------------------------------------------" << LL_ENDL; @@ -244,17 +261,19 @@ void LLCoros::winlevel(const callable_t& callable) #endif // Top-level wrapper around caller's coroutine callable. -void LLCoros::toplevel(const std::string& name, const callable_t& callable) +// Normally we like to pass strings and such by const reference -- but in this +// case, we WANT to copy both the name and the callable to our local stack! +void LLCoros::toplevel(std::string name, callable_t callable) { - CoroData* corodata = new(std::nothrow) CoroData(name); + CoroData* corodata = new CoroData(name); if (corodata == NULL) { // Out of memory? printActiveCoroutines(); LL_ERRS("LLCoros") << "Failed to start coroutine: " << name << " Stacksize: " << mStackSize << " Total coroutines: " << mCoros.size() << LL_ENDL; } - // Store it in our pointer map. Oddly, must cast away const-ness of key. - mCoros.insert(const_cast(name), corodata); + // Store it in our pointer map. + mCoros.insert(name, corodata); // also set it as current mCurrent.reset(corodata); @@ -267,18 +286,39 @@ void LLCoros::toplevel(const std::string& name, const callable_t& callable) callable(); #endif } + catch (const Stop& exc) + { + LL_INFOS("LLCoros") << "coroutine " << name << " terminating because " + << exc.what() << LL_ENDL; + } catch (const LLContinueError&) { // Any uncaught exception derived from LLContinueError will be caught // here and logged. This coroutine will terminate but the rest of the // viewer will carry on. - LOG_UNHANDLED_EXCEPTION(STRINGIZE("coroutine " << corodata->mName)); + LOG_UNHANDLED_EXCEPTION(STRINGIZE("coroutine " << name)); } catch (...) { // Any OTHER kind of uncaught exception will cause the viewer to // crash, hopefully informatively. - CRASH_ON_UNHANDLED_EXCEPTION(STRINGIZE("coroutine " << corodata->mName)); + CRASH_ON_UNHANDLED_EXCEPTION(STRINGIZE("coroutine " << name)); + } +} + +void LLCoros::checkStop() +{ + if (wasDeleted()) + { + LLTHROW(Shutdown("LLCoros was deleted")); + } + if (LLApp::isStopped()) + { + LLTHROW(Stopped("viewer is stopped")); + } + if (! LLApp::isRunning()) + { + LLTHROW(Stopping("viewer is stopping")); } } @@ -294,6 +334,19 @@ void LLCoros::delete_CoroData(CoroData* cdptr) { // This custom cleanup function is necessarily static. Find and bind the // LLCoros instance. + // In case the LLCoros instance has already been deleted, just warn and + // scoot. We do NOT want to reinstantiate LLCoros during shutdown! + if (wasDeleted()) + { + // The LLSingletons involved in logging may have been deleted too. + // This warning may help developers track down coroutines that have + // not yet been cleaned up. + // But cdptr is very likely a dangling pointer by this time, so don't + // try to dereference mName. + logwarns("Coroutine terminating after LLCoros instance deleted"); + return; + } + LLCoros& self(LLCoros::instance()); // We set mCurrent on entry to a new fiber, expecting that the // corresponding entry has already been stored in mCoros. It is an diff --git a/indra/llcommon/llcoros.h b/indra/llcommon/llcoros.h index dedb6c8eca..de7b691284 100644 --- a/indra/llcommon/llcoros.h +++ b/indra/llcommon/llcoros.h @@ -29,6 +29,7 @@ #if ! defined(LL_LLCOROS_H) #define LL_LLCOROS_H +#include "llexception.h" #include #include #include @@ -74,6 +75,7 @@ class LL_COMMON_API LLCoros: public LLSingleton { LLSINGLETON(LLCoros); + ~LLCoros(); public: /// The viewer's use of the term "coroutine" became deeply embedded before /// the industry term "fiber" emerged to distinguish userland threads from @@ -147,8 +149,8 @@ public: */ void setStackSize(S32 stacksize); - /// for delayed initialization - void printActiveCoroutines(); + /// diagnostic + void printActiveCoroutines(const std::string& when=std::string()); /// get the current coro::id for those who really really care static coro::id get_self(); @@ -176,6 +178,7 @@ public: { set_consuming(consuming); } + OverrideConsuming(const OverrideConsuming&) = delete; ~OverrideConsuming() { set_consuming(mPrevConsuming); @@ -185,6 +188,58 @@ public: bool mPrevConsuming; }; + /// set string coroutine status for diagnostic purposes + static void setStatus(const std::string& status); + static std::string getStatus(); + + /// RAII control of status + class TempStatus + { + public: + TempStatus(const std::string& status): + mOldStatus(getStatus()) + { + setStatus(status); + } + TempStatus(const TempStatus&) = delete; + ~TempStatus() + { + setStatus(mOldStatus); + } + + private: + std::string mOldStatus; + }; + + /// thrown by checkStop() + struct Stop: public LLContinueError + { + Stop(const std::string& what): LLContinueError(what) {} + }; + + /// early stages + struct Stopping: public Stop + { + Stopping(const std::string& what): Stop(what) {} + }; + + /// cleaning up + struct Stopped: public Stop + { + Stopped(const std::string& what): Stop(what) {} + }; + + /// cleaned up -- not much survives! + struct Shutdown: public Stop + { + Shutdown(const std::string& what): Stop(what) {} + }; + + /// Call this intermittently if there's a chance your coroutine might + /// continue running into application shutdown. Throws Stop if LLCoros has + /// been cleaned up. + static void checkStop(); + /** * Aliases for promise and future. An older underlying future implementation * required us to wrap future; that's no longer needed. However -- if it's @@ -204,13 +259,12 @@ public: private: std::string generateDistinctName(const std::string& prefix) const; - void toplevel(const std::string& name, const callable_t& callable); + void toplevel(std::string name, callable_t callable); struct CoroData; #if LL_WINDOWS static void winlevel(const callable_t& callable); #endif - CoroData& get_CoroData(const std::string& caller); - const CoroData& get_CoroData(const std::string& caller) const; + static CoroData& get_CoroData(const std::string& caller); S32 mStackSize; @@ -223,6 +277,8 @@ private: const std::string mName; // set_consuming() state bool mConsuming; + // setStatus() state + std::string mStatus; F64 mCreationTime; // since epoch }; typedef boost::ptr_map CoroMap; diff --git a/indra/llcommon/tests/lleventcoro_test.cpp b/indra/llcommon/tests/lleventcoro_test.cpp index c13920eefd..032923a108 100644 --- a/indra/llcommon/tests/lleventcoro_test.cpp +++ b/indra/llcommon/tests/lleventcoro_test.cpp @@ -40,6 +40,7 @@ #include #include "../test/lltut.h" +#include "../test/lltestapp.h" #include "llsd.h" #include "llsdutil.h" #include "llevents.h" @@ -98,6 +99,7 @@ namespace tut std::string replyName, errorName, threw, stringdata; LLSD result, errordata; int which; + LLTestApp testApp; void explicit_wait(boost::shared_ptr>& cbp); void waitForEventOn1(); 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) */ diff --git a/indra/viewer_components/login/tests/lllogin_test.cpp b/indra/viewer_components/login/tests/lllogin_test.cpp index 23db8d0fe3..0255e10e53 100644 --- a/indra/viewer_components/login/tests/lllogin_test.cpp +++ b/indra/viewer_components/login/tests/lllogin_test.cpp @@ -42,6 +42,7 @@ // other Linden headers #include "llsd.h" #include "../../../test/lltut.h" +#include "../../../test/lltestapp.h" //#define DEBUG_ON #include "../../../test/debug.h" #include "llevents.h" @@ -201,6 +202,7 @@ namespace tut pumps.clear(); } LLEventPumps& pumps; + LLTestApp testApp; }; typedef test_group llviewerlogin_group; From d24a3d4fcc68adbd1ae58befd719889e85c7c705 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 22 Oct 2019 17:14:26 -0400 Subject: [PATCH 23/38] DRTVWR-476: Terminate long-lived coroutines to avoid shutdown crash. Add LLCoros::TempStatus instances around known suspension points so printActiveCoroutines() can report what each suspended coroutine is waiting for. Similarly, sprinkle checkStop() calls at known suspension points. Make LLApp::setStatus() post an event to a new LLEventPump "LLApp" with a string corresponding to the status value being set, but only until ~LLEventPumps() -- since setStatus() also gets called very late in the application's lifetime. Make postAndSuspendSetup() (used by postAndSuspend(), suspendUntilEventOn(), postAndSuspendWithTimeout(), suspendUntilEventOnWithTimeout()) add a listener on the new "LLApp" LLEventPump that pushes the new LLCoros::Stopping exception to the coroutine waiting on the LLCoros::Promise. Make it return the new LLBoundListener along with the previous one. Accordingly, make postAndSuspend() and postAndSuspendWithTimeout() store the new LLBoundListener returned by postAndSuspendSetup() in a LLTempBoundListener (as with the previous one) so it will automatically disconnect once the wait is over. Make each LLCoprocedurePool instance listen on "LLApp" with a listener that closes the queue on which new work items are dispatched. Closing the queue causes the waiting dispatch coroutine to terminate. Store the connection in an LLTempBoundListener on the LLCoprocedurePool so it will disconnect automatically on destruction. Refactor the loop in coprocedureInvokerCoro() to instantiate TempStatus around the suspending call. Change a couple spammy LL_INFOS() calls to LL_DEBUGS(). Give all logging calls in that module a "CoProcMgr" tag to make it straightforward to re-enable the LL_DEBUGS() calls as desired. --- indra/llcommon/llapp.cpp | 36 ++++++++++- indra/llcommon/lleventcoro.cpp | 80 +++++++++++++++++++----- indra/llmessage/llcoproceduremanager.cpp | 47 +++++++++++--- 3 files changed, 138 insertions(+), 25 deletions(-) diff --git a/indra/llcommon/llapp.cpp b/indra/llcommon/llapp.cpp index 421af3006e..3dab632aef 100644 --- a/indra/llcommon/llapp.cpp +++ b/indra/llcommon/llapp.cpp @@ -49,6 +49,8 @@ #include "google_breakpad/exception_handler.h" #include "stringize.h" #include "llcleanup.h" +#include "llevents.h" +#include "llsdutil.h" // // Signal handling @@ -561,10 +563,42 @@ void LLApp::runErrorHandler() LLApp::setStopped(); } +namespace +{ + +static std::map statusDesc +{ + { LLApp::APP_STATUS_RUNNING, "running" }, + { LLApp::APP_STATUS_QUITTING, "quitting" }, + { LLApp::APP_STATUS_STOPPED, "stopped" }, + { LLApp::APP_STATUS_ERROR, "error" } +}; + +} // anonymous namespace + // static void LLApp::setStatus(EAppStatus status) { - sStatus = status; + sStatus = status; + + // This can also happen very late in the application lifecycle -- don't + // resurrect a deleted LLSingleton + if (! LLEventPumps::wasDeleted()) + { + // notify interested parties of status change + LLSD statsd; + auto found = statusDesc.find(status); + if (found != statusDesc.end()) + { + statsd = found->second; + } + else + { + // unknown status? at least report value + statsd = LLSD::Integer(status); + } + LLEventPumps::instance().obtain("LLApp").post(llsd::map("status", statsd)); + } } diff --git a/indra/llcommon/lleventcoro.cpp b/indra/llcommon/lleventcoro.cpp index bcc35955cc..e15ea27945 100644 --- a/indra/llcommon/lleventcoro.cpp +++ b/indra/llcommon/lleventcoro.cpp @@ -32,6 +32,7 @@ #include "lleventcoro.h" // STL headers #include +#include // std headers // external library headers #include @@ -39,6 +40,7 @@ #include "llsdserialize.h" #include "llerror.h" #include "llcoros.h" +#include "stringize.h" namespace { @@ -146,29 +148,39 @@ void storeToLLSDPath(LLSD& dest, const LLSD& rawPath, const LLSD& value) void llcoro::suspend() { + LLCoros::checkStop(); + LLCoros::TempStatus st("waiting one tick"); boost::this_fiber::yield(); } void llcoro::suspendUntilTimeout(float seconds) { + LLCoros::checkStop(); // The fact that we accept non-integer seconds means we should probably // use granularity finer than one second. However, given the overhead of // the rest of our processing, it seems silly to use granularity finer // than a millisecond. + LLCoros::TempStatus st(STRINGIZE("waiting for " << seconds << "s")); boost::this_fiber::sleep_for(std::chrono::milliseconds(long(seconds * 1000))); } namespace { -LLBoundListener postAndSuspendSetup(const std::string& callerName, - const std::string& listenerName, - LLCoros::Promise& promise, - const LLSD& event, - const LLEventPumpOrPumpName& requestPumpP, - const LLEventPumpOrPumpName& replyPumpP, - const LLSD& replyPumpNamePath) +// returns a listener on replyPumpP, also on "mainloop" -- both should be +// stored in LLTempBoundListeners on the caller's stack frame +std::pair +postAndSuspendSetup(const std::string& callerName, + const std::string& listenerName, + LLCoros::Promise& promise, + const LLSD& event, + const LLEventPumpOrPumpName& requestPumpP, + const LLEventPumpOrPumpName& replyPumpP, + const LLSD& replyPumpNamePath) { + // Before we get any farther -- should we be stopping instead of + // suspending? + LLCoros::checkStop(); // Get the consuming attribute for THIS coroutine, the one that's about to // suspend. Don't call get_consuming() in the lambda body: that would // return the consuming attribute for some other coroutine, most likely @@ -178,6 +190,38 @@ LLBoundListener postAndSuspendSetup(const std::string& callerName, // value to the promise, thus fulfilling its future llassert_always_msg(replyPumpP, ("replyPump required for " + callerName)); LLEventPump& replyPump(replyPumpP.getPump()); + // The relative order of the two listen() calls below would only matter if + // "LLApp" were an LLEventMailDrop. But if we ever go there, we'd want to + // notice the pending LLApp status first. + LLBoundListener stopper( + LLEventPumps::instance().obtain("LLApp").listen( + listenerName, + [&promise, listenerName](const LLSD& status) + { + // anything except "running" should wake up the waiting + // coroutine + auto& statsd = status["status"]; + if (statsd.asString() != "running") + { + LL_DEBUGS("lleventcoro") << listenerName + << " spotted status " << statsd + << ", throwing Stopping" << LL_ENDL; + try + { + promise.set_exception( + std::make_exception_ptr( + LLCoros::Stopping("status " + statsd.asString()))); + } + catch (const boost::fibers::promise_already_satisfied& exc) + { + LL_WARNS("lleventcoro") << listenerName + << " couldn't throw Stopping " + "because promise already set" << LL_ENDL; + } + } + // do not consume -- every listener must see status + return false; + })); LLBoundListener connection( replyPump.listen( listenerName, @@ -200,6 +244,7 @@ LLBoundListener postAndSuspendSetup(const std::string& callerName, return false; } })); + // skip the "post" part if requestPump is default-constructed if (requestPumpP) { @@ -219,7 +264,7 @@ LLBoundListener postAndSuspendSetup(const std::string& callerName, LL_DEBUGS("lleventcoro") << callerName << ": coroutine " << listenerName << " about to wait on LLEventPump " << replyPump.getName() << LL_ENDL; - return connection; + return { connection, stopper }; } } // anonymous @@ -230,15 +275,17 @@ LLSD llcoro::postAndSuspend(const LLSD& event, const LLEventPumpOrPumpName& requ LLCoros::Promise promise; std::string listenerName(listenerNameForCoro()); - // Store connection into an LLTempBoundListener so we implicitly + // Store both connections into LLTempBoundListeners so we implicitly // disconnect on return from this function. - LLTempBoundListener connection = + auto connections = postAndSuspendSetup("postAndSuspend()", listenerName, promise, event, requestPump, replyPump, replyPumpNamePath); + LLTempBoundListener connection(connections.first), stopper(connections.second); // declare the future LLCoros::Future future = LLCoros::getFuture(promise); // calling get() on the future makes us wait for it + LLCoros::TempStatus st(STRINGIZE("waiting for " << replyPump.getPump().getName())); LLSD value(future.get()); LL_DEBUGS("lleventcoro") << "postAndSuspend(): coroutine " << listenerName << " resuming with " << value << LL_ENDL; @@ -255,17 +302,22 @@ LLSD llcoro::postAndSuspendWithTimeout(const LLSD& event, LLCoros::Promise promise; std::string listenerName(listenerNameForCoro()); - // Store connection into an LLTempBoundListener so we implicitly + // Store both connections into LLTempBoundListeners so we implicitly // disconnect on return from this function. - LLTempBoundListener connection = + auto connections = postAndSuspendSetup("postAndSuspendWithTimeout()", listenerName, promise, event, requestPump, replyPump, replyPumpNamePath); + LLTempBoundListener connection(connections.first), stopper(connections.second); // declare the future LLCoros::Future future = LLCoros::getFuture(promise); // wait for specified timeout - boost::fibers::future_status status = - future.wait_for(std::chrono::milliseconds(long(timeout * 1000))); + boost::fibers::future_status status; + { + LLCoros::TempStatus st(STRINGIZE("waiting for " << replyPump.getPump().getName() + << " for " << timeout << "s")); + status = future.wait_for(std::chrono::milliseconds(long(timeout * 1000))); + } // if the future is NOT yet ready, return timeoutResult instead if (status == boost::fibers::future_status::timeout) { diff --git a/indra/llmessage/llcoproceduremanager.cpp b/indra/llmessage/llcoproceduremanager.cpp index 89eb00a2b7..a8f6b8aa67 100644 --- a/indra/llmessage/llcoproceduremanager.cpp +++ b/indra/llmessage/llcoproceduremanager.cpp @@ -102,6 +102,7 @@ private: size_t mPoolSize; CoprocQueue_t mPendingCoprocs; ActiveCoproc_t mActiveCoprocs; + LLTempBoundListener mStatusListener; typedef std::map CoroAdapterMap_t; LLCore::HttpRequest::policy_t mHTTPPolicy; @@ -149,7 +150,7 @@ LLCoprocedureManager::poolPtr_t LLCoprocedureManager::initializePool(const std:: mPropertyDefineFn(keyName, size, "Coroutine Pool size for " + poolName); } - LL_WARNS() << "LLCoprocedureManager: No setting for \"" << keyName << "\" setting pool size to default of " << size << LL_ENDL; + LL_WARNS("CoProcMgr") << "LLCoprocedureManager: No setting for \"" << keyName << "\" setting pool size to default of " << size << LL_ENDL; } poolPtr_t pool(new LLCoprocedurePool(poolName, size)); @@ -214,9 +215,28 @@ LLCoprocedurePool::LLCoprocedurePool(const std::string &poolName, size_t size): mPoolName(poolName), mPoolSize(size), mPendingCoprocs(DEFAULT_QUEUE_SIZE), - mCoroMapping(), - mHTTPPolicy(LLCore::HttpRequest::DEFAULT_POLICY_ID) + mHTTPPolicy(LLCore::HttpRequest::DEFAULT_POLICY_ID), + mCoroMapping() { + // store in our LLTempBoundListener so that when the LLCoprocedurePool is + // destroyed, we implicitly disconnect from this LLEventPump + mStatusListener = LLEventPumps::instance().obtain("LLApp").listen( + poolName, + [this, poolName](const LLSD& status) + { + auto& statsd = status["status"]; + if (statsd.asString() != "running") + { + LL_INFOS("CoProcMgr") << "Pool " << poolName + << " closing queue because status " << statsd + << LL_ENDL; + // This should ensure that all waiting coprocedures in this + // pool will wake up and terminate. + mPendingCoprocs.close(); + } + return false; + }); + for (size_t count = 0; count < mPoolSize; ++count) { LLCoreHttpUtil::HttpCoroutineAdapter::ptr_t httpAdapter(new LLCoreHttpUtil::HttpCoroutineAdapter( mPoolName + "Adapter", mHTTPPolicy)); @@ -227,7 +247,7 @@ LLCoprocedurePool::LLCoprocedurePool(const std::string &poolName, size_t size): mCoroMapping.insert(CoroAdapterMap_t::value_type(pooledCoro, httpAdapter)); } - LL_INFOS() << "Created coprocedure pool named \"" << mPoolName << "\" with " << size << " items." << LL_ENDL; + LL_INFOS("CoProcMgr") << "Created coprocedure pool named \"" << mPoolName << "\" with " << size << " items." << LL_ENDL; } LLCoprocedurePool::~LLCoprocedurePool() @@ -240,7 +260,7 @@ LLUUID LLCoprocedurePool::enqueueCoprocedure(const std::string &name, LLCoproced LLUUID id(LLUUID::generateNewID()); mPendingCoprocs.push(QueuedCoproc::ptr_t(new QueuedCoproc(name, id, proc))); - LL_INFOS() << "Coprocedure(" << name << ") enqueued with id=" << id.asString() << " in pool \"" << mPoolName << "\"" << LL_ENDL; + LL_INFOS("CoProcMgr") << "Coprocedure(" << name << ") enqueued with id=" << id.asString() << " in pool \"" << mPoolName << "\"" << LL_ENDL; return id; } @@ -250,8 +270,17 @@ void LLCoprocedurePool::coprocedureInvokerCoro(LLCoreHttpUtil::HttpCoroutineAdap { QueuedCoproc::ptr_t coproc; boost::fibers::channel_op_status status; - while ((status = mPendingCoprocs.pop_wait_for(coproc, std::chrono::seconds(10))) != boost::fibers::channel_op_status::closed) + for (;;) { + { + LLCoros::TempStatus st("waiting for work for 10s"); + status = mPendingCoprocs.pop_wait_for(coproc, std::chrono::seconds(10)); + } + if (status == boost::fibers::channel_op_status::closed) + { + break; + } + if(status == boost::fibers::channel_op_status::timeout) { LL_INFOS_ONCE() << "pool '" << mPoolName << "' stalled." << LL_ENDL; @@ -260,8 +289,7 @@ void LLCoprocedurePool::coprocedureInvokerCoro(LLCoreHttpUtil::HttpCoroutineAdap ActiveCoproc_t::iterator itActive = mActiveCoprocs.insert(ActiveCoproc_t::value_type(coproc->mId, httpAdapter)).first; - // Nicky: This is super spammy. Consider using LL_DEBUGS here? - LL_INFOS() << "Dequeued and invoking coprocedure(" << coproc->mName << ") with id=" << coproc->mId.asString() << " in pool \"" << mPoolName << "\"" << LL_ENDL; + LL_DEBUGS("CoProcMgr") << "Dequeued and invoking coprocedure(" << coproc->mName << ") with id=" << coproc->mId.asString() << " in pool \"" << mPoolName << "\"" << LL_ENDL; try { @@ -277,8 +305,7 @@ void LLCoprocedurePool::coprocedureInvokerCoro(LLCoreHttpUtil::HttpCoroutineAdap throw; } - // Nicky: This is super spammy. Consider using LL_DEBUGS here? - LL_INFOS() << "Finished coprocedure(" << coproc->mName << ")" << " in pool \"" << mPoolName << "\"" << LL_ENDL; + LL_DEBUGS("CoProcMgr") << "Finished coprocedure(" << coproc->mName << ")" << " in pool \"" << mPoolName << "\"" << LL_ENDL; mActiveCoprocs.erase(itActive); } From adc5543f2035d66943df5838c8c0bbd96619d790 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 22 Oct 2019 20:48:36 -0400 Subject: [PATCH 24/38] DRTVWR-476: Don't name the caught exception unless we're going to reference it. --- indra/llcommon/lleventcoro.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/indra/llcommon/lleventcoro.cpp b/indra/llcommon/lleventcoro.cpp index e15ea27945..378f72ca4c 100644 --- a/indra/llcommon/lleventcoro.cpp +++ b/indra/llcommon/lleventcoro.cpp @@ -212,7 +212,7 @@ postAndSuspendSetup(const std::string& callerName, std::make_exception_ptr( LLCoros::Stopping("status " + statsd.asString()))); } - catch (const boost::fibers::promise_already_satisfied& exc) + catch (const boost::fibers::promise_already_satisfied&) { LL_WARNS("lleventcoro") << listenerName << " couldn't throw Stopping " From 3fb47a820bbded434f11d7d555613c17345f974a Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 23 Oct 2019 11:13:04 -0400 Subject: [PATCH 25/38] OPEN-347: Remove ineffective, disabled 'hdiutil internet-enable'. macOS Catalina no longer even recognizes that subcommand. Remove it. --- indra/newview/viewer_manifest.py | 1 - 1 file changed, 1 deletion(-) diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index fcb97ded8f..c490b58914 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -1370,7 +1370,6 @@ class DarwinManifest(ViewerManifest): print "Converting temp disk image to final disk image" self.run_command(['hdiutil', 'convert', sparsename, '-format', 'UDZO', '-imagekey', 'zlib-level=9', '-o', finalname]) - self.run_command(['hdiutil', 'internet-enable', '-yes', finalname]) # get rid of the temp file self.package_file = finalname self.remove(sparsename) From 33c417033630787629c400277cdf541be64e4274 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 24 Oct 2019 12:54:38 -0400 Subject: [PATCH 26/38] DRTVWR-476: Pump coroutines a few more times when we start quitting. By the time "LLApp" listeners are notified that the app is quitting, the mainloop is no longer running. Even though those listeners do things like close work queues and inject exceptions into pending promises, any coroutines waiting on those resources must regain control before they can notice and shut down properly. Add a final "LLApp" listener that resumes ready coroutines a few more times. Make sure every other "LLApp" listener is positioned before that new one. --- indra/llcommon/llcoros.cpp | 32 ++++++++++++++++++++++++ indra/llcommon/llcoros.h | 3 +++ indra/llcommon/lleventcoro.cpp | 5 +++- indra/llmessage/llcoproceduremanager.cpp | 9 ++++--- 4 files changed, 45 insertions(+), 4 deletions(-) diff --git a/indra/llcommon/llcoros.cpp b/indra/llcommon/llcoros.cpp index 9e2ea79f89..2611e893b1 100644 --- a/indra/llcommon/llcoros.cpp +++ b/indra/llcommon/llcoros.cpp @@ -128,6 +128,38 @@ LLCoros::LLCoros(): mStackSize(256*1024) #endif { + // Set up a listener to notice when the viewer is starting to shut down. + // Store the connection in an LLTempBoundListener so it will automatically + // disconnect. + mAppListener = LLEventPumps::instance().obtain("LLApp").listen( + "final", // must be the LAST listener on this LLEventPump + [this](const LLSD& status) + { + if (status["status"].asString() == "quitting") + { + // Other LLApp status-change listeners do things like close + // work queues and inject the Stop exception into pending + // promises, to force coroutines waiting on those things to + // notice and terminate. The only problem is that by the time + // LLApp sets "quitting" status, the main loop has stopped + // pumping the fiber scheduler with yield() calls. A waiting + // coroutine still might not wake up until after resources on + // which it depends have been freed. Pump it a few times + // ourselves. Of course, stop pumping as soon as the last of + // the coroutines has terminated. + for (size_t count = 0; count < 10 && ! mCoros.empty(); ++count) + { + // don't use llcoro::suspend() because that module depends + // on this one + boost::this_fiber::yield(); + } + } + // If we're really the last listener, it shouldn't matter whether + // we consume this event -- but our being last depends on every + // other listen() call specifying before "final", which would be + // all too easy to forget. So do not consume the event. + return false; + }); } LLCoros::~LLCoros() diff --git a/indra/llcommon/llcoros.h b/indra/llcommon/llcoros.h index de7b691284..171d1ebd2a 100644 --- a/indra/llcommon/llcoros.h +++ b/indra/llcommon/llcoros.h @@ -34,6 +34,7 @@ #include #include #include "llsingleton.h" +#include "llevents.h" #include #include #include @@ -284,6 +285,8 @@ private: typedef boost::ptr_map CoroMap; CoroMap mCoros; + LLTempBoundListener mAppListener; + // Identify the current coroutine's CoroData. This local_ptr isn't static // because it's a member of an LLSingleton, and we rely on it being // cleaned up in proper dependency order. diff --git a/indra/llcommon/lleventcoro.cpp b/indra/llcommon/lleventcoro.cpp index 378f72ca4c..8dc41c1291 100644 --- a/indra/llcommon/lleventcoro.cpp +++ b/indra/llcommon/lleventcoro.cpp @@ -193,6 +193,7 @@ postAndSuspendSetup(const std::string& callerName, // The relative order of the two listen() calls below would only matter if // "LLApp" were an LLEventMailDrop. But if we ever go there, we'd want to // notice the pending LLApp status first. + // Run this listener before the "final" listener. LLBoundListener stopper( LLEventPumps::instance().obtain("LLApp").listen( listenerName, @@ -221,7 +222,9 @@ postAndSuspendSetup(const std::string& callerName, } // do not consume -- every listener must see status return false; - })); + }, + LLEventPump::NameList{}, // after + LLEventPump::NameList{ "final "})); // before LLBoundListener connection( replyPump.listen( listenerName, diff --git a/indra/llmessage/llcoproceduremanager.cpp b/indra/llmessage/llcoproceduremanager.cpp index a8f6b8aa67..a71a31bfd2 100644 --- a/indra/llmessage/llcoproceduremanager.cpp +++ b/indra/llmessage/llcoproceduremanager.cpp @@ -218,8 +218,9 @@ LLCoprocedurePool::LLCoprocedurePool(const std::string &poolName, size_t size): mHTTPPolicy(LLCore::HttpRequest::DEFAULT_POLICY_ID), mCoroMapping() { - // store in our LLTempBoundListener so that when the LLCoprocedurePool is - // destroyed, we implicitly disconnect from this LLEventPump + // Store in our LLTempBoundListener so that when the LLCoprocedurePool is + // destroyed, we implicitly disconnect from this LLEventPump. + // Run this listener before the "final" listener. mStatusListener = LLEventPumps::instance().obtain("LLApp").listen( poolName, [this, poolName](const LLSD& status) @@ -235,7 +236,9 @@ LLCoprocedurePool::LLCoprocedurePool(const std::string &poolName, size_t size): mPendingCoprocs.close(); } return false; - }); + }, + LLEventPump::NameList{}, // after + LLEventPump::NameList{ "final "}); // before for (size_t count = 0; count < mPoolSize; ++count) { From 24d4f87afb9c2a3a35057d261220a9b6c7f9b34b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 24 Oct 2019 16:05:37 -0400 Subject: [PATCH 27/38] DRTVWR-476: Back out changeset 40c0c6a8407d ("final" LLApp listener) --- indra/llcommon/llcoros.cpp | 32 ------------------------ indra/llcommon/llcoros.h | 3 --- indra/llcommon/lleventcoro.cpp | 5 +--- indra/llmessage/llcoproceduremanager.cpp | 9 +++---- 4 files changed, 4 insertions(+), 45 deletions(-) diff --git a/indra/llcommon/llcoros.cpp b/indra/llcommon/llcoros.cpp index 2611e893b1..9e2ea79f89 100644 --- a/indra/llcommon/llcoros.cpp +++ b/indra/llcommon/llcoros.cpp @@ -128,38 +128,6 @@ LLCoros::LLCoros(): mStackSize(256*1024) #endif { - // Set up a listener to notice when the viewer is starting to shut down. - // Store the connection in an LLTempBoundListener so it will automatically - // disconnect. - mAppListener = LLEventPumps::instance().obtain("LLApp").listen( - "final", // must be the LAST listener on this LLEventPump - [this](const LLSD& status) - { - if (status["status"].asString() == "quitting") - { - // Other LLApp status-change listeners do things like close - // work queues and inject the Stop exception into pending - // promises, to force coroutines waiting on those things to - // notice and terminate. The only problem is that by the time - // LLApp sets "quitting" status, the main loop has stopped - // pumping the fiber scheduler with yield() calls. A waiting - // coroutine still might not wake up until after resources on - // which it depends have been freed. Pump it a few times - // ourselves. Of course, stop pumping as soon as the last of - // the coroutines has terminated. - for (size_t count = 0; count < 10 && ! mCoros.empty(); ++count) - { - // don't use llcoro::suspend() because that module depends - // on this one - boost::this_fiber::yield(); - } - } - // If we're really the last listener, it shouldn't matter whether - // we consume this event -- but our being last depends on every - // other listen() call specifying before "final", which would be - // all too easy to forget. So do not consume the event. - return false; - }); } LLCoros::~LLCoros() diff --git a/indra/llcommon/llcoros.h b/indra/llcommon/llcoros.h index 171d1ebd2a..de7b691284 100644 --- a/indra/llcommon/llcoros.h +++ b/indra/llcommon/llcoros.h @@ -34,7 +34,6 @@ #include #include #include "llsingleton.h" -#include "llevents.h" #include #include #include @@ -285,8 +284,6 @@ private: typedef boost::ptr_map CoroMap; CoroMap mCoros; - LLTempBoundListener mAppListener; - // Identify the current coroutine's CoroData. This local_ptr isn't static // because it's a member of an LLSingleton, and we rely on it being // cleaned up in proper dependency order. diff --git a/indra/llcommon/lleventcoro.cpp b/indra/llcommon/lleventcoro.cpp index 8dc41c1291..378f72ca4c 100644 --- a/indra/llcommon/lleventcoro.cpp +++ b/indra/llcommon/lleventcoro.cpp @@ -193,7 +193,6 @@ postAndSuspendSetup(const std::string& callerName, // The relative order of the two listen() calls below would only matter if // "LLApp" were an LLEventMailDrop. But if we ever go there, we'd want to // notice the pending LLApp status first. - // Run this listener before the "final" listener. LLBoundListener stopper( LLEventPumps::instance().obtain("LLApp").listen( listenerName, @@ -222,9 +221,7 @@ postAndSuspendSetup(const std::string& callerName, } // do not consume -- every listener must see status return false; - }, - LLEventPump::NameList{}, // after - LLEventPump::NameList{ "final "})); // before + })); LLBoundListener connection( replyPump.listen( listenerName, diff --git a/indra/llmessage/llcoproceduremanager.cpp b/indra/llmessage/llcoproceduremanager.cpp index a71a31bfd2..a8f6b8aa67 100644 --- a/indra/llmessage/llcoproceduremanager.cpp +++ b/indra/llmessage/llcoproceduremanager.cpp @@ -218,9 +218,8 @@ LLCoprocedurePool::LLCoprocedurePool(const std::string &poolName, size_t size): mHTTPPolicy(LLCore::HttpRequest::DEFAULT_POLICY_ID), mCoroMapping() { - // Store in our LLTempBoundListener so that when the LLCoprocedurePool is - // destroyed, we implicitly disconnect from this LLEventPump. - // Run this listener before the "final" listener. + // store in our LLTempBoundListener so that when the LLCoprocedurePool is + // destroyed, we implicitly disconnect from this LLEventPump mStatusListener = LLEventPumps::instance().obtain("LLApp").listen( poolName, [this, poolName](const LLSD& status) @@ -236,9 +235,7 @@ LLCoprocedurePool::LLCoprocedurePool(const std::string &poolName, size_t size): mPendingCoprocs.close(); } return false; - }, - LLEventPump::NameList{}, // after - LLEventPump::NameList{ "final "}); // before + }); for (size_t count = 0; count < mPoolSize; ++count) { From edd0ad6c793c24b34b1da9631adbd8d6caab4212 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 25 Oct 2019 06:55:45 -0400 Subject: [PATCH 28/38] DRTVWR-476: Keep coroutine-local data on toplevel()'s stack frame. Instead of heap-allocating a CoroData instance per coroutine, storing the pointer in a ptr_map and deleting it from the ptr_map once the fiber_specific_ptr for that coroutine is cleaned up -- just declare a stack instance on the top-level stack frame, the simplest C++ lifespan management. Derive CoroData from LLInstanceTracker to detect potential name collisions and to enumerate instances. Continue registering each coroutine's CoroData instance in our fiber_specific_ptr, but use a no-op deleter function. Make ~LLCoros() directly pump the fiber scheduler a few times, instead of having a special "LLApp" listener. --- indra/llcommon/llcoros.cpp | 97 ++++++++++++------------------ indra/llcommon/llcoros.h | 13 +--- indra/llcommon/llinstancetracker.h | 4 +- indra/newview/llvoicevivox.cpp | 6 +- 4 files changed, 48 insertions(+), 72 deletions(-) diff --git a/indra/llcommon/llcoros.cpp b/indra/llcommon/llcoros.cpp index 9e2ea79f89..6649916661 100644 --- a/indra/llcommon/llcoros.cpp +++ b/indra/llcommon/llcoros.cpp @@ -76,9 +76,9 @@ LLCoros::CoroData& LLCoros::get_CoroData(const std::string& caller) // It's tempting to provide a distinct name for each thread's "main // coroutine." But as getName() has always returned the empty string // to mean "not in a coroutine," empty string should suffice here. - static CoroData sMain(""); - // We need not reset() the local_ptr to this read-only data: reuse the - // same instance for every thread's main coroutine. + static thread_local CoroData sMain(""); + // We need not reset() the local_ptr to this instance; we'll simply + // find it again every time we discover that current is null. current = &sMain; } return *current; @@ -123,16 +123,36 @@ LLCoros::LLCoros(): // boost::context::guarded_stack_allocator::default_stacksize(); // empirically this is insufficient. #if ADDRESS_SIZE == 64 - mStackSize(512*1024) + mStackSize(512*1024), #else - mStackSize(256*1024) + mStackSize(256*1024), #endif + // mCurrent does NOT own the current CoroData instance -- it simply + // points to it. So initialize it with a no-op deleter. + mCurrent{ [](CoroData*){} } { } LLCoros::~LLCoros() { - printActiveCoroutines("at LLCoros destruction"); + printActiveCoroutines("at entry to ~LLCoros()"); + // Other LLApp status-change listeners do things like close + // work queues and inject the Stop exception into pending + // promises, to force coroutines waiting on those things to + // notice and terminate. The only problem is that by the time + // LLApp sets "quitting" status, the main loop has stopped + // pumping the fiber scheduler with yield() calls. A waiting + // coroutine still might not wake up until after resources on + // which it depends have been freed. Pump it a few times + // ourselves. Of course, stop pumping as soon as the last of + // the coroutines has terminated. + for (size_t count = 0; count < 10 && CoroData::instanceCount() > 0; ++count) + { + // don't use llcoro::suspend() because that module depends + // on this one + boost::this_fiber::yield(); + } + printActiveCoroutines("after pumping"); } std::string LLCoros::generateDistinctName(const std::string& prefix) const @@ -149,7 +169,7 @@ std::string LLCoros::generateDistinctName(const std::string& prefix) const std::string name(prefix); // Until we find an unused name, append a numeric suffix for uniqueness. - while (mCoros.find(name) != mCoros.end()) + while (CoroData::getInstance(name)) { name = STRINGIZE(prefix << unique++); } @@ -186,16 +206,17 @@ void LLCoros::setStackSize(S32 stacksize) void LLCoros::printActiveCoroutines(const std::string& when) { LL_INFOS("LLCoros") << "Number of active coroutines " << when - << ": " << (S32)mCoros.size() << LL_ENDL; - if (mCoros.size() > 0) + << ": " << CoroData::instanceCount() << LL_ENDL; + if (CoroData::instanceCount() > 0) { LL_INFOS("LLCoros") << "-------------- List of active coroutines ------------"; F64 time = LLTimer::getTotalSeconds(); - for (const auto& pair : mCoros) + for (auto it(CoroData::beginInstances()), end(CoroData::endInstances()); + it != end; ++it) { - F64 life_time = time - pair.second->mCreationTime; - LL_CONT << LL_NEWLINE << pair.first << ' ' << pair.second->mStatus - << " life: " << life_time; + F64 life_time = time - it->mCreationTime; + LL_CONT << LL_NEWLINE + << it->mName << ' ' << it->mStatus << " life: " << life_time; } LL_CONT << LL_ENDL; LL_INFOS("LLCoros") << "-----------------------------------------------------" << LL_ENDL; @@ -265,17 +286,10 @@ void LLCoros::winlevel(const callable_t& callable) // case, we WANT to copy both the name and the callable to our local stack! void LLCoros::toplevel(std::string name, callable_t callable) { - CoroData* corodata = new CoroData(name); - if (corodata == NULL) - { - // Out of memory? - printActiveCoroutines(); - LL_ERRS("LLCoros") << "Failed to start coroutine: " << name << " Stacksize: " << mStackSize << " Total coroutines: " << mCoros.size() << LL_ENDL; - } - // Store it in our pointer map. - mCoros.insert(name, corodata); - // also set it as current - mCurrent.reset(corodata); + // keep the CoroData on this top-level function's stack frame + CoroData corodata(name); + // set it as current + mCurrent.reset(&corodata); // run the code the caller actually wants in the coroutine try @@ -323,43 +337,10 @@ void LLCoros::checkStop() } LLCoros::CoroData::CoroData(const std::string& name): + LLInstanceTracker(name), mName(name), // don't consume events unless specifically directed mConsuming(false), mCreationTime(LLTimer::getTotalSeconds()) { } - -void LLCoros::delete_CoroData(CoroData* cdptr) -{ - // This custom cleanup function is necessarily static. Find and bind the - // LLCoros instance. - // In case the LLCoros instance has already been deleted, just warn and - // scoot. We do NOT want to reinstantiate LLCoros during shutdown! - if (wasDeleted()) - { - // The LLSingletons involved in logging may have been deleted too. - // This warning may help developers track down coroutines that have - // not yet been cleaned up. - // But cdptr is very likely a dangling pointer by this time, so don't - // try to dereference mName. - logwarns("Coroutine terminating after LLCoros instance deleted"); - return; - } - - LLCoros& self(LLCoros::instance()); - // We set mCurrent on entry to a new fiber, expecting that the - // corresponding entry has already been stored in mCoros. It is an - // error if we do not find that entry. - CoroMap::iterator found = self.mCoros.find(cdptr->mName); - if (found == self.mCoros.end()) - { - LL_ERRS("LLCoros") << "Coroutine '" << cdptr->mName << "' terminated " - << "without being stored in LLCoros::mCoros" - << LL_ENDL; - } - - // Oh good, we found the mCoros entry. Erase it. Because it's a ptr_map, - // that will implicitly delete this CoroData. - self.mCoros.erase(found); -} diff --git a/indra/llcommon/llcoros.h b/indra/llcommon/llcoros.h index de7b691284..7b3420cc8f 100644 --- a/indra/llcommon/llcoros.h +++ b/indra/llcommon/llcoros.h @@ -34,7 +34,7 @@ #include #include #include "llsingleton.h" -#include +#include "llinstancetracker.h" #include #include @@ -269,7 +269,7 @@ private: S32 mStackSize; // coroutine-local storage, as it were: one per coro we track - struct CoroData + struct CoroData: public LLInstanceTracker { CoroData(const std::string& name); @@ -281,18 +281,11 @@ private: std::string mStatus; F64 mCreationTime; // since epoch }; - typedef boost::ptr_map CoroMap; - CoroMap mCoros; // Identify the current coroutine's CoroData. This local_ptr isn't static // because it's a member of an LLSingleton, and we rely on it being // cleaned up in proper dependency order. - // As each coroutine terminates, use our custom cleanup function to remove - // the corresponding entry from mCoros. - local_ptr mCurrent{delete_CoroData}; - - // Cleanup function for each fiber's instance of mCurrent. - static void delete_CoroData(CoroData* cdptr); + local_ptr mCurrent; }; namespace llcoro diff --git a/indra/llcommon/llinstancetracker.h b/indra/llcommon/llinstancetracker.h index 363d0bcbd5..4c7a58bf25 100644 --- a/indra/llcommon/llinstancetracker.h +++ b/indra/llcommon/llinstancetracker.h @@ -207,6 +207,9 @@ public: return instance_iter(getMap_().end()); } + // while iterating over instances, might want to request the key + virtual const KEY& getKey() const { return mInstanceKey; } + static S32 instanceCount() { return getMap_().size(); @@ -235,7 +238,6 @@ protected: remove_(); } virtual void setKey(KEY key) { remove_(); add_(key); } - virtual const KEY& getKey() const { return mInstanceKey; } private: LLInstanceTracker( const LLInstanceTracker& ); diff --git a/indra/newview/llvoicevivox.cpp b/indra/newview/llvoicevivox.cpp index 2ec7d256e3..2975d1678a 100644 --- a/indra/newview/llvoicevivox.cpp +++ b/indra/newview/llvoicevivox.cpp @@ -390,7 +390,7 @@ void LLVivoxVoiceClient::init(LLPumpIO *pump) // constructor will set up LLVoiceClient::getInstance() LLVivoxVoiceClient::getInstance()->mPump = pump; -// LLCoros::instance().launch("LLVivoxVoiceClient::voiceControlCoro();", +// LLCoros::instance().launch("LLVivoxVoiceClient::voiceControlCoro", // boost::bind(&LLVivoxVoiceClient::voiceControlCoro, LLVivoxVoiceClient::getInstance())); } @@ -2523,7 +2523,7 @@ void LLVivoxVoiceClient::tuningStart() mTuningMode = true; if (!mIsCoroutineActive) { - LLCoros::instance().launch("LLVivoxVoiceClient::voiceControlCoro();", + LLCoros::instance().launch("LLVivoxVoiceClient::voiceControlCoro", boost::bind(&LLVivoxVoiceClient::voiceControlCoro, LLVivoxVoiceClient::getInstance())); } else if (mIsInChannel) @@ -5118,7 +5118,7 @@ void LLVivoxVoiceClient::setVoiceEnabled(bool enabled) if (!mIsCoroutineActive) { - LLCoros::instance().launch("LLVivoxVoiceClient::voiceControlCoro();", + LLCoros::instance().launch("LLVivoxVoiceClient::voiceControlCoro", boost::bind(&LLVivoxVoiceClient::voiceControlCoro, LLVivoxVoiceClient::getInstance())); } else From 088b7690946670e42d3736790dd5a5682890d5f5 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 25 Oct 2019 16:10:56 -0400 Subject: [PATCH 29/38] DRTVWR-476: Use shared_ptr to manage lifespan of coprocedure queue. Since the consuming coroutine LLCoprocedurePool::coprocedureInvokerCoro() has been observed to outlive the LLCoprocedurePool instance that owns the CoprocQueue_t, closing that queue isn't enough to keep the coroutine from crashing at shutdown: accessing a deleted CoprocQueue_t is fatal whether or not it's been closed. Make LLCoprocedurePool store a shared_ptr to a heap CoprocQueue_t instance, and pass that shared_ptr by value to consuming coroutines. That way the CoprocQueue_t instance is guaranteed to live as long as the last interested party. --- indra/llmessage/llcoproceduremanager.cpp | 33 +++++++++++++++--------- indra/llmessage/llcoproceduremanager.h | 1 + 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/indra/llmessage/llcoproceduremanager.cpp b/indra/llmessage/llcoproceduremanager.cpp index a8f6b8aa67..0b161ad2b4 100644 --- a/indra/llmessage/llcoproceduremanager.cpp +++ b/indra/llmessage/llcoproceduremanager.cpp @@ -94,13 +94,17 @@ private: // we use a buffered_channel here rather than unbuffered_channel since we want to be able to // push values without blocking,even if there's currently no one calling a pop operation (due to - // fibber running right now) + // fiber running right now) typedef boost::fibers::buffered_channel CoprocQueue_t; + // Use shared_ptr to control the lifespan of our CoprocQueue_t instance + // because the consuming coroutine might outlive this LLCoprocedurePool + // instance. + typedef boost::shared_ptr CoprocQueuePtr; typedef std::map ActiveCoproc_t; std::string mPoolName; size_t mPoolSize; - CoprocQueue_t mPendingCoprocs; + CoprocQueuePtr mPendingCoprocs; ActiveCoproc_t mActiveCoprocs; LLTempBoundListener mStatusListener; @@ -109,7 +113,8 @@ private: CoroAdapterMap_t mCoroMapping; - void coprocedureInvokerCoro(LLCoreHttpUtil::HttpCoroutineAdapter::ptr_t httpAdapter); + void coprocedureInvokerCoro(CoprocQueuePtr pendingCoprocs, + LLCoreHttpUtil::HttpCoroutineAdapter::ptr_t httpAdapter); }; //========================================================================= @@ -214,7 +219,7 @@ void LLCoprocedureManager::close(const std::string &pool) LLCoprocedurePool::LLCoprocedurePool(const std::string &poolName, size_t size): mPoolName(poolName), mPoolSize(size), - mPendingCoprocs(DEFAULT_QUEUE_SIZE), + mPendingCoprocs(boost::make_shared(DEFAULT_QUEUE_SIZE)), mHTTPPolicy(LLCore::HttpRequest::DEFAULT_POLICY_ID), mCoroMapping() { @@ -222,7 +227,7 @@ LLCoprocedurePool::LLCoprocedurePool(const std::string &poolName, size_t size): // destroyed, we implicitly disconnect from this LLEventPump mStatusListener = LLEventPumps::instance().obtain("LLApp").listen( poolName, - [this, poolName](const LLSD& status) + [pendingCoprocs=mPendingCoprocs, poolName](const LLSD& status) { auto& statsd = status["status"]; if (statsd.asString() != "running") @@ -232,7 +237,7 @@ LLCoprocedurePool::LLCoprocedurePool(const std::string &poolName, size_t size): << LL_ENDL; // This should ensure that all waiting coprocedures in this // pool will wake up and terminate. - mPendingCoprocs.close(); + pendingCoprocs->close(); } return false; }); @@ -241,8 +246,10 @@ LLCoprocedurePool::LLCoprocedurePool(const std::string &poolName, size_t size): { LLCoreHttpUtil::HttpCoroutineAdapter::ptr_t httpAdapter(new LLCoreHttpUtil::HttpCoroutineAdapter( mPoolName + "Adapter", mHTTPPolicy)); - std::string pooledCoro = LLCoros::instance().launch("LLCoprocedurePool("+mPoolName+")::coprocedureInvokerCoro", - boost::bind(&LLCoprocedurePool::coprocedureInvokerCoro, this, httpAdapter)); + std::string pooledCoro = LLCoros::instance().launch( + "LLCoprocedurePool("+mPoolName+")::coprocedureInvokerCoro", + boost::bind(&LLCoprocedurePool::coprocedureInvokerCoro, this, + mPendingCoprocs, httpAdapter)); mCoroMapping.insert(CoroAdapterMap_t::value_type(pooledCoro, httpAdapter)); } @@ -259,14 +266,16 @@ LLUUID LLCoprocedurePool::enqueueCoprocedure(const std::string &name, LLCoproced { LLUUID id(LLUUID::generateNewID()); - mPendingCoprocs.push(QueuedCoproc::ptr_t(new QueuedCoproc(name, id, proc))); + mPendingCoprocs->push(QueuedCoproc::ptr_t(new QueuedCoproc(name, id, proc))); LL_INFOS("CoProcMgr") << "Coprocedure(" << name << ") enqueued with id=" << id.asString() << " in pool \"" << mPoolName << "\"" << LL_ENDL; return id; } //------------------------------------------------------------------------- -void LLCoprocedurePool::coprocedureInvokerCoro(LLCoreHttpUtil::HttpCoroutineAdapter::ptr_t httpAdapter) +void LLCoprocedurePool::coprocedureInvokerCoro( + CoprocQueuePtr pendingCoprocs, + LLCoreHttpUtil::HttpCoroutineAdapter::ptr_t httpAdapter) { QueuedCoproc::ptr_t coproc; boost::fibers::channel_op_status status; @@ -274,7 +283,7 @@ void LLCoprocedurePool::coprocedureInvokerCoro(LLCoreHttpUtil::HttpCoroutineAdap { { LLCoros::TempStatus st("waiting for work for 10s"); - status = mPendingCoprocs.pop_wait_for(coproc, std::chrono::seconds(10)); + status = pendingCoprocs->pop_wait_for(coproc, std::chrono::seconds(10)); } if (status == boost::fibers::channel_op_status::closed) { @@ -313,5 +322,5 @@ void LLCoprocedurePool::coprocedureInvokerCoro(LLCoreHttpUtil::HttpCoroutineAdap void LLCoprocedurePool::close() { - mPendingCoprocs.close(); + mPendingCoprocs->close(); } diff --git a/indra/llmessage/llcoproceduremanager.h b/indra/llmessage/llcoproceduremanager.h index 299ec6742f..c9b853fc4c 100644 --- a/indra/llmessage/llcoproceduremanager.h +++ b/indra/llmessage/llcoproceduremanager.h @@ -32,6 +32,7 @@ #include "llcoros.h" #include "llcorehttputil.h" #include "lluuid.h" +#include class LLCoprocedurePool; From c3e4dfdda8f5bb774a9043f40837af30faf28023 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 28 Oct 2019 14:21:27 -0400 Subject: [PATCH 30/38] DRTVWR-476: Add LLUniqueFile, adding RAII semantics to LLFILE*. LLUniqueFile wraps an LLFILE* in a move-only class that closes the wrapped LLFILE* on destruction. It provides conversion operators to permit idiomatic usage as an LLFILE* value. --- indra/llcommon/llfile.h | 63 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/indra/llcommon/llfile.h b/indra/llcommon/llfile.h index 398938b729..9de095b45d 100644 --- a/indra/llcommon/llfile.h +++ b/indra/llcommon/llfile.h @@ -86,6 +86,69 @@ public: static const char * tmpdir(); }; +/// RAII class +class LLUniqueFile +{ +public: + // empty + LLUniqueFile(): mFileHandle(nullptr) {} + // wrap (e.g.) result of LLFile::fopen() + LLUniqueFile(LLFILE* f): mFileHandle(f) {} + // no copy + LLUniqueFile(const LLUniqueFile&) = delete; + // move construction + LLUniqueFile(LLUniqueFile&& other) + { + mFileHandle = other.mFileHandle; + other.mFileHandle = nullptr; + } + // The point of LLUniqueFile is to close on destruction. + ~LLUniqueFile() + { + close(); + } + + // simple assignment + LLUniqueFile& operator=(LLFILE* f) + { + close(); + mFileHandle = f; + return *this; + } + // copy assignment deleted + LLUniqueFile& operator=(const LLUniqueFile&) = delete; + // move assignment + LLUniqueFile& operator=(LLUniqueFile&& other) + { + close(); + std::swap(mFileHandle, other.mFileHandle); + return *this; + } + + // explicit close operation + void close() + { + if (mFileHandle) + { + // in case close() throws, set mFileHandle null FIRST + LLFILE* h{nullptr}; + std::swap(h, mFileHandle); + LLFile::close(h); + } + } + + // detect whether the wrapped LLFILE is open or not + explicit operator bool() const { return bool(mFileHandle); } + bool operator!() { return ! mFileHandle; } + + // LLUniqueFile should be usable for any operation that accepts LLFILE* + // (or FILE* for that matter) + operator LLFILE*() const { return mFileHandle; } + +private: + LLFILE* mFileHandle; +}; + #if LL_WINDOWS /** * @brief Controlling input for files. From 686e452659b6eb6399a48acd5d4733e7e2aece7e Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 28 Oct 2019 14:33:36 -0400 Subject: [PATCH 31/38] DRTVWR-476: Try to log stderr output from classic-C libraries. Some of the libraries we use produce log output to stderr. Such output can be informative, but is invisible unless you launch the viewer from a console. In particular, it's invisible to anyone trying to diagnose a problem by reading someone else's SecondLife.log file. Make RecordToFile -- the Recorder subclass engaged by LLError::logToFile() -- redirect STDERR_FILENO to the newly-opened log file so that any subsequent writes to stderr (or cerr, for that matter) will be captured in the log file. But first duplicate the original stderr file handle, and restore it when RecordToFile is destroyed. That way, output written to stderr during the final moments of application shutdown should still appear on (console) stderr. --- indra/llcommon/llerror.cpp | 68 +++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 30 deletions(-) diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 2e8e18b450..770e141aab 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -119,59 +119,67 @@ namespace { { public: RecordToFile(const std::string& filename): - mName(filename) + mName(filename), + mFile(LLFile::fopen(filename, "a")) { - mFile.open(filename.c_str(), std::ios_base::out | std::ios_base::app); if (!mFile) { - LL_INFOS() << "Error setting log file to " << filename << LL_ENDL; + LL_WARNS() << "Error setting log file to " << filename << LL_ENDL; } else { - if (!LLError::getAlwaysFlush()) - { - mFile.sync_with_stdio(false); - } +#if LL_DARWIN || LL_LINUX + // We use a number of classic-C libraries, some of which write + // log output to stderr. The trouble with that is that unless + // you launch the viewer from a console, stderr output is + // lost. Redirect STDERR_FILENO to write into this log file. + // But first, save the original stream in case we want it later. + mSavedStderr = ::dup(STDERR_FILENO); + ::dup2(::fileno(mFile), STDERR_FILENO); +#endif } } ~RecordToFile() { +#if LL_DARWIN || LL_LINUX + // restore stderr to its original fileno so any subsequent output + // to stderr goes to original stream + ::dup2(mSavedStderr, STDERR_FILENO); +#endif mFile.close(); } - virtual bool enabled() override - { + virtual bool enabled() override + { #ifdef LL_RELEASE_FOR_DOWNLOAD - return 1; + return 1; #else - return LLError::getEnabledLogTypesMask() & 0x02; + return LLError::getEnabledLogTypesMask() & 0x02; #endif - } - - bool okay() const { return mFile.good(); } + } - std::string getFilename() const { return mName; } + bool okay() const { return bool(mFile); } - virtual void recordMessage(LLError::ELevel level, - const std::string& message) override - { - if (LLError::getAlwaysFlush()) - { - mFile << message << std::endl; - } - else - { - mFile << message << "\n"; - } - } + std::string getFilename() const { return mName; } + + virtual void recordMessage(LLError::ELevel level, + const std::string& message) override + { + fwrite(message.c_str(), sizeof(char), message.length(), mFile); + if (LLError::getAlwaysFlush()) + { + ::fflush(mFile); + } + } private: const std::string mName; - llofstream mFile; + LLUniqueFile mFile; + int mSavedStderr{0}; }; - - + + class RecordToStderr : public LLError::Recorder { public: From 158ab272d78f18a6ee1526dbb2ef28ff45023a72 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 28 Oct 2019 14:40:20 -0400 Subject: [PATCH 32/38] DRTVWR-476: Add diagnostic output to llviewerjoystick.cpp. Add ndof_dump_list() call, to enumerate available devices, when ndof_init_first() returns failure. Add "joystick" tags to existing LL_INFOS() (etc.) calls in llviewerjoystick.cpp to make it easier to enable and disable such log messages. Add a specialized operator<<() function to log the contents of an NDOF_Device struct. Add a couple LL_DEBUGS() calls for more visibility into library operations. --- indra/newview/llviewerjoystick.cpp | 69 ++++++++++++++++++++++++------ 1 file changed, 55 insertions(+), 14 deletions(-) diff --git a/indra/newview/llviewerjoystick.cpp b/indra/newview/llviewerjoystick.cpp index e44d80b7ce..320bd04fe8 100644 --- a/indra/newview/llviewerjoystick.cpp +++ b/indra/newview/llviewerjoystick.cpp @@ -62,6 +62,43 @@ F32 LLViewerJoystick::sDelta[] = {0,0,0,0,0,0,0}; #define MAX_SPACENAVIGATOR_INPUT 3000.0f #define MAX_JOYSTICK_INPUT_VALUE MAX_SPACENAVIGATOR_INPUT +#if LIB_NDOF +std::ostream& operator<<(std::ostream& out, NDOF_Device* ptr) +{ + if (! ptr) + { + return out << "nullptr"; + } + out << "NDOF_Device{ "; + out << "axes ["; + const char* delim = ""; + for (short axis = 0; axis < ptr->axes_count; ++axis) + { + out << delim << ptr->axes[axis]; + delim = ", "; + } + out << "]"; + out << ", buttons ["; + delim = ""; + for (short button = 0; button < ptr->btn_count; ++button) + { + out << delim << ptr->buttons[button]; + delim = ", "; + } + out << "]"; + out << ", range " << ptr->axes_min << ':' << ptr->axes_max; + // If we don't coerce these to unsigned, they're streamed as characters, + // e.g. ctrl-A or nul. + out << ", absolute " << unsigned(ptr->absolute); + out << ", valid " << unsigned(ptr->valid); + out << ", manufacturer '" << ptr->manufacturer << "'"; + out << ", product '" << ptr->product << "'"; + out << ", private " << ptr->private_data; + out << " }"; + return out; +} +#endif // LIB_NDOF + // ----------------------------------------------------------------------------- void LLViewerJoystick::updateEnabled(bool autoenable) { @@ -107,11 +144,11 @@ NDOF_HotPlugResult LLViewerJoystick::HotPlugAddCallback(NDOF_Device *dev) LLViewerJoystick* joystick(LLViewerJoystick::getInstance()); if (joystick->mDriverState == JDS_UNINITIALIZED) { - LL_INFOS() << "HotPlugAddCallback: will use device:" << LL_ENDL; + LL_INFOS("joystick") << "HotPlugAddCallback: will use device:" << LL_ENDL; ndof_dump(dev); joystick->mNdofDev = dev; - joystick->mDriverState = JDS_INITIALIZED; - res = NDOF_KEEP_HOTPLUGGED; + joystick->mDriverState = JDS_INITIALIZED; + res = NDOF_KEEP_HOTPLUGGED; } joystick->updateEnabled(true); return res; @@ -125,7 +162,7 @@ void LLViewerJoystick::HotPlugRemovalCallback(NDOF_Device *dev) LLViewerJoystick* joystick(LLViewerJoystick::getInstance()); if (joystick->mNdofDev == dev) { - LL_INFOS() << "HotPlugRemovalCallback: joystick->mNdofDev=" + LL_INFOS("joystick") << "HotPlugRemovalCallback: joystick->mNdofDev=" << joystick->mNdofDev << "; removed device:" << LL_ENDL; ndof_dump(dev); joystick->mDriverState = JDS_UNINITIALIZED; @@ -193,6 +230,7 @@ void LLViewerJoystick::init(bool autoenable) { if (mNdofDev) { + LL_DEBUGS("joystick") << "ndof_create() returned: " << mNdofDev << LL_ENDL; // Different joysticks will return different ranges of raw values. // Since we want to handle every device in the same uniform way, // we initialize the mNdofDev struct and we set the range @@ -211,16 +249,19 @@ void LLViewerJoystick::init(bool autoenable) // just have the absolute values instead. mNdofDev->absolute = 1; + LL_DEBUGS("joystick") << "ndof_init_first() received: " << mNdofDev << LL_ENDL; // init & use the first suitable NDOF device found on the USB chain if (ndof_init_first(mNdofDev, NULL)) { mDriverState = JDS_UNINITIALIZED; - LL_WARNS() << "ndof_init_first FAILED" << LL_ENDL; + LL_WARNS("joystick") << "ndof_init_first FAILED" << LL_ENDL; + ndof_dump_list(); } else { mDriverState = JDS_INITIALIZED; } + LL_DEBUGS("joystick") << "ndof_init_first() left: " << mNdofDev << LL_ENDL; } else { @@ -258,8 +299,8 @@ void LLViewerJoystick::init(bool autoenable) { // No device connected, don't change any settings } - - LL_INFOS() << "ndof: mDriverState=" << mDriverState << "; mNdofDev=" + + LL_INFOS("joystick") << "ndof: mDriverState=" << mDriverState << "; mNdofDev=" << mNdofDev << "; libinit=" << libinit << LL_ENDL; #endif } @@ -270,7 +311,7 @@ void LLViewerJoystick::terminate() #if LIB_NDOF ndof_libcleanup(); - LL_INFOS() << "Terminated connection with NDOF device." << LL_ENDL; + LL_INFOS("joystick") << "Terminated connection with NDOF device." << LL_ENDL; mDriverState = JDS_UNINITIALIZED; #endif } @@ -1075,7 +1116,7 @@ std::string LLViewerJoystick::getDescription() bool LLViewerJoystick::isLikeSpaceNavigator() const { -#if LIB_NDOF +#if LIB_NDOF return (isJoystickInitialized() && (strncmp(mNdofDev->product, "SpaceNavigator", 14) == 0 || strncmp(mNdofDev->product, "SpaceExplorer", 13) == 0 @@ -1099,10 +1140,10 @@ void LLViewerJoystick::setSNDefaults() const float platformScaleAvXZ = 2.f; const bool is_3d_cursor = true; #endif - + //gViewerWindow->alertXml("CacheWillClear"); - LL_INFOS() << "restoring SpaceNavigator defaults..." << LL_ENDL; - + LL_INFOS("joystick") << "restoring SpaceNavigator defaults..." << LL_ENDL; + gSavedSettings.setS32("JoystickAxis0", 1); // z (at) gSavedSettings.setS32("JoystickAxis1", 0); // x (slide) gSavedSettings.setS32("JoystickAxis2", 2); // y (up) @@ -1110,11 +1151,11 @@ void LLViewerJoystick::setSNDefaults() gSavedSettings.setS32("JoystickAxis4", 3); // roll gSavedSettings.setS32("JoystickAxis5", 5); // yaw gSavedSettings.setS32("JoystickAxis6", -1); - + gSavedSettings.setBOOL("Cursor3D", is_3d_cursor); gSavedSettings.setBOOL("AutoLeveling", true); gSavedSettings.setBOOL("ZoomDirect", false); - + gSavedSettings.setF32("AvatarAxisScale0", 1.f * platformScaleAvXZ); gSavedSettings.setF32("AvatarAxisScale1", 1.f * platformScaleAvXZ); gSavedSettings.setF32("AvatarAxisScale2", 1.f); From e5bb19d2fa3ff49c243dd719e5985429a0d54e7f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 28 Oct 2019 17:15:40 -0400 Subject: [PATCH 33/38] DRTVWR-476: Try to extend stderr redirection to Windows as well. Make the LLError::Settings LLSingleton duplicate the file handle for stderr (usually 2) on construction. Make its destructor restore the original target for that file handle. Provide a getDupStderr() method to obtain the duplicate file handle. Move Settings declaration up to the top of the file so other code can reference it. Make RecordToFile (the Recorder subclass engaged by LLError::logToFile()), instead of duplicating stderr's file handle itself, capture the duplicate stderr file handle from Settings to revert stderr redirection on destruction. Make RecordToStderr (the Recorder subclass engaged by LLError::logToStderr()) use fdopen() to create an LLFILE* targeting the duplicate file handle from Settings. Write output to that instead of to stderr so logToStderr() continues to provide output for the user instead of duplicating each line into the log file. --- indra/llcommon/llerror.cpp | 118 +++++++++++++++++++++++++------------ 1 file changed, 79 insertions(+), 39 deletions(-) diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 770e141aab..0b1a9df8cb 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -53,6 +53,46 @@ #include "llstl.h" #include "lltimer.h" +#if LL_WINDOWS +#define fhclose _close +#define fhdup _dup +#define fhdup2 _dup2 +#define fhfdopen _fdopen +#define fhfileno _fileno +#else +#define fhclose ::close +#define fhdup ::dup +#define fhdup2 ::dup2 +#define fhfdopen ::fdopen +#define fhfileno ::fileno +#endif + +namespace LLError +{ + + class SettingsConfig; + typedef LLPointer SettingsConfigPtr; + + class Settings : public LLSingleton + { + LLSINGLETON(Settings); + public: + SettingsConfigPtr getSettingsConfig(); + ~Settings(); + + void reset(); + SettingsStoragePtr saveAndReset(); + void restore(SettingsStoragePtr pSettingsStorage); + + int getDupStderr() const; + + private: + SettingsConfigPtr mSettingsConfig; + int mDupStderr; + }; + +} // namespace LLError + namespace { #if LL_WINDOWS void debugger_print(const std::string& s) @@ -120,7 +160,8 @@ namespace { public: RecordToFile(const std::string& filename): mName(filename), - mFile(LLFile::fopen(filename, "a")) + mFile(LLFile::fopen(filename, "a")), + mSavedStderr(LLError::Settings::instance().getDupStderr()) { if (!mFile) { @@ -128,25 +169,19 @@ namespace { } else { -#if LL_DARWIN || LL_LINUX // We use a number of classic-C libraries, some of which write // log output to stderr. The trouble with that is that unless // you launch the viewer from a console, stderr output is // lost. Redirect STDERR_FILENO to write into this log file. - // But first, save the original stream in case we want it later. - mSavedStderr = ::dup(STDERR_FILENO); - ::dup2(::fileno(mFile), STDERR_FILENO); -#endif + fhdup2(fhfileno(mFile), fhfileno(stderr)); } } ~RecordToFile() { -#if LL_DARWIN || LL_LINUX // restore stderr to its original fileno so any subsequent output // to stderr goes to original stream - ::dup2(mSavedStderr, STDERR_FILENO); -#endif + fhdup2(mSavedStderr, fhfileno(stderr)); mFile.close(); } @@ -166,7 +201,8 @@ namespace { virtual void recordMessage(LLError::ELevel level, const std::string& message) override { - fwrite(message.c_str(), sizeof(char), message.length(), mFile); + ::fwrite(message.c_str(), sizeof(char), message.length(), mFile); + ::fputc('\n', mFile); if (LLError::getAlwaysFlush()) { ::fflush(mFile); @@ -176,22 +212,26 @@ namespace { private: const std::string mName; LLUniqueFile mFile; - int mSavedStderr{0}; + int mSavedStderr; }; class RecordToStderr : public LLError::Recorder { public: - RecordToStderr(bool timestamp) : mUseANSI(checkANSI()) + RecordToStderr(bool timestamp) : + mUseANSI(checkANSI()), + // use duplicate stderr file handle so THIS output isn't affected + // by our internal redirection of all (other) stderr output + mStderr(fhfdopen(LLError::Settings::instance().getDupStderr(), "a")) { - this->showMultiline(true); + this->showMultiline(true); + } + + virtual bool enabled() override + { + return LLError::getEnabledLogTypesMask() & 0x04; } - - virtual bool enabled() override - { - return LLError::getEnabledLogTypesMask() & 0x04; - } virtual void recordMessage(LLError::ELevel level, const std::string& message) override @@ -214,17 +254,18 @@ namespace { break; } } - fprintf(stderr, "%s\n", message.c_str()); + fprintf(mStderr, "%s\n", message.c_str()); if (mUseANSI) colorANSI("0"); // reset } - + private: bool mUseANSI; + LLFILE* mStderr; void colorANSI(const std::string color) { // ANSI color code escape sequence - fprintf(stderr, "\033[%sm", color.c_str() ); + fprintf(mStderr, "\033[%sm", color.c_str() ); }; static bool checkANSI(void) @@ -233,7 +274,7 @@ namespace { // Check whether it's okay to use ANSI; if stderr is // a tty then we assume yes. Can be turned off with // the LL_NO_ANSI_COLOR env var. - return (0 != isatty(2)) && + return (0 != isatty(fhfileno(stderr))) && (NULL == getenv("LL_NO_ANSI_COLOR")); #endif // LL_LINUX return false; @@ -497,22 +538,6 @@ namespace LLError SettingsConfig(); }; - typedef LLPointer SettingsConfigPtr; - - class Settings : public LLSingleton - { - LLSINGLETON(Settings); - public: - SettingsConfigPtr getSettingsConfig(); - - void reset(); - SettingsStoragePtr saveAndReset(); - void restore(SettingsStoragePtr pSettingsStorage); - - private: - SettingsConfigPtr mSettingsConfig; - }; - SettingsConfig::SettingsConfig() : LLRefCount(), mDefaultLevel(LLError::LEVEL_DEBUG), @@ -536,10 +561,20 @@ namespace LLError } Settings::Settings(): - mSettingsConfig(new SettingsConfig()) + mSettingsConfig(new SettingsConfig()), + // duplicate stderr file handle right away + mDupStderr(fhdup(fhfileno(stderr))) { } + Settings::~Settings() + { + // restore original stderr + fhdup2(mDupStderr, fhfileno(stderr)); + // and close the duplicate + fhclose(mDupStderr); + } + SettingsConfigPtr Settings::getSettingsConfig() { return mSettingsConfig; @@ -565,6 +600,11 @@ namespace LLError mSettingsConfig = newSettingsConfig; } + int Settings::getDupStderr() const + { + return mDupStderr; + } + bool is_available() { return Settings::instanceExists() && Globals::instanceExists(); From 75344c296e6842baf559e2b7cb21b6ed93d5f571 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 29 Oct 2019 07:32:10 -0400 Subject: [PATCH 34/38] DRTVWR-476: On Windows, dup2() et al. need --- indra/llcommon/llerror.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 0b1a9df8cb..4aab3b13ca 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -39,6 +39,8 @@ #if !LL_WINDOWS # include # include +#else +# include #endif // !LL_WINDOWS #include #include "string.h" From 1042f822ee57c13306634d4597be06678e1fcc20 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 30 Oct 2019 11:18:12 -0400 Subject: [PATCH 35/38] DRTVWR-476: Defend against late ~LLWatchdogTimeout() calls. LLAppViewer's heap LLWatchdogTimeout might be destroyed very late -- as late as in LLAppViewer's destructor. By that time, LLAppViewer::cleanup() has already called LLSingletonBase::deleteAll(), destroying the LLWatchdog LLSingleton instance. But LLWatchdogTimeout isa LLWatchdogEntry, and ~LLWatchdogEntry() calls stop(), and stop() tries to remove that instance from LLWatchdog, thus inadvertently resurrecting the deleted LLWatchdog. Which is pointless because the resurrected LLWatchdog has never heard of the LLWatchdogTimeout instance trying to remove itself. Defend LLWatchdogEntry::stop() against the case in which LLWatchdog has already been deleted. --- indra/newview/llwatchdog.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/indra/newview/llwatchdog.cpp b/indra/newview/llwatchdog.cpp index dd6c77ca7d..6273f10c69 100644 --- a/indra/newview/llwatchdog.cpp +++ b/indra/newview/llwatchdog.cpp @@ -91,7 +91,11 @@ void LLWatchdogEntry::start() void LLWatchdogEntry::stop() { - LLWatchdog::getInstance()->remove(this); + // this can happen very late in the shutdown sequence + if (! LLWatchdog::wasDeleted()) + { + LLWatchdog::getInstance()->remove(this); + } } // LLWatchdogTimeout From 66a2fe2e668fec0df9dcc92eef9c202f657fce3a Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 30 Oct 2019 12:38:42 -0400 Subject: [PATCH 36/38] DRTVWR-476: Update libndofdev to codeticket version 532324. --- autobuild.xml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/autobuild.xml b/autobuild.xml index 5189560509..76e55ad742 100644 --- a/autobuild.xml +++ b/autobuild.xml @@ -1892,9 +1892,9 @@ archive hash - aa2dcce6634256b3280813828e294b58 + 62b72bf45189b6c876947ac4e8209025 url - http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/44301/391550/libndofdev-0.1.531359-darwin64-531359.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/45899/405069/libndofdev-0.1.532324-darwin64-532324.tar.bz2 name darwin64 @@ -1904,9 +1904,9 @@ archive hash - b9dc910aaba2207d04718b31a84dc10f + 28df735378e7ba7154517ebc7bd53a69 url - http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/44307/391585/libndofdev-0.1.531359-windows-531359.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/45901/405083/libndofdev-0.1.532324-windows-532324.tar.bz2 name windows @@ -1916,16 +1916,16 @@ archive hash - fac91eace685540d467dc24016342cd4 + f397f1eb92f5ba75df7b69041156c944 url - http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/44305/391576/libndofdev-0.1.531359-windows64-531359.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/45900/405076/libndofdev-0.1.532324-windows64-532324.tar.bz2 name windows64 version - 0.1.531359 + 0.1.532324 libpng From 8afe593d9417ced188f6b5c8d95d2a271b751c14 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 30 Oct 2019 12:44:31 -0400 Subject: [PATCH 37/38] DRTVWR-476: Update llviewerjoystick.cpp for updated libndofdev API. --- indra/newview/llviewerjoystick.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/indra/newview/llviewerjoystick.cpp b/indra/newview/llviewerjoystick.cpp index 320bd04fe8..3d06c95080 100644 --- a/indra/newview/llviewerjoystick.cpp +++ b/indra/newview/llviewerjoystick.cpp @@ -145,7 +145,7 @@ NDOF_HotPlugResult LLViewerJoystick::HotPlugAddCallback(NDOF_Device *dev) if (joystick->mDriverState == JDS_UNINITIALIZED) { LL_INFOS("joystick") << "HotPlugAddCallback: will use device:" << LL_ENDL; - ndof_dump(dev); + ndof_dump(stderr, dev); joystick->mNdofDev = dev; joystick->mDriverState = JDS_INITIALIZED; res = NDOF_KEEP_HOTPLUGGED; @@ -164,7 +164,7 @@ void LLViewerJoystick::HotPlugRemovalCallback(NDOF_Device *dev) { LL_INFOS("joystick") << "HotPlugRemovalCallback: joystick->mNdofDev=" << joystick->mNdofDev << "; removed device:" << LL_ENDL; - ndof_dump(dev); + ndof_dump(stderr, dev); joystick->mDriverState = JDS_UNINITIALIZED; } joystick->updateEnabled(true); @@ -255,7 +255,7 @@ void LLViewerJoystick::init(bool autoenable) { mDriverState = JDS_UNINITIALIZED; LL_WARNS("joystick") << "ndof_init_first FAILED" << LL_ENDL; - ndof_dump_list(); + ndof_dump_list(stderr); } else { From 05e7a79cb401c0c4390e0ad3ad45a34a8c0ecd1e Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 30 Oct 2019 20:02:28 -0400 Subject: [PATCH 38/38] DRTVWR-476: Update to slvoice build 532334 --- autobuild.xml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/autobuild.xml b/autobuild.xml index beb96a1612..2b672a344a 100644 --- a/autobuild.xml +++ b/autobuild.xml @@ -2952,9 +2952,9 @@ Copyright (c) 2012, 2014, 2015, 2016 nghttp2 contributors archive hash - 924485833711af852b14517163c2c1c1 + d1c28eef1aa72d0aaa227e5122b43659 url - http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/44299/391536/slvoice-4.9.0002.27586.531358-darwin64-531358.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/45922/405171/slvoice-4.10.0000.32327.5fc3fe7c.532334-darwin64-532334.tar.bz2 name darwin64 @@ -2988,9 +2988,9 @@ Copyright (c) 2012, 2014, 2015, 2016 nghttp2 contributors archive hash - d5e155d066ce8506ee3574da66cb90bc + fcb26f570187eff2c724c3272581628f url - http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/44303/391564/slvoice-4.9.0002.27586.531358-windows-531358.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/45927/405216/slvoice-4.10.0000.32327.5fc3fe7c.532334-windows-532334.tar.bz2 name windows @@ -3000,16 +3000,16 @@ Copyright (c) 2012, 2014, 2015, 2016 nghttp2 contributors archive hash - 65f004cf214e4c4152864118bda64112 + cf6800e2221ff54d8af1de23b10f2797 url - http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/44300/391543/slvoice-4.9.0002.27586.531358-windows64-531358.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/45926/405209/slvoice-4.10.0000.32327.5fc3fe7c.532334-windows64-532334.tar.bz2 name windows64 version - 4.9.0002.27586.531358 + 4.10.0000.32327.5fc3fe7c.532334 tut