diff --git a/autobuild.xml b/autobuild.xml index 9a09569284..02b4057dfc 100644 --- a/autobuild.xml +++ b/autobuild.xml @@ -464,9 +464,9 @@ archive hash - 5f210778a1fd7bcbc286ea1eec09f5d9 + 1e7a892d237096c0c252dc26bb0bcf1d url - http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/44384/392137/boost-1.67-darwin64-531381.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/48114/427588/boost-1.67-darwin64-533856.tar.bz2 name darwin64 @@ -500,9 +500,9 @@ archive hash - 59add8c74a7955917c88a25ebc84b568 + 3fdcf88309a340c5e0b6db7be5182c1a url - http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/44393/392045/boost-1.67-windows-531381.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/48105/427355/boost-1.67-windows-533856.tar.bz2 name windows @@ -512,9 +512,9 @@ archive hash - 9a6d1801c88e18f42a2228c21043800f + ba2bd6b732d32b6feff6c329cff67041 url - http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/44389/392031/boost-1.67-windows64-531381.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/48116/427536/boost-1.67-windows64-533856.tar.bz2 name windows64 @@ -606,9 +606,9 @@ archive hash - 1a731435cb559831ab418d5f60e346cd + ca6bb7108b8cf4bac4c4acac515b9b5f url - http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/44416/392216/colladadom-2.3.531391-darwin64-531391.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/48135/427607/colladadom-2.3.533872-darwin64-533872.tar.bz2 name darwin64 @@ -642,9 +642,9 @@ archive hash - 1076600dc11b5c36dddcd80c2b291530 + 2a0161276047081ac27e2b5c260e61d8 url - http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/44417/392222/colladadom-2.3.531391-windows-531391.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/48138/427646/colladadom-2.3.533872-windows-533872.tar.bz2 name windows @@ -654,16 +654,16 @@ archive hash - 5977db3bd9b5c02b4337096de92fb598 + b0d938c122f1e3a2ab5fdbb33bae165c url - http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/44414/392209/colladadom-2.3.531391-windows64-531391.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/48134/427621/colladadom-2.3.533872-windows64-533872.tar.bz2 name windows64 version - 2.3.531391 + 2.3.533872 curl @@ -1656,9 +1656,9 @@ archive hash - eaba8da53b483e72ea8237cde78e26da + cfa0065ee16facb7e4372bd9be834124 url - http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/44415/392202/googlemock-1.7.0.531390-darwin64-531390.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/48136/427614/googlemock-1.7.0.533873-darwin64-533873.tar.bz2 name darwin64 @@ -1692,9 +1692,9 @@ archive hash - 6013840ad4d6c8a2e68fde2403e0800b + f36799013f5f5dcc07526b55902f2188 url - http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/44412/392188/googlemock-1.7.0.531390-windows-531390.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/48144/427663/googlemock-1.7.0.533873-windows-533873.tar.bz2 name windows @@ -1704,16 +1704,16 @@ archive hash - 7e57a4f153e0f2cbcea097652b37602c + 9722f105a90eddb6142aef6c7b42eb18 url - http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/44409/392173/googlemock-1.7.0.531390-windows64-531390.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/48141/427655/googlemock-1.7.0.533873-windows64-533873.tar.bz2 name windows64 version - 1.7.0.531390 + 1.7.0.533873 gstreamer @@ -3558,9 +3558,9 @@ Copyright (c) 2012, 2014, 2015, 2016 nghttp2 contributors archive hash - 9b341f9a6f36eb595ef2ea88425add21 + 1ba3fb47e32eda07b9308babb278ee69 url - http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/47032/414972/viewer_manager-2.0.533067-darwin64-533067.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/48017/426433/viewer_manager-2.0.533794-darwin64-533794.tar.bz2 name darwin64 @@ -3594,9 +3594,9 @@ Copyright (c) 2012, 2014, 2015, 2016 nghttp2 contributors archive hash - ef076d8d018c55e25124b7a577bae132 + 65bd60bf3fe8a9d6f8efc230dcf3bd6b url - http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/47034/414989/viewer_manager-2.0.533067-windows-533067.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/ct2/48018/426440/viewer_manager-2.0.533794-windows-533794.tar.bz2 name windows @@ -3607,7 +3607,7 @@ Copyright (c) 2012, 2014, 2015, 2016 nghttp2 contributors source_type hg version - 2.0.533067 + 2.0.533794 vlc-bin diff --git a/indra/llappearance/llwearabletype.cpp b/indra/llappearance/llwearabletype.cpp index 136786c109..8cd076f7a8 100644 --- a/indra/llappearance/llwearabletype.cpp +++ b/indra/llappearance/llwearabletype.cpp @@ -32,7 +32,8 @@ struct WearableEntry : public LLDictionaryEntry { - WearableEntry(const std::string &name, + WearableEntry(LLWearableType& wtype, + const std::string &name, const std::string& default_new_name, LLAssetType::EType assetType, LLInventoryType::EIconName iconName, @@ -41,7 +42,7 @@ struct WearableEntry : public LLDictionaryEntry LLDictionaryEntry(name), mAssetType(assetType), mDefaultNewName(default_new_name), - mLabel(LLWearableType::getInstance()->mTrans->getString(name)), + mLabel(wtype.mTrans->getString(name)), mIconName(iconName), mDisableCameraSwitch(disable_camera_switch), mAllowMultiwear(allow_multiwear) @@ -56,10 +57,10 @@ struct WearableEntry : public LLDictionaryEntry BOOL mAllowMultiwear; }; -class LLWearableDictionary : public LLSingleton, +class LLWearableDictionary : public LLParamSingleton, public LLDictionary { - LLSINGLETON(LLWearableDictionary); + LLSINGLETON(LLWearableDictionary, LLWearableType&); // [RLVa:KB] - Checked: 2010-03-03 (RLVa-1.2.0a) | Added: RLVa-1.2.0a protected: @@ -68,7 +69,7 @@ protected: // [/RLVa:KB] }; -LLWearableDictionary::LLWearableDictionary() +LLWearableDictionary::LLWearableDictionary(LLWearableType& wtype) { if (!LLWearableType::instanceExists()) { @@ -76,30 +77,30 @@ LLWearableDictionary::LLWearableDictionary() // Todo: consider merging LLWearableType and LLWearableDictionary LL_WARNS() << "Initing LLWearableDictionary without LLWearableType" << LL_ENDL; } - addEntry(LLWearableType::WT_SHAPE, new WearableEntry("shape", "New Shape", LLAssetType::AT_BODYPART, LLInventoryType::ICONNAME_BODYPART_SHAPE, FALSE, FALSE)); - addEntry(LLWearableType::WT_SKIN, new WearableEntry("skin", "New Skin", LLAssetType::AT_BODYPART, LLInventoryType::ICONNAME_BODYPART_SKIN, FALSE, FALSE)); - addEntry(LLWearableType::WT_HAIR, new WearableEntry("hair", "New Hair", LLAssetType::AT_BODYPART, LLInventoryType::ICONNAME_BODYPART_HAIR, FALSE, FALSE)); - addEntry(LLWearableType::WT_EYES, new WearableEntry("eyes", "New Eyes", LLAssetType::AT_BODYPART, LLInventoryType::ICONNAME_BODYPART_EYES, FALSE, FALSE)); - addEntry(LLWearableType::WT_SHIRT, new WearableEntry("shirt", "New Shirt", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_SHIRT, FALSE, TRUE)); - addEntry(LLWearableType::WT_PANTS, new WearableEntry("pants", "New Pants", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_PANTS, FALSE, TRUE)); - addEntry(LLWearableType::WT_SHOES, new WearableEntry("shoes", "New Shoes", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_SHOES, FALSE, TRUE)); - addEntry(LLWearableType::WT_SOCKS, new WearableEntry("socks", "New Socks", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_SOCKS, FALSE, TRUE)); - addEntry(LLWearableType::WT_JACKET, new WearableEntry("jacket", "New Jacket", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_JACKET, FALSE, TRUE)); - addEntry(LLWearableType::WT_GLOVES, new WearableEntry("gloves", "New Gloves", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_GLOVES, FALSE, TRUE)); - addEntry(LLWearableType::WT_UNDERSHIRT, new WearableEntry("undershirt", "New Undershirt", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_UNDERSHIRT, FALSE, TRUE)); - addEntry(LLWearableType::WT_UNDERPANTS, new WearableEntry("underpants", "New Underpants", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_UNDERPANTS, FALSE, TRUE)); - addEntry(LLWearableType::WT_SKIRT, new WearableEntry("skirt", "New Skirt", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_SKIRT, FALSE, TRUE)); - addEntry(LLWearableType::WT_ALPHA, new WearableEntry("alpha", "New Alpha", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_ALPHA, FALSE, TRUE)); - addEntry(LLWearableType::WT_TATTOO, new WearableEntry("tattoo", "New Tattoo", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_TATTOO, FALSE, TRUE)); - addEntry(LLWearableType::WT_UNIVERSAL, new WearableEntry("universal", "New Universal", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_UNIVERSAL, FALSE, TRUE)); + addEntry(LLWearableType::WT_SHAPE, new WearableEntry(wtype, "shape", "New Shape", LLAssetType::AT_BODYPART, LLInventoryType::ICONNAME_BODYPART_SHAPE, FALSE, FALSE)); + addEntry(LLWearableType::WT_SKIN, new WearableEntry(wtype, "skin", "New Skin", LLAssetType::AT_BODYPART, LLInventoryType::ICONNAME_BODYPART_SKIN, FALSE, FALSE)); + addEntry(LLWearableType::WT_HAIR, new WearableEntry(wtype, "hair", "New Hair", LLAssetType::AT_BODYPART, LLInventoryType::ICONNAME_BODYPART_HAIR, FALSE, FALSE)); + addEntry(LLWearableType::WT_EYES, new WearableEntry(wtype, "eyes", "New Eyes", LLAssetType::AT_BODYPART, LLInventoryType::ICONNAME_BODYPART_EYES, FALSE, FALSE)); + addEntry(LLWearableType::WT_SHIRT, new WearableEntry(wtype, "shirt", "New Shirt", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_SHIRT, FALSE, TRUE)); + addEntry(LLWearableType::WT_PANTS, new WearableEntry(wtype, "pants", "New Pants", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_PANTS, FALSE, TRUE)); + addEntry(LLWearableType::WT_SHOES, new WearableEntry(wtype, "shoes", "New Shoes", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_SHOES, FALSE, TRUE)); + addEntry(LLWearableType::WT_SOCKS, new WearableEntry(wtype, "socks", "New Socks", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_SOCKS, FALSE, TRUE)); + addEntry(LLWearableType::WT_JACKET, new WearableEntry(wtype, "jacket", "New Jacket", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_JACKET, FALSE, TRUE)); + addEntry(LLWearableType::WT_GLOVES, new WearableEntry(wtype, "gloves", "New Gloves", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_GLOVES, FALSE, TRUE)); + addEntry(LLWearableType::WT_UNDERSHIRT, new WearableEntry(wtype, "undershirt", "New Undershirt", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_UNDERSHIRT, FALSE, TRUE)); + addEntry(LLWearableType::WT_UNDERPANTS, new WearableEntry(wtype, "underpants", "New Underpants", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_UNDERPANTS, FALSE, TRUE)); + addEntry(LLWearableType::WT_SKIRT, new WearableEntry(wtype, "skirt", "New Skirt", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_SKIRT, FALSE, TRUE)); + addEntry(LLWearableType::WT_ALPHA, new WearableEntry(wtype, "alpha", "New Alpha", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_ALPHA, FALSE, TRUE)); + addEntry(LLWearableType::WT_TATTOO, new WearableEntry(wtype, "tattoo", "New Tattoo", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_TATTOO, FALSE, TRUE)); + addEntry(LLWearableType::WT_UNIVERSAL, new WearableEntry(wtype, "universal", "New Universal", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_UNIVERSAL, FALSE, TRUE)); // [SL:KB] - Patch: Appearance-Misc | Checked: 2011-05-29 (Catznip-2.6) - addEntry(LLWearableType::WT_PHYSICS, new WearableEntry("physics", "New Physics", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_PHYSICS, TRUE, FALSE)); + addEntry(LLWearableType::WT_PHYSICS, new WearableEntry(wtype, "physics", "New Physics", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_PHYSICS, TRUE, FALSE)); // [/SL:KB] -// addEntry(LLWearableType::WT_PHYSICS, new WearableEntry("physics", "New Physics", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_PHYSICS, TRUE, TRUE)); +// addEntry(LLWearableType::WT_PHYSICS, new WearableEntry(wtype, "physics", "New Physics", LLAssetType::AT_CLOTHING, LLInventoryType::ICONNAME_CLOTHING_PHYSICS, TRUE, TRUE)); - addEntry(LLWearableType::WT_INVALID, new WearableEntry("invalid", "Invalid Wearable", LLAssetType::AT_NONE, LLInventoryType::ICONNAME_UNKNOWN, FALSE, FALSE)); - addEntry(LLWearableType::WT_NONE, new WearableEntry("none", "Invalid Wearable", LLAssetType::AT_NONE, LLInventoryType::ICONNAME_NONE, FALSE, FALSE)); + addEntry(LLWearableType::WT_INVALID, new WearableEntry(wtype, "invalid", "Invalid Wearable", LLAssetType::AT_NONE, LLInventoryType::ICONNAME_UNKNOWN, FALSE, FALSE)); + addEntry(LLWearableType::WT_NONE, new WearableEntry(wtype, "none", "Invalid Wearable", LLAssetType::AT_NONE, LLInventoryType::ICONNAME_NONE, FALSE, FALSE)); } @@ -116,6 +117,14 @@ LLWearableType::~LLWearableType() delete mTrans; } +void LLWearableType::initSingleton() +{ + // To make sure all wrapping functions will crash without initing LLWearableType; + LLWearableDictionary::initParamSingleton(*this); + + // Todo: consider merging LLWearableType and LLWearableDictionary +} + // static LLWearableType::EType LLWearableType::typeNameToType(const std::string& type_name) { diff --git a/indra/llappearance/llwearabletype.h b/indra/llappearance/llwearabletype.h index 5fe969822a..0396f1fa45 100644 --- a/indra/llappearance/llwearabletype.h +++ b/indra/llappearance/llwearabletype.h @@ -47,7 +47,8 @@ class LLWearableType : public LLParamSingleton LLSINGLETON(LLWearableType, LLTranslationBridge* trans); ~LLWearableType(); friend struct WearableEntry; -public: + void initSingleton(); +public: enum EType { WT_SHAPE = 0, diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index a5b4ce97ad..cd59780a0f 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -187,6 +187,7 @@ set(llcommon_HEADER_FILES llleaplistener.h llliveappconfig.h lllivefile.h + llmainthreadtask.h llmd5.h llmemory.h llmemorystream.h @@ -246,6 +247,7 @@ set(llcommon_HEADER_FILES llwin32headers.h llwin32headerslean.h llworkerthread.h + lockstatic.h stdtypes.h stringize.h timer.h @@ -374,6 +376,7 @@ if (LL_TESTS) LL_ADD_INTEGRATION_TEST(llheteromap "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llinstancetracker "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llleap "" "${test_libs}") + LL_ADD_INTEGRATION_TEST(llmainthreadtask "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llpounceable "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llprocess "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llprocessor "" "${test_libs}") diff --git a/indra/llcommon/llapr.h b/indra/llcommon/llapr.h index da92b36212..616dcb03d8 100644 --- a/indra/llcommon/llapr.h +++ b/indra/llcommon/llapr.h @@ -41,17 +41,7 @@ #include "llstring.h" -#if LL_WINDOWS -#pragma warning (push) -#pragma warning (disable:4265) -#endif -// warning C4265: 'std::_Pad' : class has virtual functions, but destructor is not virtual - -#include - -#if LL_WINDOWS -#pragma warning (pop) -#endif +#include "mutex.h" struct apr_dso_handle_t; /** diff --git a/indra/llcommon/llcoros.cpp b/indra/llcommon/llcoros.cpp index f881fe3672..2f1ca791ed 100644 --- a/indra/llcommon/llcoros.cpp +++ b/indra/llcommon/llcoros.cpp @@ -34,6 +34,7 @@ #include "llcoros.h" // STL headers // std headers +#include // external library headers #include #include @@ -73,10 +74,9 @@ LLCoros::CoroData& LLCoros::get_CoroData(const std::string& caller) // canonical values. if (! current) { - // 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 thread_local CoroData sMain(""); + static std::atomic which_thread(0); + // Use alternate CoroData constructor. + static thread_local CoroData sMain(which_thread++); // 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; @@ -197,6 +197,13 @@ std::string LLCoros::getName() return get_CoroData("getName()").mName; } +//static +std::string LLCoros::logname() +{ + LLCoros::CoroData& data(get_CoroData("logname()")); + return data.mName.empty()? data.getKey() : data.mName; +} + void LLCoros::setStackSize(S32 stacksize) { LL_DEBUGS("LLCoros") << "Setting coroutine stack size to " << stacksize << LL_ENDL; @@ -211,12 +218,11 @@ void LLCoros::printActiveCoroutines(const std::string& when) { LL_INFOS("LLCoros") << "-------------- List of active coroutines ------------"; F64 time = LLTimer::getTotalSeconds(); - for (auto it(CoroData::beginInstances()), end(CoroData::endInstances()); - it != end; ++it) + for (auto& cd : CoroData::instance_snapshot()) { - F64 life_time = time - it->mCreationTime; + F64 life_time = time - cd.mCreationTime; LL_CONT << LL_NEWLINE - << it->mName << ' ' << it->mStatus << " life: " << life_time; + << cd.getKey() << ' ' << cd.mStatus << " life: " << life_time; } LL_CONT << LL_ENDL; LL_INFOS("LLCoros") << "-----------------------------------------------------" << LL_ENDL; @@ -355,3 +361,16 @@ LLCoros::CoroData::CoroData(const std::string& name): mCreationTime(LLTimer::getTotalSeconds()) { } + +LLCoros::CoroData::CoroData(int n): + // This constructor is used for the thread_local instance belonging to the + // default coroutine on each thread. We must give each one a different + // LLInstanceTracker key because LLInstanceTracker's map spans all + // threads, but we want the default coroutine on each thread to have the + // empty string as its visible name because some consumers test for that. + LLInstanceTracker("main" + stringize(n)), + mName(), + mConsuming(false), + mCreationTime(LLTimer::getTotalSeconds()) +{ +} diff --git a/indra/llcommon/llcoros.h b/indra/llcommon/llcoros.h index 2e4cd8ccad..95859198d4 100644 --- a/indra/llcommon/llcoros.h +++ b/indra/llcommon/llcoros.h @@ -142,6 +142,13 @@ public: */ static std::string getName(); + /** + * This variation returns a name suitable for log messages: the explicit + * name for an explicitly-launched coroutine, or "mainN" for the default + * coroutine on a thread. + */ + static std::string logname(); + /** * For delayed initialization. To be clear, this will only affect * coroutines launched @em after this point. The underlying facility @@ -272,6 +279,7 @@ private: struct CoroData: public LLInstanceTracker { CoroData(const std::string& name); + CoroData(int n); // tweaked name of the current coroutine const std::string mName; @@ -292,12 +300,7 @@ namespace llcoro { inline -std::string logname() -{ - static std::string main("main"); - std::string name(LLCoros::getName()); - return name.empty()? main : name; -} +std::string logname() { return LLCoros::logname(); } } // llcoro diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index e5853b26aa..88999abf64 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -307,28 +307,35 @@ namespace LLError { #ifdef __GNUC__ // GCC: type_info::name() returns a mangled class name,st demangle - // passing nullptr, 0 forces allocation of a unique buffer we can free - // fixing MAINT-8724 on OSX 10.14 + // passing nullptr, 0 forces allocation of a unique buffer we can free + // fixing MAINT-8724 on OSX 10.14 int status = -1; char* name = abi::__cxa_demangle(mangled, nullptr, 0, &status); - std::string result(name ? name : mangled); - free(name); - return result; + std::string result(name ? name : mangled); + free(name); + return result; + #elif LL_WINDOWS - // DevStudio: type_info::name() includes the text "class " at the start - - static const std::string class_prefix = "class "; + // Visual Studio: type_info::name() includes the text "class " at the start std::string name = mangled; - if (0 != name.compare(0, class_prefix.length(), class_prefix)) + for (const auto& prefix : std::vector{ "class ", "struct " }) { - LL_DEBUGS() << "Did not see '" << class_prefix << "' prefix on '" - << name << "'" << LL_ENDL; - return name; + if (0 == name.compare(0, prefix.length(), prefix)) + { + return name.substr(prefix.length()); + } } + // huh, that's odd, we should see one or the other prefix -- but don't + // try to log unless logging is already initialized + if (is_available()) + { + // in Python, " or ".join(vector) -- but in C++, a PITB + LL_DEBUGS() << "Did not see 'class' or 'struct' prefix on '" + << name << "'" << LL_ENDL; + } + return name; - return name.substr(class_prefix.length()); - -#else +#else // neither GCC nor Visual Studio return mangled; #endif } @@ -1213,8 +1220,25 @@ namespace } namespace { - LLMutex gLogMutex; - LLMutex gCallStacksLogMutex; + // We need a couple different mutexes, but we want to use the same mechanism + // for both. Make getMutex() a template function with different instances + // for different MutexDiscriminator values. + enum MutexDiscriminator + { + LOG_MUTEX, + STACKS_MUTEX + }; + // Some logging calls happen very early in processing -- so early that our + // module-static variables aren't yet initialized. getMutex() wraps a + // function-static LLMutex so that early calls can still have a valid + // LLMutex instance. + template + LLMutex* getMutex() + { + // guaranteed to be initialized the first time control reaches here + static LLMutex sMutex; + return &sMutex; + } bool checkLevelMap(const LevelMap& map, const std::string& key, LLError::ELevel& level) @@ -1267,7 +1291,7 @@ namespace LLError bool Log::shouldLog(CallSite& site) { - LLMutexTrylock lock(&gLogMutex, 5); + LLMutexTrylock lock(getMutex(), 5); if (!lock.isLocked()) { return false; @@ -1318,7 +1342,7 @@ namespace LLError std::ostringstream* Log::out() { - LLMutexTrylock lock(&gLogMutex,5); + LLMutexTrylock lock(getMutex(),5); // If we hit a logging request very late during shutdown processing, // when either of the relevant LLSingletons has already been deleted, // DO NOT resurrect them. @@ -1338,7 +1362,7 @@ namespace LLError void Log::flush(std::ostringstream* out, char* message) { - LLMutexTrylock lock(&gLogMutex,5); + LLMutexTrylock lock(getMutex(),5); if (!lock.isLocked()) { return; @@ -1378,7 +1402,7 @@ namespace LLError void Log::flush(std::ostringstream* out, const CallSite& site) { - LLMutexTrylock lock(&gLogMutex,5); + LLMutexTrylock lock(getMutex(),5); if (!lock.isLocked()) { return; @@ -1549,34 +1573,34 @@ namespace LLError S32 LLCallStacks::sIndex = 0 ; //static - void LLCallStacks::allocateStackBuffer() - { - if(sBuffer == NULL) - { - sBuffer = new char*[512] ; - sBuffer[0] = new char[512 * 128] ; - for(S32 i = 1 ; i < 512 ; i++) - { - sBuffer[i] = sBuffer[i-1] + 128 ; - } - sIndex = 0 ; - } - } + void LLCallStacks::allocateStackBuffer() + { + if(sBuffer == NULL) + { + sBuffer = new char*[512] ; + sBuffer[0] = new char[512 * 128] ; + for(S32 i = 1 ; i < 512 ; i++) + { + sBuffer[i] = sBuffer[i-1] + 128 ; + } + sIndex = 0 ; + } + } - void LLCallStacks::freeStackBuffer() - { - if(sBuffer != NULL) - { - delete [] sBuffer[0] ; - delete [] sBuffer ; - sBuffer = NULL ; - } - } + void LLCallStacks::freeStackBuffer() + { + if(sBuffer != NULL) + { + delete [] sBuffer[0] ; + delete [] sBuffer ; + sBuffer = NULL ; + } + } - //static - void LLCallStacks::push(const char* function, const int line) - { - LLMutexTrylock lock(&gCallStacksLogMutex, 5); + //static + void LLCallStacks::push(const char* function, const int line) + { + LLMutexTrylock lock(getMutex(), 5); if (!lock.isLocked()) { return; @@ -1611,7 +1635,7 @@ namespace LLError //static void LLCallStacks::end(std::ostringstream* _out) { - LLMutexTrylock lock(&gCallStacksLogMutex, 5); + LLMutexTrylock lock(getMutex(), 5); if (!lock.isLocked()) { return; @@ -1633,7 +1657,7 @@ namespace LLError //static void LLCallStacks::print() { - LLMutexTrylock lock(&gCallStacksLogMutex, 5); + LLMutexTrylock lock(getMutex(), 5); if (!lock.isLocked()) { return; @@ -1676,7 +1700,7 @@ namespace LLError bool debugLoggingEnabled(const std::string& tag) { - LLMutexTrylock lock(&gLogMutex, 5); + LLMutexTrylock lock(getMutex(), 5); if (!lock.isLocked()) { return false; diff --git a/indra/llcommon/lleventtimer.cpp b/indra/llcommon/lleventtimer.cpp index 05419d452f..f575a7b6bf 100644 --- a/indra/llcommon/lleventtimer.cpp +++ b/indra/llcommon/lleventtimer.cpp @@ -57,35 +57,17 @@ LLEventTimer::~LLEventTimer() //static void LLEventTimer::updateClass() { - std::list completed_timers; - - // Minimize calls to getInstances per frame - // for (instance_iter iter = beginInstances(); iter != endInstances(); ) - - instance_iter end = endInstances(); - for (instance_iter iter = beginInstances(); iter != end; ) - // + for (auto& timer : instance_snapshot()) { - LLEventTimer& timer = *iter++; F32 et = timer.mEventTimer.getElapsedTimeF32(); if (timer.mEventTimer.getStarted() && et > timer.mPeriod) { timer.mEventTimer.reset(); if ( timer.tick() ) { - completed_timers.push_back( &timer ); + delete &timer; } } } - - if ( completed_timers.size() > 0 ) - { - for (std::list::iterator completed_iter = completed_timers.begin(); - completed_iter != completed_timers.end(); - completed_iter++ ) - { - delete *completed_iter; - } - } } diff --git a/indra/llcommon/llexception.cpp b/indra/llcommon/llexception.cpp index c0483e8927..5ce8958687 100644 --- a/indra/llcommon/llexception.cpp +++ b/indra/llcommon/llexception.cpp @@ -24,6 +24,11 @@ // `_GNU_SOURCE` macro or `BOOST_STACKTRACE_GNU_SOURCE_NOT_REQUIRED` if // _Unwind_Backtrace is available without `_GNU_SOURCE`." #define BOOST_STACKTRACE_GNU_SOURCE_NOT_REQUIRED +#if LL_WINDOWS +// On Windows, header-only implementation causes macro collisions -- use +// prebuilt library +#define BOOST_STACKTRACE_LINK +#endif // LL_WINDOWS #include // other Linden headers #include "llerror.h" diff --git a/indra/llcommon/llfasttimer.cpp b/indra/llcommon/llfasttimer.cpp index c60ffac5b9..ab93713b44 100644 --- a/indra/llcommon/llfasttimer.cpp +++ b/indra/llcommon/llfasttimer.cpp @@ -199,27 +199,26 @@ TimeBlockTreeNode& BlockTimerStatHandle::getTreeNode() const void BlockTimer::bootstrapTimerTree() { - for (BlockTimerStatHandle::instance_tracker_t::instance_iter it = BlockTimerStatHandle::instance_tracker_t::beginInstances(), end_it = BlockTimerStatHandle::instance_tracker_t::endInstances(); - it != end_it; - ++it) + for (auto& base : BlockTimerStatHandle::instance_snapshot()) { - BlockTimerStatHandle& timer = static_cast(*it); + // because of indirect derivation from LLInstanceTracker, have to downcast + BlockTimerStatHandle& timer = static_cast(base); if (&timer == &BlockTimer::getRootTimeBlock()) continue; // bootstrap tree construction by attaching to last timer to be on stack // when this timer was called if (timer.getParent() == &BlockTimer::getRootTimeBlock()) -{ + { TimeBlockAccumulator& accumulator = timer.getCurrentAccumulator(); if (accumulator.mLastCaller) - { + { timer.setParent(accumulator.mLastCaller); accumulator.mParent = accumulator.mLastCaller; - } + } // no need to push up tree on first use, flag can be set spuriously accumulator.mMoveUpTree = false; - } + } } } @@ -312,12 +311,10 @@ void BlockTimer::processTimes() updateTimes(); // reset for next frame - for (BlockTimerStatHandle::instance_tracker_t::instance_iter it = BlockTimerStatHandle::instance_tracker_t::beginInstances(), - end_it = BlockTimerStatHandle::instance_tracker_t::endInstances(); - it != end_it; - ++it) + for (auto& base : BlockTimerStatHandle::instance_snapshot()) { - BlockTimerStatHandle& timer = static_cast(*it); + // because of indirect derivation from LLInstanceTracker, have to downcast + BlockTimerStatHandle& timer = static_cast(base); TimeBlockAccumulator& accumulator = timer.getCurrentAccumulator(); accumulator.mLastCaller = NULL; @@ -368,12 +365,10 @@ void BlockTimer::logStats() LLSD sd; { - for (BlockTimerStatHandle::instance_tracker_t::instance_iter it = BlockTimerStatHandle::instance_tracker_t::beginInstances(), - end_it = BlockTimerStatHandle::instance_tracker_t::endInstances(); - it != end_it; - ++it) + for (auto& base : BlockTimerStatHandle::instance_snapshot()) { - BlockTimerStatHandle& timer = static_cast(*it); + // because of indirect derivation from LLInstanceTracker, have to downcast + BlockTimerStatHandle& timer = static_cast(base); LLTrace::PeriodicRecording& frame_recording = LLTrace::get_frame_recording(); sd[timer.getName()]["Time"] = (LLSD::Real) (frame_recording.getLastRecording().getSum(timer).value()); sd[timer.getName()]["Calls"] = (LLSD::Integer) (frame_recording.getLastRecording().getSum(timer.callCount())); diff --git a/indra/llcommon/llinstancetracker.cpp b/indra/llcommon/llinstancetracker.cpp index 3f990f4869..e7193b70b5 100644 --- a/indra/llcommon/llinstancetracker.cpp +++ b/indra/llcommon/llinstancetracker.cpp @@ -27,25 +27,15 @@ #include "linden_common.h" // associated header #include "llinstancetracker.h" -#include "llapr.h" - +#include "llerror.h" // STL headers // std headers // external library headers // other Linden headers -void LLInstanceTrackerBase::StaticBase::incrementDepth() +void LLInstanceTrackerPrivate::logerrs(const char* cls, const std::string& arg1, + const std::string& arg2, const std::string& arg3) { - ++sIterationNestDepth; -} - -void LLInstanceTrackerBase::StaticBase::decrementDepth() -{ - llassert(sIterationNestDepth); - --sIterationNestDepth; -} - -U32 LLInstanceTrackerBase::StaticBase::getDepth() -{ - return sIterationNestDepth; + LL_ERRS("LLInstanceTracker") << LLError::Log::demangle(cls) + << arg1 << arg2 << arg3 << LL_ENDL; } diff --git a/indra/llcommon/llinstancetracker.h b/indra/llcommon/llinstancetracker.h index 5d32ce9424..402333cca7 100644 --- a/indra/llcommon/llinstancetracker.h +++ b/indra/llcommon/llinstancetracker.h @@ -28,440 +28,432 @@ #ifndef LL_LLINSTANCETRACKER_H #define LL_LLINSTANCETRACKER_H -#include #include +#include +#include #include +#include +#include + +#include "mutex.h" -#include -#include "llstringtable.h" #include #include -// -#ifdef LL_DEBUG -#include "llerror.h" -#endif -// +#include -// As of 2017-05-06, as far as nat knows, only clang supports __has_feature(). -// Unfortunately VS2013's preprocessor shortcut logic doesn't prevent it from -// producing (fatal) warnings for defined(__clang__) && __has_feature(...). -// Have to work around that. -#if ! defined(__clang__) -#define __has_feature(x) 0 -#endif // __clang__ +#include "lockstatic.h" +#include "stringize.h" -#if defined(LL_TEST_llinstancetracker) && __has_feature(cxx_noexcept) -// ~LLInstanceTracker() performs llassert_always() validation. That's fine in -// production code, since the llassert_always() is implemented as an LL_ERRS -// message, which will crash-with-message. In our integration test executable, -// though, this llassert_always() throws an exception instead so we can test -// error conditions and continue running the test. However -- as of C++11, -// destructors are implicitly noexcept(true). Unless we mark -// ~LLInstanceTracker() noexcept(false), the test executable crashes even on -// the ATTEMPT to throw. -#define LLINSTANCETRACKER_DTOR_NOEXCEPT noexcept(false) -#else -// If we're building for production, or in fact building *any other* test, or -// we're using a compiler that doesn't support __has_feature(), or we're not -// compiling with a C++ version that supports noexcept -- don't specify it. -#define LLINSTANCETRACKER_DTOR_NOEXCEPT -#endif - -// As of 2017-05-06, as far as nat knows, only clang supports __has_feature(). -// Unfortunately VS2013's preprocessor shortcut logic doesn't prevent it from -// producing (fatal) warnings for defined(__clang__) && __has_feature(...). -// Have to work around that. -#if ! defined(__clang__) -#define __has_feature(x) 0 -#endif // __clang__ - -#if defined(LL_TEST_llinstancetracker) && __has_feature(cxx_noexcept) -// ~LLInstanceTracker() performs llassert_always() validation. That's fine in -// production code, since the llassert_always() is implemented as an LL_ERRS -// message, which will crash-with-message. In our integration test executable, -// though, this llassert_always() throws an exception instead so we can test -// error conditions and continue running the test. However -- as of C++11, -// destructors are implicitly noexcept(true). Unless we mark -// ~LLInstanceTracker() noexcept(false), the test executable crashes even on -// the ATTEMPT to throw. -#define LLINSTANCETRACKER_DTOR_NOEXCEPT noexcept(false) -#else -// If we're building for production, or in fact building *any other* test, or -// we're using a compiler that doesn't support __has_feature(), or we're not -// compiling with a C++ version that supports noexcept -- don't specify it. -#define LLINSTANCETRACKER_DTOR_NOEXCEPT -#endif - -/** - * Base class manages "class-static" data that must actually have singleton - * semantics: one instance per process, rather than one instance per module as - * sometimes happens with data simply declared static. - */ -class LL_COMMON_API LLInstanceTrackerBase +/***************************************************************************** +* StaticBase +*****************************************************************************/ +namespace LLInstanceTrackerPrivate { -protected: - /// It's not essential to derive your STATICDATA (for use with - /// getStatic()) from StaticBase; it's just that both known - /// implementations do. struct StaticBase { - // Only needed in debug builds -#ifdef LL_DEBUG - StaticBase() - : sIterationNestDepth(0) - {} + // We need to be able to lock static data while manipulating it. + std::mutex mMutex; + }; -#else - StaticBase() - {} -#endif - // - - void incrementDepth(); - void decrementDepth(); - U32 getDepth(); - private: -#ifdef LL_WINDOWS - std::atomic_uint32_t sIterationNestDepth; -#else - std::atomic_uint sIterationNestDepth; -#endif - }; -}; - -LL_COMMON_API void assert_main_thread(); + void logerrs(const char* cls, const std::string&, const std::string&, const std::string&); +} // namespace LLInstanceTrackerPrivate +/***************************************************************************** +* LLInstanceTracker with key +*****************************************************************************/ enum EInstanceTrackerAllowKeyCollisions { - LLInstanceTrackerErrorOnCollision, - LLInstanceTrackerReplaceOnCollision + LLInstanceTrackerErrorOnCollision, + LLInstanceTrackerReplaceOnCollision }; /// This mix-in class adds support for tracking all instances of the specified class parameter T /// The (optional) key associates a value of type KEY with a given instance of T, for quick lookup /// If KEY is not provided, then instances are stored in a simple set /// @NOTE: see explicit specialization below for default KEY==void case -/// @NOTE: this class is not thread-safe unless used as read-only -template -class LLInstanceTracker : public LLInstanceTrackerBase +template +class LLInstanceTracker { -protected: - typedef LLInstanceTracker self_t; - typedef typename std::multimap InstanceMap; - struct StaticData: public StaticBase - { - InstanceMap sMap; - }; - static StaticData& getStatic() { static StaticData sData; return sData;} - static InstanceMap& getMap_() { return getStatic().sMap; } + typedef std::map> InstanceMap; + struct StaticData: public LLInstanceTrackerPrivate::StaticBase + { + InstanceMap mMap; + }; + typedef llthread::LockStatic LockStatic; public: - class instance_iter : public boost::iterator_facade - { - public: - typedef boost::iterator_facade super_t; - - instance_iter(const typename InstanceMap::iterator& it) - : mIterator(it) - { - // Minimize calls to getStatic -#ifdef LL_DEBUG - getStatic().incrementDepth(); -#endif - // - } + // snapshot of std::pair> pairs + class snapshot + { + // It's very important that what we store in this snapshot are + // weak_ptrs, NOT shared_ptrs. That's how we discover whether any + // instance has been deleted during the lifespan of a snapshot. + typedef std::vector>> VectorType; + // Dereferencing our iterator produces a std::shared_ptr for each + // instance that still exists. Since we store weak_ptrs, that involves + // two chained transformations: + // - a transform_iterator to lock the weak_ptr and return a shared_ptr + // - a filter_iterator to skip any shared_ptr that has become invalid. + // It is very important that we filter lazily, that is, during + // traversal. Any one of our stored weak_ptrs might expire during + // traversal. + typedef std::pair> strong_pair; + // Note for future reference: nat has not yet had any luck (up to + // Boost 1.67) trying to use boost::transform_iterator with a hand- + // coded functor, only with actual functions. In my experience, an + // internal boost::result_of() operation fails, even with an explicit + // result_type typedef. But this works. + static strong_pair strengthen(typename VectorType::value_type& pair) + { + return { pair.first, pair.second.lock() }; + } + static bool dead_skipper(const strong_pair& pair) + { + return bool(pair.second); + } - ~instance_iter() - { - // Minimize calls to getStatic -#ifdef LL_DEBUG - getStatic().decrementDepth(); -#endif - // - } + public: + snapshot(): + // populate our vector with a snapshot of (locked!) InstanceMap + // note, this assigns pair to pair + mData(mLock->mMap.begin(), mLock->mMap.end()) + { + // release the lock once we've populated mData + mLock.unlock(); + } + // You can't make a transform_iterator (or anything else) that + // literally stores a C++ function (decltype(strengthen)) -- but you + // can make a transform_iterator based on a _function pointer._ + typedef boost::transform_iterator strong_iterator; + typedef boost::filter_iterator iterator; - private: - friend class boost::iterator_core_access; + iterator begin() { return make_iterator(mData.begin()); } + iterator end() { return make_iterator(mData.end()); } - void increment() { mIterator++; } - bool equal(instance_iter const& other) const - { - return mIterator == other.mIterator; - } + private: + iterator make_iterator(typename VectorType::iterator iter) + { + // transform_iterator only needs the base iterator and the transform. + // filter_iterator wants the predicate and both ends of the range. + return iterator(dead_skipper, + strong_iterator(iter, strengthen), + strong_iterator(mData.end(), strengthen)); + } - T& dereference() const - { - return *(mIterator->second); - } + // lock static data during construction +#if ! LL_WINDOWS + LockStatic mLock; +#else // LL_WINDOWS + // We want to be able to use (e.g.) our instance_snapshot subclass as: + // for (auto& inst : T::instance_snapshot()) ... + // But when this snapshot base class directly contains LockStatic, as + // above, Visual Studio 2017 requires us to code instead: + // for (auto& inst : std::move(T::instance_snapshot())) ... + // nat thinks this should be unnecessary, as an anonymous class + // instance is already a temporary. It shouldn't need to be cast to + // rvalue reference (the role of std::move()). clang evidently agrees, + // as the short form works fine with Xcode on Mac. + // To support the succinct usage, instead of directly storing + // LockStatic, store std::shared_ptr, which is copyable. + std::shared_ptr mLockp{std::make_shared()}; + LockStatic& mLock{*mLockp}; +#endif // LL_WINDOWS + VectorType mData; + }; - typename InstanceMap::iterator mIterator; - }; + // iterate over this for references to each instance + class instance_snapshot: public snapshot + { + private: + static T& instance_getter(typename snapshot::iterator::reference pair) + { + return *pair.second; + } + public: + typedef boost::transform_iterator iterator; + iterator begin() { return iterator(snapshot::begin(), instance_getter); } + iterator end() { return iterator(snapshot::end(), instance_getter); } - class key_iter : public boost::iterator_facade - { - public: - typedef boost::iterator_facade super_t; + void deleteAll() + { + for (auto it(snapshot::begin()), end(snapshot::end()); it != end; ++it) + { + delete it->second.get(); + } + } + }; - key_iter(typename InstanceMap::iterator it) - : mIterator(it) - { - // Minimize calls to getStatic -#ifdef LL_DEBUG - getStatic().incrementDepth(); -#endif - // - } + // iterate over this for each key + class key_snapshot: public snapshot + { + private: + static KEY key_getter(typename snapshot::iterator::reference pair) + { + return pair.first; + } + public: + typedef boost::transform_iterator iterator; + iterator begin() { return iterator(snapshot::begin(), key_getter); } + iterator end() { return iterator(snapshot::end(), key_getter); } + }; - key_iter(const key_iter& other) - : mIterator(other.mIterator) - { - // Minimize calls to getStatic -#ifdef LL_DEBUG - getStatic().incrementDepth(); -#endif - // - } + static T* getInstance(const KEY& k) + { + LockStatic lock; + const InstanceMap& map(lock->mMap); + typename InstanceMap::const_iterator found = map.find(k); + return (found == map.end()) ? NULL : found->second.get(); + } - ~key_iter() - { - // Minimize calls to getStatic -#ifdef LL_DEBUG - getStatic().decrementDepth(); -#endif - // - } - - - private: - friend class boost::iterator_core_access; - - void increment() { mIterator++; } - bool equal(key_iter const& other) const - { - return mIterator == other.mIterator; - } - - KEY& dereference() const - { - return const_cast(mIterator->first); - } - - typename InstanceMap::iterator mIterator; - }; - - static T* getInstance(const KEY& k) - { - const InstanceMap& map(getMap_()); - typename InstanceMap::const_iterator found = map.find(k); - return (found == map.end()) ? NULL : found->second; - } - - static instance_iter beginInstances() - { - return instance_iter(getMap_().begin()); - } - - static instance_iter endInstances() - { - 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(); - } - - static key_iter beginKeys() - { - return key_iter(getMap_().begin()); - } - static key_iter endKeys() - { - return key_iter(getMap_().end()); - } + static S32 instanceCount() + { + return LockStatic()->mMap.size(); + } protected: - LLInstanceTracker(const KEY& key) - { - // make sure static data outlives all instances - getStatic(); - add_(key); - } - virtual ~LLInstanceTracker() LLINSTANCETRACKER_DTOR_NOEXCEPT - { - // it's unsafe to delete instances of this type while all instances are being iterated over. - - // Minimize calls to getStatic -#ifdef LL_DEBUG - llassert_always(getStatic().getDepth() == 0); -#endif - // - - remove_(); - } - virtual void setKey(KEY key) { remove_(); add_(key); } + LLInstanceTracker(const KEY& key) + { + // We do not intend to manage the lifespan of this object with + // shared_ptr, so give it a no-op deleter. We store shared_ptrs in our + // InstanceMap specifically so snapshot can store weak_ptrs so we can + // detect deletions during traversals. + std::shared_ptr ptr(static_cast(this), [](T*){}); + LockStatic lock; + add_(lock, key, ptr); + } +public: + virtual ~LLInstanceTracker() + { + LockStatic lock; + remove_(lock); + } +protected: + virtual void setKey(KEY key) + { + LockStatic lock; + // Even though the shared_ptr we store in our map has a no-op deleter + // for T itself, letting the use count decrement to 0 will still + // delete the use-count object. Capture the shared_ptr we just removed + // and re-add it to the map with the new key. + auto ptr = remove_(lock); + add_(lock, key, ptr); + } +public: + virtual const KEY& getKey() const { return mInstanceKey; } private: - LLInstanceTracker( const LLInstanceTracker& ); - const LLInstanceTracker& operator=( const LLInstanceTracker& ); + LLInstanceTracker( const LLInstanceTracker& ) = delete; + LLInstanceTracker& operator=( const LLInstanceTracker& ) = delete; - void add_(const KEY& key) - { - mInstanceKey = key; - InstanceMap& map = getMap_(); - typename InstanceMap::iterator insertion_point_it = map.lower_bound(key); - if (insertion_point_it != map.end() - && insertion_point_it->first == key) - { // found existing entry with that key - switch(KEY_COLLISION_BEHAVIOR) - { - case LLInstanceTrackerErrorOnCollision: - { - // use assert here instead of LL_ERRS(), otherwise the error will be ignored since this call is made during global object initialization - llassert_always_msg(false, "Instance with this same key already exists!"); - break; - } - case LLInstanceTrackerReplaceOnCollision: - { - // replace pointer, but leave key (should have compared equal anyway) - insertion_point_it->second = static_cast(this); - break; - } - default: - break; - } - } - else - { // new key - map.insert(insertion_point_it, std::make_pair(key, static_cast(this))); - } - } - void remove_() - { - InstanceMap& map = getMap_(); - typename InstanceMap::iterator iter = map.find(mInstanceKey); - if (iter != map.end()) - { - map.erase(iter); - } - } + // for logging + template + static std::string report(K key) { return stringize(key); } + static std::string report(const std::string& key) { return "'" + key + "'"; } + static std::string report(const char* key) { return report(std::string(key)); } + + // caller must instantiate LockStatic + void add_(LockStatic& lock, const KEY& key, const std::shared_ptr& ptr) + { + mInstanceKey = key; + InstanceMap& map = lock->mMap; + switch(KEY_COLLISION_BEHAVIOR) + { + case LLInstanceTrackerErrorOnCollision: + { + // map stores shared_ptr to self + auto pair = map.emplace(key, ptr); + if (! pair.second) + { + LLInstanceTrackerPrivate::logerrs(typeid(*this).name(), " instance with key ", + report(key), " already exists!"); + } + break; + } + case LLInstanceTrackerReplaceOnCollision: + map[key] = ptr; + break; + default: + break; + } + } + std::shared_ptr remove_(LockStatic& lock) + { + InstanceMap& map = lock->mMap; + typename InstanceMap::iterator iter = map.find(mInstanceKey); + if (iter != map.end()) + { + auto ret = iter->second; + map.erase(iter); + return ret; + } + return {}; + } private: - KEY mInstanceKey; + KEY mInstanceKey; }; +/***************************************************************************** +* LLInstanceTracker without key +*****************************************************************************/ +// TODO: +// - For the case of omitted KEY template parameter, consider storing +// std::map> instead of std::set>. +// That might let us share more of the implementation between KEY and +// non-KEY LLInstanceTracker subclasses. +// - Even if not that, consider trying to unify the snapshot implementations. +// The trouble is that the 'iterator' published by each (and by their +// subclasses) must reflect the specific type of the callables that +// distinguish them. (Maybe make instance_snapshot() and key_snapshot() +// factory functions that pass lambdas to a factory function for the generic +// template class?) + /// explicit specialization for default case where KEY is void /// use a simple std::set template -class LLInstanceTracker : public LLInstanceTrackerBase +class LLInstanceTracker { -protected: - typedef LLInstanceTracker self_t; - typedef typename std::set InstanceSet; - struct StaticData: public StaticBase - { - InstanceSet sSet; - }; - static StaticData& getStatic() { static StaticData sData; return sData; } - static InstanceSet& getSet_() { return getStatic().sSet; } + typedef std::set> InstanceSet; + struct StaticData: public LLInstanceTrackerPrivate::StaticBase + { + InstanceSet mSet; + }; + typedef llthread::LockStatic LockStatic; public: + /** + * Storing a dumb T* somewhere external is a bad idea, since + * LLInstanceTracker subclasses are explicitly destroyed rather than + * managed by smart pointers. It's legal to declare stack instances of an + * LLInstanceTracker subclass. But it's reasonable to store a + * std::weak_ptr, which will become invalid when the T instance is + * destroyed. + */ + std::weak_ptr getWeak() + { + return mSelf; + } + + static S32 instanceCount() { return LockStatic()->mSet.size(); } - /** - * Does a particular instance still exist? Of course, if you already have - * a T* in hand, you need not call getInstance() to @em locate the - * instance -- unlike the case where getInstance() accepts some kind of - * key. Nonetheless this method is still useful to @em validate a - * particular T*, since each instance's destructor removes itself from the - * underlying set. - */ - static T* getInstance(T* k) - { - const InstanceSet& set(getSet_()); - typename InstanceSet::const_iterator found = set.find(k); - return (found == set.end())? NULL : *found; - } - static S32 instanceCount() { return getSet_().size(); } + // snapshot of std::shared_ptr pointers + class snapshot + { + // It's very important that what we store in this snapshot are + // weak_ptrs, NOT shared_ptrs. That's how we discover whether any + // instance has been deleted during the lifespan of a snapshot. + typedef std::vector> VectorType; + // Dereferencing our iterator produces a std::shared_ptr for each + // instance that still exists. Since we store weak_ptrs, that involves + // two chained transformations: + // - a transform_iterator to lock the weak_ptr and return a shared_ptr + // - a filter_iterator to skip any shared_ptr that has become invalid. + typedef std::shared_ptr strong_ptr; + static strong_ptr strengthen(typename VectorType::value_type& ptr) + { + return ptr.lock(); + } + static bool dead_skipper(const strong_ptr& ptr) + { + return bool(ptr); + } - class instance_iter : public boost::iterator_facade - { - public: - instance_iter(const typename InstanceSet::iterator& it) - : mIterator(it) - { - // Minimize calls to getStatic -#ifdef LL_DEBUG - getStatic().incrementDepth(); -#endif - // - } + public: + snapshot(): + // populate our vector with a snapshot of (locked!) InstanceSet + // note, this assigns stored shared_ptrs to weak_ptrs for snapshot + mData(mLock->mSet.begin(), mLock->mSet.end()) + { + // release the lock once we've populated mData + mLock.unlock(); + } - instance_iter(const instance_iter& other) - : mIterator(other.mIterator) - { - // Minimize calls to getStatic -#ifdef LL_DEBUG - getStatic().incrementDepth(); -#endif - } + typedef boost::transform_iterator strong_iterator; + typedef boost::filter_iterator iterator; - ~instance_iter() - { - // Minimize calls to getStatic -#ifdef LL_DEBUG - getStatic().decrementDepth(); -#endif - // - } + iterator begin() { return make_iterator(mData.begin()); } + iterator end() { return make_iterator(mData.end()); } - private: - friend class boost::iterator_core_access; + private: + iterator make_iterator(typename VectorType::iterator iter) + { + // transform_iterator only needs the base iterator and the transform. + // filter_iterator wants the predicate and both ends of the range. + return iterator(dead_skipper, + strong_iterator(iter, strengthen), + strong_iterator(mData.end(), strengthen)); + } - void increment() { mIterator++; } - bool equal(instance_iter const& other) const - { - return mIterator == other.mIterator; - } + // lock static data during construction +#if ! LL_WINDOWS + LockStatic mLock; +#else // LL_WINDOWS + // We want to be able to use our instance_snapshot subclass as: + // for (auto& inst : T::instance_snapshot()) ... + // But when this snapshot base class directly contains LockStatic, as + // above, Visual Studio 2017 requires us to code instead: + // for (auto& inst : std::move(T::instance_snapshot())) ... + // nat thinks this should be unnecessary, as an anonymous class + // instance is already a temporary. It shouldn't need to be cast to + // rvalue reference (the role of std::move()). clang evidently agrees, + // as the short form works fine with Xcode on Mac. + // To support the succinct usage, instead of directly storing + // LockStatic, store std::shared_ptr, which is copyable. + std::shared_ptr mLockp{std::make_shared()}; + LockStatic& mLock{*mLockp}; +#endif // LL_WINDOWS + VectorType mData; + }; - T& dereference() const - { - return **mIterator; - } + // iterate over this for references to each instance + struct instance_snapshot: public snapshot + { + typedef boost::indirect_iterator iterator; + iterator begin() { return iterator(snapshot::begin()); } + iterator end() { return iterator(snapshot::end()); } - typename InstanceSet::iterator mIterator; - }; - - static instance_iter beginInstances() { return instance_iter(getSet_().begin()); } - static instance_iter endInstances() { return instance_iter(getSet_().end()); } + void deleteAll() + { + for (auto it(snapshot::begin()), end(snapshot::end()); it != end; ++it) + { + delete it->get(); + } + } + }; protected: - LLInstanceTracker() - { - // make sure static data outlives all instances - getStatic(); - getSet_().insert(static_cast(this)); - } - virtual ~LLInstanceTracker() LLINSTANCETRACKER_DTOR_NOEXCEPT - { - // it's unsafe to delete instances of this type while all instances are being iterated over. + LLInstanceTracker() + { + // Since we do not intend for this shared_ptr to manage lifespan, give + // it a no-op deleter. + std::shared_ptr ptr(static_cast(this), [](T*){}); + // save corresponding weak_ptr for future reference + mSelf = ptr; + // Also store it in our class-static set to track this instance. + LockStatic()->mSet.emplace(ptr); + } +public: + virtual ~LLInstanceTracker() + { + // convert weak_ptr to shared_ptr because that's what we store in our + // InstanceSet + LockStatic()->mSet.erase(mSelf.lock()); + } +protected: + LLInstanceTracker(const LLInstanceTracker& other): + LLInstanceTracker() + {} - // Minimize calls to getStatic -#ifdef LL_DEBUG - llassert_always(getStatic().getDepth() == 0); -#endif - // - - getSet_().erase(static_cast(this)); - } - - LLInstanceTracker(const LLInstanceTracker& other) - { - getSet_().insert(static_cast(this)); - } +private: + // Storing a weak_ptr to self is a bit like deriving from + // std::enable_shared_from_this(), except more explicit. + std::weak_ptr mSelf; }; #endif diff --git a/indra/llcommon/llleaplistener.cpp b/indra/llcommon/llleaplistener.cpp index ed7d7b47e3..3e6ce9092c 100644 --- a/indra/llcommon/llleaplistener.cpp +++ b/indra/llcommon/llleaplistener.cpp @@ -208,13 +208,11 @@ void LLLeapListener::getAPIs(const LLSD& request) const { Response reply(LLSD(), request); - for (LLEventAPI::instance_iter eai(LLEventAPI::beginInstances()), - eaend(LLEventAPI::endInstances()); - eai != eaend; ++eai) + for (auto& ea : LLEventAPI::instance_snapshot()) { LLSD info; - info["desc"] = eai->getDesc(); - reply[eai->getName()] = info; + info["desc"] = ea.getDesc(); + reply[ea.getName()] = info; } } diff --git a/indra/llcommon/llmainthreadtask.cpp b/indra/llcommon/llmainthreadtask.cpp new file mode 100644 index 0000000000..e0d70cacd8 --- /dev/null +++ b/indra/llcommon/llmainthreadtask.cpp @@ -0,0 +1,22 @@ +/** + * @file llmainthreadtask.cpp + * @author Nat Goodspeed + * @date 2019-12-05 + * @brief Implementation for llmainthreadtask. + * + * $LicenseInfo:firstyear=2019&license=viewerlgpl$ + * Copyright (c) 2019, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "llmainthreadtask.h" +// STL headers +// std headers +// external library headers +// other Linden headers + +// This file is required by our CMake integration-test machinery. It +// contributes no code to the viewer executable. diff --git a/indra/llcommon/llmainthreadtask.h b/indra/llcommon/llmainthreadtask.h new file mode 100644 index 0000000000..d509b687c0 --- /dev/null +++ b/indra/llcommon/llmainthreadtask.h @@ -0,0 +1,99 @@ +/** + * @file llmainthreadtask.h + * @author Nat Goodspeed + * @date 2019-12-04 + * @brief LLMainThreadTask dispatches work to the main thread. When invoked on + * the main thread, it performs the work inline. + * + * $LicenseInfo:firstyear=2019&license=viewerlgpl$ + * Copyright (c) 2019, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_LLMAINTHREADTASK_H) +#define LL_LLMAINTHREADTASK_H + +#include "lleventtimer.h" +#include "llthread.h" +#include "llmake.h" +#include +#include // std::result_of + +/** + * LLMainThreadTask provides a way to perform some task specifically on the + * main thread, waiting for it to complete. A task consists of a C++ nullary + * invocable (i.e. any callable that requires no arguments) with arbitrary + * return type. + * + * Instead of instantiating LLMainThreadTask, pass your invocable to its + * static dispatch() method. dispatch() returns the result of calling your + * task. (Or, if your task throws an exception, dispatch() throws that + * exception. See std::packaged_task.) + * + * When you call dispatch() on the main thread (as determined by + * on_main_thread() in llthread.h), it simply calls your task and returns the + * result. + * + * When you call dispatch() on a secondary thread, it instantiates an + * LLEventTimer subclass scheduled immediately. Next time the main loop calls + * LLEventTimer::updateClass(), your task will be run, and LLMainThreadTask + * will fulfill a future with its result. Meanwhile the requesting thread + * blocks on that future. As soon as it is set, the requesting thread wakes up + * with the task result. + */ +class LLMainThreadTask +{ +private: + // Don't instantiate this class -- use dispatch() instead. + LLMainThreadTask() {} + +public: + /// dispatch() is the only way to invoke this functionality. + template + static auto dispatch(CALLABLE&& callable) -> decltype(callable()) + { + if (on_main_thread()) + { + // we're already running on the main thread, perfect + return callable(); + } + else + { + // It's essential to construct LLEventTimer subclass instances on + // the heap because, on completion, LLEventTimer deletes them. + // Once we enable C++17, we can use Class Template Argument + // Deduction. Until then, use llmake_heap(). + auto* task = llmake_heap(std::forward(callable)); + auto future = task->mTask.get_future(); + // Now simply block on the future. + return future.get(); + } + } + +private: + template + struct Task: public LLEventTimer + { + Task(CALLABLE&& callable): + // no wait time: call tick() next chance we get + LLEventTimer(0), + mTask(std::forward(callable)) + {} + BOOL tick() override + { + // run the task on the main thread, will populate the future + // obtained by get_future() + mTask(); + // tell LLEventTimer we're done (one shot) + return TRUE; + } + // Given arbitrary CALLABLE, which might be a lambda, how are we + // supposed to obtain its signature for std::packaged_task? It seems + // redundant to have to add an argument list to engage result_of, then + // add the argument list again to complete the signature. At least we + // only support a nullary CALLABLE. + std::packaged_task::type()> mTask; + }; +}; + +#endif /* ! defined(LL_LLMAINTHREADTASK_H) */ diff --git a/indra/llcommon/llmake.h b/indra/llcommon/llmake.h index d3538aa693..f901ee2bf1 100644 --- a/indra/llcommon/llmake.h +++ b/indra/llcommon/llmake.h @@ -12,10 +12,8 @@ * * also relevant: * - * Template argument deduction for class templates - * http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0091r3.html - * was apparently adopted in June 2016? Unclear when compilers will - * portably support this, but there is hope. + * Template argument deduction for class templates (C++17) + * https://en.cppreference.com/w/cpp/language/class_template_argument_deduction * * $LicenseInfo:firstyear=2015&license=viewerlgpl$ * Copyright (c) 2015, Linden Research, Inc. @@ -25,10 +23,62 @@ #if ! defined(LL_LLMAKE_H) #define LL_LLMAKE_H +// If we're using a compiler newer than VS 2013, use variadic llmake(). +#if (! defined(_MSC_VER)) || (_MSC_VER > 1800) + +/** + * Usage: llmake(args...) + * + * Deduces the types T... of 'args' and returns an instance of + * SomeTemplate(args...). + */ template class CLASS_TEMPLATE, typename... ARGS> CLASS_TEMPLATE llmake(ARGS && ... args) { return CLASS_TEMPLATE(std::forward(args)...); } +#else // older implementation for VS 2013 + +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 // VS 2013 workaround + +/// dumb pointer template just in case that's what's wanted +template +using dumb_pointer = T*; + +/** + * Same as llmake(), but returns a pointer to a new heap instance of + * SomeTemplate(args...) using the pointer of your choice. + * + * @code + * auto* dumb = llmake_heap(args...); + * auto shared = llmake_heap(args...); + * auto unique = llmake_heap(args...); + * @endcode + */ +// POINTER_TEMPLATE is characterized as template rather than as +// template because (e.g.) std::unique_ptr has multiple template +// arguments. Even though we only engage one, std::unique_ptr doesn't match a +// template template parameter that itself takes only one template parameter. +template class CLASS_TEMPLATE, + template class POINTER_TEMPLATE=dumb_pointer, + typename... ARGS> +POINTER_TEMPLATE> llmake_heap(ARGS&&... args) +{ + return POINTER_TEMPLATE>( + new CLASS_TEMPLATE(std::forward(args)...)); +} + #endif /* ! defined(LL_LLMAKE_H) */ diff --git a/indra/llcommon/llmutex.cpp b/indra/llcommon/llmutex.cpp index 75f43a4704..4d73c04d07 100644 --- a/indra/llcommon/llmutex.cpp +++ b/indra/llcommon/llmutex.cpp @@ -32,8 +32,7 @@ //============================================================================ LLMutex::LLMutex() : - mCount(0), - mLockingThread(NO_THREAD) + mCount(0) { } @@ -55,7 +54,7 @@ void LLMutex::lock() #if MUTEX_DEBUG // Have to have the lock before we can access the debug info - U32 id = LLThread::currentID(); + auto id = LLThread::currentID(); if (mIsLocked[id] != FALSE) LL_ERRS() << "Already locked in Thread: " << id << LL_ENDL; mIsLocked[id] = TRUE; @@ -74,13 +73,13 @@ void LLMutex::unlock() #if MUTEX_DEBUG // Access the debug info while we have the lock - U32 id = LLThread::currentID(); + auto id = LLThread::currentID(); if (mIsLocked[id] != TRUE) LL_ERRS() << "Not locked in Thread: " << id << LL_ENDL; mIsLocked[id] = FALSE; #endif - mLockingThread = NO_THREAD; + mLockingThread = LLThread::id_t(); mMutex.unlock(); } @@ -102,7 +101,7 @@ bool LLMutex::isSelfLocked() return mLockingThread == LLThread::currentID(); } -U32 LLMutex::lockingThread() const +LLThread::id_t LLMutex::lockingThread() const { return mLockingThread; } @@ -122,7 +121,7 @@ bool LLMutex::trylock() #if MUTEX_DEBUG // Have to have the lock before we can access the debug info - U32 id = LLThread::currentID(); + auto id = LLThread::currentID(); if (mIsLocked[id] != FALSE) LL_ERRS() << "Already locked in Thread: " << id << LL_ENDL; mIsLocked[id] = TRUE; diff --git a/indra/llcommon/llmutex.h b/indra/llcommon/llmutex.h index f841d7f950..838d7d34c0 100644 --- a/indra/llcommon/llmutex.h +++ b/indra/llcommon/llmutex.h @@ -28,20 +28,12 @@ #define LL_LLMUTEX_H #include "stdtypes.h" +#include "llthread.h" #include -#if LL_WINDOWS -#pragma warning (push) -#pragma warning (disable:4265) -#endif -// 'std::_Pad' : class has virtual functions, but destructor is not virtual -#include +#include "mutex.h" #include -#if LL_WINDOWS -#pragma warning (pop) -#endif - //============================================================================ #define MUTEX_DEBUG (LL_DEBUG || LL_RELEASE_WITH_DEBUG_INFO) @@ -53,11 +45,6 @@ class LL_COMMON_API LLMutex { public: - typedef enum - { - NO_THREAD = 0xFFFFFFFF - } e_locking_thread; - LLMutex(); virtual ~LLMutex(); @@ -66,15 +53,15 @@ public: void unlock(); // undefined behavior when called on mutex not being held bool isLocked(); // non-blocking, but does do a lock/unlock so not free bool isSelfLocked(); //return true if locked in a same thread - U32 lockingThread() const; //get ID of locking thread - + LLThread::id_t lockingThread() const; //get ID of locking thread + protected: std::mutex mMutex; mutable U32 mCount; - mutable U32 mLockingThread; + mutable LLThread::id_t mLockingThread; #if MUTEX_DEBUG - std::map mIsLocked; + std::map mIsLocked; #endif }; diff --git a/indra/llcommon/llrefcount.h b/indra/llcommon/llrefcount.h index fb0411d27b..7e4af6ea66 100644 --- a/indra/llcommon/llrefcount.h +++ b/indra/llcommon/llrefcount.h @@ -28,9 +28,10 @@ #include #include -#include "llmutex.h" #include "llatomic.h" +class LLMutex; + //---------------------------------------------------------------------------- // RefCount objects should generally only be accessed by way of LLPointer<>'s // see llthread.h for LLThreadSafeRefCount diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index cfaa60d452..745af9d9ef 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -30,6 +30,7 @@ #include "llerror.h" #include "llerrorcontrol.h" // LLError::is_available() #include "lldependencies.h" +#include "llexception.h" #include "llcoros.h" #include "llexception.h" #include @@ -42,8 +43,6 @@ namespace { void log(LLError::ELevel level, const char* p1, const char* p2, const char* p3, const char* p4); -void logdebugs(const char* p1="", const char* p2="", const char* p3="", const char* p4=""); - bool oktolog(); } // anonymous namespace @@ -117,7 +116,7 @@ private: // stack for every running coroutine. Therefore this stack must be based // on a coroutine-local pointer. // This local_ptr isn't static because it's a member of an LLSingleton. - LLCoros::local_ptr mInitializing; + LLCoros::local_ptr mInitializing; public: // Instantiate this to obtain a reference to the coroutine-specific @@ -297,7 +296,7 @@ void LLSingletonBase::MasterList::LockedInitializing::log(const char* verb, cons } } -void LLSingletonBase::capture_dependency(EInitState initState) +void LLSingletonBase::capture_dependency() { MasterList::LockedInitializing locked_list; list_t& initializing(locked_list.get()); @@ -329,21 +328,8 @@ void LLSingletonBase::capture_dependency(EInitState initState) LLSingletonBase* foundp(*found); out << classname(foundp) << " -> "; } - // We promise to capture dependencies from both the constructor - // and the initSingleton() method, so an LLSingleton's instance - // pointer is on the initializing list during both. Now that we've - // detected circularity, though, we must distinguish the two. If - // the recursive call is from the constructor, we CAN'T honor it: - // otherwise we'd be returning a pointer to a partially- - // constructed object! But from initSingleton() is okay: that - // method exists specifically to support circularity. // Decide which log helper to call. - if (initState == CONSTRUCTING) - { - logerrs("LLSingleton circularity in Constructor: ", out.str().c_str(), - classname(this).c_str(), ""); - } - else if (it_next == initializing.end()) + if (it_next == initializing.end()) { // Points to self after construction, but during initialization. // Singletons can initialize other classes that depend onto them, @@ -391,7 +377,7 @@ LLSingletonBase::vec_t LLSingletonBase::dep_sort() SingletonDeps sdeps; // Lock while traversing the master list MasterList::LockedMaster master; - BOOST_FOREACH(LLSingletonBase* sp, master.get()) + for (LLSingletonBase* sp : master.get()) { // Build the SingletonDeps structure by adding, for each // LLSingletonBase* sp in the master list, sp itself. It has no @@ -408,14 +394,14 @@ LLSingletonBase::vec_t LLSingletonBase::dep_sort() // extracts just the first (key) element from each sorted_iterator, then // uses vec_t's range constructor... but frankly this is more // straightforward, as long as we remember the above reserve() call! - BOOST_FOREACH(SingletonDeps::sorted_iterator::value_type pair, sdeps.sort()) + for (const SingletonDeps::sorted_iterator::value_type& pair : sdeps.sort()) { ret.push_back(pair.first); } // The master list is not itself pushed onto the master list. Add it as // the very last entry -- it is the LLSingleton on which ALL others // depend! -- so our caller will process it. - ret.push_back(MasterList::getInstance()); + ret.push_back(&master.Lock::get()); return ret; } @@ -426,13 +412,9 @@ void LLSingletonBase::cleanup_() { cleanupSingleton(); } - catch (const std::exception& e) - { - logwarns("Exception in ", classname(this).c_str(), "::cleanupSingleton(): ", e.what()); - } catch (...) { - logwarns("Unknown exception in ", classname(this).c_str(), "::cleanupSingleton()"); + LOG_UNHANDLED_EXCEPTION(classname(this) + "::cleanupSingleton()"); } } @@ -503,10 +485,6 @@ void log(LLError::ELevel level, } } -void logdebugs(const char* p1, const char* p2, const char* p3, const char* p4) -{ - log(LLError::LEVEL_DEBUG, p1, p2, p3, p4); -} } // anonymous namespace //static @@ -515,6 +493,18 @@ void LLSingletonBase::logwarns(const char* p1, const char* p2, const char* p3, c log(LLError::LEVEL_WARN, p1, p2, p3, p4); } +//static +void LLSingletonBase::loginfos(const char* p1, const char* p2, const char* p3, const char* p4) +{ + log(LLError::LEVEL_INFO, p1, p2, p3, p4); +} + +//static +void LLSingletonBase::logdebugs(const char* p1, const char* p2, const char* p3, const char* p4) +{ + log(LLError::LEVEL_DEBUG, p1, p2, p3, p4); +} + //static void LLSingletonBase::logerrs(const char* p1, const char* p2, const char* p3, const char* p4) { diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index b80cc9bc00..04d7a27d3e 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -30,18 +30,10 @@ #include #include #include - -#if LL_WINDOWS -#pragma warning (push) -#pragma warning (disable:4265) -#endif -// warning C4265: 'std::_Pad' : class has virtual functions, but destructor is not virtual - -#include - -#if LL_WINDOWS -#pragma warning (pop) -#endif +#include "mutex.h" +#include "lockstatic.h" +#include "llthread.h" // on_main_thread() +#include "llmainthreadtask.h" class LLSingletonBase: private boost::noncopyable { @@ -66,8 +58,8 @@ protected: typedef enum e_init_state { UNINITIALIZED = 0, // must be default-initialized state + QUEUED, // construction queued, not yet executing CONSTRUCTING, // within DERIVED_TYPE constructor - CONSTRUCTED, // finished DERIVED_TYPE constructor INITIALIZING, // within DERIVED_TYPE::initSingleton() INITIALIZED, // normal case DELETED // deleteSingleton() or deleteAll() called @@ -116,14 +108,17 @@ protected: protected: // If a given call to B::getInstance() happens during either A::A() or // A::initSingleton(), record that A directly depends on B. - void capture_dependency(EInitState); + void capture_dependency(); - // delegate LL_ERRS() logging to llsingleton.cpp + // delegate logging calls to llsingleton.cpp static void logerrs(const char* p1, const char* p2="", const char* p3="", const char* p4=""); - // delegate LL_WARNS() logging to llsingleton.cpp static void logwarns(const char* p1, const char* p2="", const char* p3="", const char* p4=""); + static void loginfos(const char* p1, const char* p2="", + const char* p3="", const char* p4=""); + static void logdebugs(const char* p1, const char* p2="", + const char* p3="", const char* p4=""); static std::string demangle(const char* mangled); template static std::string classname() { return demangle(typeid(T).name()); } @@ -190,9 +185,9 @@ struct LLSingleton_manage_master { return LLSingletonBase::get_initializing_size(); } - void capture_dependency(LLSingletonBase* sb, LLSingletonBase::EInitState state) + void capture_dependency(LLSingletonBase* sb) { - sb->capture_dependency(state); + sb->capture_dependency(); } }; @@ -207,13 +202,13 @@ struct LLSingleton_manage_master // since we never pushed, no need to clean up void reset_initializing(LLSingletonBase::list_t::size_type size) {} LLSingletonBase::list_t::size_type get_initializing_size() { return 0; } - void capture_dependency(LLSingletonBase*, LLSingletonBase::EInitState) {} + void capture_dependency(LLSingletonBase*) {} }; // Now we can implement LLSingletonBase's template constructor. template LLSingletonBase::LLSingletonBase(tag): - mDeleteSingleton(NULL) + mDeleteSingleton(nullptr) { // This is the earliest possible point at which we can push this new // instance onto the init stack. LLSingleton::constructSingleton() can't @@ -283,11 +278,38 @@ class LLParamSingleton; * initSingleton() method explicitly depends on some other LLSingleton * subclass, you may continue to rely on that other subclass in your * cleanupSingleton() method. + * + * We introduce a special cleanupSingleton() method because cleanupSingleton() + * operations can involve nontrivial realtime, or might throw an exception. A + * destructor should do neither! + * + * If your cleanupSingleton() method throws an exception, we log that + * exception but proceed with the remaining cleanupSingleton() calls. + * + * Similarly, if at some point you call LLSingletonBase::deleteAll(), all + * remaining LLSingleton instances will be destroyed in dependency order. (Or + * call MySubclass::deleteSingleton() to specifically destroy the canonical + * MySubclass instance.) */ template class LLSingleton : public LLSingletonBase { private: + // LLSingleton must have a distinct instance of + // SingletonData for every distinct DERIVED_TYPE. It's tempting to + // consider hoisting SingletonData up into LLSingletonBase. Don't do it. + struct SingletonData + { + // Use a recursive_mutex in case of constructor circularity. With a + // non-recursive mutex, that would result in deadlock. + typedef std::recursive_mutex mutex_t; + mutex_t mMutex; // LockStatic looks for mMutex + + EInitState mInitState{UNINITIALIZED}; + DERIVED_TYPE* mInstance{nullptr}; + }; + typedef llthread::LockStatic LockStatic; + // Allow LLParamSingleton subclass -- but NOT DERIVED_TYPE itself -- to // access our private members. friend class LLParamSingleton; @@ -337,17 +359,17 @@ private: // purpose for its subclass LLParamSingleton is to support Singletons // requiring constructor arguments. constructSingleton() supports both use // cases. + // Accepting LockStatic& requires that the caller has already locked our + // static data before calling. template - static void constructSingleton(Args&&... args) + static void constructSingleton(LockStatic& lk, Args&&... args) { auto prev_size = LLSingleton_manage_master().get_initializing_size(); - // getInstance() calls are from within constructor - sData.mInitState = CONSTRUCTING; + // Any getInstance() calls after this point are from within constructor + lk->mInitState = CONSTRUCTING; try { - sData.mInstance = new DERIVED_TYPE(std::forward(args)...); - // we have called constructor, have not yet called initSingleton() - sData.mInitState = CONSTRUCTED; + lk->mInstance = new DERIVED_TYPE(std::forward(args)...); } catch (const std::exception& err) { @@ -361,58 +383,56 @@ private: // There isn't a separate EInitState value meaning "we attempted // to construct this LLSingleton subclass but could not," so use // DELETED. That seems slightly more appropriate than UNINITIALIZED. - sData.mInitState = DELETED; + lk->mInitState = DELETED; // propagate the exception throw; } - } - static void finishInitializing() - { - // getInstance() calls are from within initSingleton() - sData.mInitState = INITIALIZING; + // Any getInstance() calls after this point are from within initSingleton() + lk->mInitState = INITIALIZING; try { // initialize singleton after constructing it so that it can // reference other singletons which in turn depend on it, thus // breaking cyclic dependencies - sData.mInstance->initSingleton(); - sData.mInitState = INITIALIZED; + lk->mInstance->initSingleton(); + lk->mInitState = INITIALIZED; // pop this off stack of initializing singletons - pop_initializing(); + pop_initializing(lk->mInstance); } catch (const std::exception& err) { // pop this off stack of initializing singletons here, too -- // BEFORE logging, so log-machinery LLSingletons don't record a // dependency on DERIVED_TYPE! - pop_initializing(); + pop_initializing(lk->mInstance); logwarns("Error in ", classname().c_str(), "::initSingleton(): ", err.what()); - // and get rid of the instance entirely + // Get rid of the instance entirely. This call depends on our + // recursive_mutex. We could have a deleteSingleton(LockStatic&) + // overload and pass lk, but we don't strictly need it. deleteSingleton(); // propagate the exception throw; } } - static void pop_initializing() + static void pop_initializing(LLSingletonBase* sb) { // route through LLSingleton_manage_master so we Do The Right Thing // (namely, nothing) for MasterList - LLSingleton_manage_master().pop_initializing(sData.mInstance); + LLSingleton_manage_master().pop_initializing(sb); } - static void capture_dependency() + static void capture_dependency(LLSingletonBase* sb) { // By this point, if DERIVED_TYPE was pushed onto the initializing // stack, it has been popped off. So the top of that stack, if any, is // an LLSingleton that directly depends on DERIVED_TYPE. If // getInstance() was called by another LLSingleton, rather than from // vanilla application code, record the dependency. - LLSingleton_manage_master().capture_dependency( - sData.mInstance, sData.mInitState); + LLSingleton_manage_master().capture_dependency(sb); } // We know of no way to instruct the compiler that every subclass @@ -442,14 +462,16 @@ protected: protected: virtual ~LLSingleton() { - // In case racing threads call getInstance() at the same moment as - // this destructor, serialize the calls. - Locker lk; + // This phase of cleanup is performed in the destructor rather than in + // deleteSingleton() to defend against manual deletion. When we moved + // cleanup to deleteSingleton(), we hit crashes due to dangling + // pointers in the MasterList. + LockStatic lk; + lk->mInstance = nullptr; + lk->mInitState = DELETED; - // remove this instance from the master list + // Remove this instance from the master list. LLSingleton_manage_master().remove(this); - sData.mInstance = NULL; - sData.mInitState = DELETED; } public: @@ -469,71 +491,144 @@ public: */ static void deleteSingleton() { - // first call cleanupSingleton() - if (sData.mInstance) + // Hold the lock while we call cleanupSingleton() and the destructor. + // Our destructor also instantiates LockStatic, requiring a recursive + // mutex. + LockStatic lk; + // of course, only cleanup and delete if there's something there + if (lk->mInstance) { - sData.mInstance->cleanup_(); + lk->mInstance->cleanup_(); + delete lk->mInstance; + // destructor clears mInstance (and mInitState) } - // capture the instance and clear SingletonData - auto lameduck = sData.mInstance; - sData.mInstance = NULL; - sData.mInitState = DELETED; - // Now delete the instance. This sequence guards against the chance - // that the destructor throws, somebody catches it and there's a - // subsequent call to getInstance(). - delete lameduck; } static DERIVED_TYPE* getInstance() { - // In case racing threads call getInstance() at the same moment, - // serialize the calls. - Locker lk; + // We know the viewer has LLSingleton dependency circularities. If you + // feel strongly motivated to eliminate them, cheers and good luck. + // (At that point we could consider a much simpler locking mechanism.) - switch (sData.mInitState) - { - case CONSTRUCTING: - // here if DERIVED_TYPE's constructor (directly or indirectly) - // calls DERIVED_TYPE::getInstance() - logerrs("Tried to access singleton ", - classname().c_str(), - " from singleton constructor!"); - return NULL; + // If A and B depend on each other, and thread T1 requests A at the + // same moment thread T2 requests B, you could get a sequence like this: + // - T1 locks A + // - T2 locks B + // - T1, having constructed A, calls A::initSingleton(), which calls + // B::getInstance() and blocks on B's lock + // - T2, having constructed B, calls B::initSingleton(), which calls + // A::getInstance() and blocks on A's lock + // In other words, classic deadlock. - case UNINITIALIZED: - constructSingleton(); - // fall through... + // Avoid that by constructing and initializing every LLSingleton on + // the main thread. In that scenario: + // - T1 locks A + // - T2 locks B + // - T1 discovers A is UNINITIALIZED, so it queues a task for the main + // thread, unlocks A and blocks on the std::future. + // - T2 discovers B is UNINITIALIZED, so it queues a task for the main + // thread, unlocks B and blocks on the std::future. + // - The main thread executes T1's request for A. It locks A and + // starts to construct it. + // - A::initSingleton() calls B::getInstance(). Fine: nobody's holding + // B's lock. + // - The main thread locks B, constructs B, calls B::initSingleton(), + // which calls A::getInstance(), which returns A. + // - B::getInstance() returns B to A::initSingleton(), unlocking B. + // - A::getInstance() returns A to the task wrapper, unlocking A. + // - The task wrapper passes A to T1 via the future. T1 resumes. + // - The main thread executes T2's request for B. Oh look, B already + // exists. The task wrapper passes B to T2 via the future. T2 + // resumes. + // This still works even if one of T1 or T2 *is* the main thread. + // This still works even if thread T3 requests B at the same moment as + // T2. Finding B still UNINITIALIZED, T3 also queues a task for the + // main thread, unlocks B and blocks on a (distinct) std::future. By + // the time the main thread executes T3's request for B, B already + // exists, and is simply delivered via the future. - case CONSTRUCTED: - // still have to call initSingleton() - finishInitializing(); - break; + { // nested scope for 'lk' + // In case racing threads call getInstance() at the same moment, + // serialize the calls. + LockStatic lk; - case INITIALIZING: - // here if DERIVED_TYPE::initSingleton() (directly or indirectly) - // calls DERIVED_TYPE::getInstance(): go ahead and allow it - case INITIALIZED: - // normal subsequent calls - break; + switch (lk->mInitState) + { + case CONSTRUCTING: + // here if DERIVED_TYPE's constructor (directly or indirectly) + // calls DERIVED_TYPE::getInstance() + logerrs("Tried to access singleton ", + classname().c_str(), + " from singleton constructor!"); + return nullptr; - case DELETED: - // called after deleteSingleton() - logwarns("Trying to access deleted singleton ", - classname().c_str(), - " -- creating new instance"); - constructSingleton(); - finishInitializing(); - break; - } + case INITIALIZING: + // here if DERIVED_TYPE::initSingleton() (directly or indirectly) + // calls DERIVED_TYPE::getInstance(): go ahead and allow it + case INITIALIZED: + // normal subsequent calls + // record the dependency, if any: check if we got here from another + // LLSingleton's constructor or initSingleton() method + capture_dependency(lk->mInstance); + return lk->mInstance; - // record the dependency, if any: check if we got here from another - // LLSingleton's constructor or initSingleton() method - capture_dependency(); - return sData.mInstance; + case DELETED: + // called after deleteSingleton() + logwarns("Trying to access deleted singleton ", + classname().c_str(), + " -- creating new instance"); + // fall through + case UNINITIALIZED: + case QUEUED: + // QUEUED means some secondary thread has already requested an + // instance, but for present purposes that's semantically + // identical to UNINITIALIZED: either way, we must ourselves + // request an instance. + break; + } + + // Here we need to construct a new instance. + if (on_main_thread()) + { + // On the main thread, directly construct the instance while + // holding the lock. + constructSingleton(lk); + capture_dependency(lk->mInstance); + return lk->mInstance; + } + + // Here we need to construct a new instance, but we're on a secondary + // thread. + lk->mInitState = QUEUED; + } // unlock 'lk' + + // Per the comment block above, dispatch to the main thread. + loginfos(classname().c_str(), + "::getInstance() dispatching to main thread"); + auto instance = LLMainThreadTask::dispatch( + [](){ + // VERY IMPORTANT to call getInstance() on the main thread, + // rather than going straight to constructSingleton()! + // During the time window before mInitState is INITIALIZED, + // multiple requests might be queued. It's essential that, as + // the main thread processes them, only the FIRST such request + // actually constructs the instance -- every subsequent one + // simply returns the existing instance. + loginfos(classname().c_str(), + "::getInstance() on main thread"); + return getInstance(); + }); + // record the dependency chain tracked on THIS thread, not the main + // thread (consider a getInstance() overload with a tag param that + // suppresses dep tracking when dispatched to the main thread) + capture_dependency(instance); + loginfos(classname().c_str(), + "::getInstance() returning on requesting thread"); + return instance; } // Reference version of getInstance() - // Preferred over getInstance() as it disallows checking for NULL + // Preferred over getInstance() as it disallows checking for nullptr static DERIVED_TYPE& instance() { return *getInstance(); @@ -544,8 +639,8 @@ public: static bool instanceExists() { // defend any access to sData from racing threads - Locker lk; - return sData.mInitState == INITIALIZED; + LockStatic lk; + return lk->mInitState == INITIALIZED; } // Has this singleton been deleted? This can be useful during shutdown @@ -554,24 +649,11 @@ public: static bool wasDeleted() { // defend any access to sData from racing threads - Locker lk; - return sData.mInitState == DELETED; + LockStatic lk; + return lk->mInitState == DELETED; } - -private: - struct SingletonData - { - // explicitly has a default constructor so that member variables are zero initialized in BSS - // and only changed by singleton logic, not constructor running during startup - EInitState mInitState; - DERIVED_TYPE* mInstance; - }; - static SingletonData sData; }; -template -typename LLSingleton::SingletonData LLSingleton::sData; - /** * LLParamSingleton is like LLSingleton, except in the following ways: @@ -596,44 +678,86 @@ class LLParamSingleton : public LLSingleton { private: typedef LLSingleton super; - using typename super::Locker; + using typename super::LockStatic; + + // Passes arguments to DERIVED_TYPE's constructor and sets appropriate + // states, returning a pointer to the new instance. + template + static DERIVED_TYPE* initParamSingleton_(Args&&... args) + { + // In case racing threads both call initParamSingleton() at the same + // time, serialize them. One should initialize; the other should see + // mInitState already set. + LockStatic lk; + // For organizational purposes this function shouldn't be called twice + if (lk->mInitState != super::UNINITIALIZED) + { + super::logerrs("Tried to initialize singleton ", + super::template classname().c_str(), + " twice!"); + return nullptr; + } + else if (on_main_thread()) + { + // on the main thread, simply construct instance while holding lock + super::logdebugs(super::template classname().c_str(), + "::initParamSingleton()"); + super::constructSingleton(lk, std::forward(args)...); + return lk->mInstance; + } + else + { + // on secondary thread, dispatch to main thread -- + // set state so we catch any other calls before the main thread + // picks up the task + lk->mInitState = super::QUEUED; + // very important to unlock here so main thread can actually process + lk.unlock(); + super::loginfos(super::template classname().c_str(), + "::initParamSingleton() dispatching to main thread"); + // Normally it would be the height of folly to reference-bind + // 'args' into a lambda to be executed on some other thread! By + // the time that thread executed the lambda, the references would + // all be dangling, and Bad Things would result. But + // LLMainThreadTask::dispatch() promises to block until the passed + // task has completed. So in this case we know the references will + // remain valid until the lambda has run, so we dare to bind + // references. + auto instance = LLMainThreadTask::dispatch( + [&](){ + super::loginfos(super::template classname().c_str(), + "::initParamSingleton() on main thread"); + return initParamSingleton_(std::forward(args)...); + }); + super::loginfos(super::template classname().c_str(), + "::initParamSingleton() returning on requesting thread"); + return instance; + } + } public: using super::deleteSingleton; using super::instanceExists; using super::wasDeleted; - // Passes arguments to DERIVED_TYPE's constructor and sets appropriate states + /// initParamSingleton() constructs the instance, returning a reference. + /// Pass whatever arguments are required to construct DERIVED_TYPE. template - static void initParamSingleton(Args&&... args) + static DERIVED_TYPE& initParamSingleton(Args&&... args) { - // In case racing threads both call initParamSingleton() at the same - // time, serialize them. One should initialize; the other should see - // mInitState already set. - Locker lk; - // For organizational purposes this function shouldn't be called twice - if (super::sData.mInitState != super::UNINITIALIZED) - { - super::logerrs("Tried to initialize singleton ", - super::template classname().c_str(), - " twice!"); - } - else - { - super::constructSingleton(std::forward(args)...); - super::finishInitializing(); - } + return *initParamSingleton_(std::forward(args)...); } static DERIVED_TYPE* getInstance() { // In case racing threads call getInstance() at the same moment as // initParamSingleton(), serialize the calls. - Locker lk; + LockStatic lk; - switch (super::sData.mInitState) + switch (lk->mInitState) { case super::UNINITIALIZED: + case super::QUEUED: super::logerrs("Uninitialized param singleton ", super::template classname().c_str()); break; @@ -644,25 +768,13 @@ public: " from singleton constructor!"); break; - case super::CONSTRUCTED: - // Should never happen!? The CONSTRUCTED state is specifically to - // navigate through LLSingleton::SingletonInitializer getting - // constructed (once) before LLSingleton::getInstance()'s switch - // on mInitState. But our initParamSingleton() method calls - // constructSingleton() and then calls finishInitializing(), which - // immediately sets INITIALIZING. Why are we here? - super::logerrs("Param singleton ", - super::template classname().c_str(), - "::initSingleton() not yet called"); - break; - case super::INITIALIZING: // As with LLSingleton, explicitly permit circular calls from // within initSingleton() case super::INITIALIZED: // for any valid call, capture dependencies - super::capture_dependency(); - return super::sData.mInstance; + super::capture_dependency(lk->mInstance); + return lk->mInstance; case super::DELETED: super::logerrs("Trying to access deleted param singleton ", @@ -706,9 +818,9 @@ public: using super::instanceExists; using super::wasDeleted; - static void construct() + static DT* construct() { - super::initParamSingleton(); + return super::initParamSingleton(); } }; diff --git a/indra/llcommon/llthread.cpp b/indra/llcommon/llthread.cpp index a4171729db..0b9dec969c 100644 --- a/indra/llcommon/llthread.cpp +++ b/indra/llcommon/llthread.cpp @@ -92,26 +92,39 @@ void set_thread_name( DWORD dwThreadID, const char* threadName) // } // //---------------------------------------------------------------------------- +namespace +{ -U32 LL_THREAD_LOCAL sThreadID = 0; + LLThread::id_t main_thread() + { + // Using a function-static variable to identify the main thread + // requires that control reach here from the main thread before it + // reaches here from any other thread. We simply trust that whichever + // thread gets here first is the main thread. + static LLThread::id_t s_thread_id = LLThread::currentID(); + return s_thread_id; + } -U32 LLThread::sIDIter = 0; +} // anonymous namespace +LL_COMMON_API bool on_main_thread() +{ + return (LLThread::currentID() == main_thread()); +} LL_COMMON_API void assert_main_thread() { - static U32 s_thread_id = LLThread::currentID(); - if (LLThread::currentID() != s_thread_id) + auto curr = LLThread::currentID(); + auto main = main_thread(); + if (curr != main) { - LL_WARNS() << "Illegal execution from thread id " << (S32) LLThread::currentID() - << " outside main thread " << (S32) s_thread_id << LL_ENDL; + LL_WARNS() << "Illegal execution from thread id " << curr + << " outside main thread " << main << LL_ENDL; } } -void LLThread::registerThreadID() -{ - sThreadID = ++sIDIter; -} +// this function has become moot +void LLThread::registerThreadID() {} // // Handed to the APR thread creation function @@ -122,11 +135,12 @@ void LLThread::threadRun() set_thread_name(-1, mName.c_str()); #endif + // this is the first point at which we're actually running in the new thread + mID = currentID(); + // for now, hard code all LLThreads to report to single master thread recorder, which is known to be running on main thread mRecorder = new LLTrace::ThreadRecorder(*LLTrace::get_master_thread_recorder()); - sThreadID = mID; - // Run the user supplied function do { @@ -168,8 +182,6 @@ LLThread::LLThread(const std::string& name, apr_pool_t *poolp) : mStatus(STOPPED), mRecorder(NULL) { - - mID = ++sIDIter; mRunCondition = new LLCondition(); mDataLock = new LLMutex(); mLocalAPRFilePoolp = NULL ; @@ -347,9 +359,9 @@ void LLThread::setQuitting() } // static -U32 LLThread::currentID() +LLThread::id_t LLThread::currentID() { - return sThreadID; + return std::this_thread::get_id(); } // static @@ -376,6 +388,16 @@ void LLThread::wakeLocked() } } +void LLThread::lockData() +{ + mDataLock->lock(); +} + +void LLThread::unlockData() +{ + mDataLock->unlock(); +} + //============================================================================ //---------------------------------------------------------------------------- diff --git a/indra/llcommon/llthread.h b/indra/llcommon/llthread.h index 863c9051f3..5cd0731f6c 100644 --- a/indra/llcommon/llthread.h +++ b/indra/llcommon/llthread.h @@ -30,12 +30,9 @@ #include "llapp.h" #include "llapr.h" #include "boost/intrusive_ptr.hpp" -#include "llmutex.h" #include "llrefcount.h" #include -LL_COMMON_API void assert_main_thread(); - namespace LLTrace { class ThreadRecorder; @@ -45,7 +42,6 @@ class LL_COMMON_API LLThread { private: friend class LLMutex; - static U32 sIDIter; public: typedef enum e_thread_status @@ -55,6 +51,7 @@ public: QUITTING= 2, // Someone wants this thread to quit CRASHED = -1 // An uncaught exception was thrown by the thread } EThreadStatus; + typedef std::thread::id id_t; LLThread(const std::string& name, apr_pool_t *poolp = NULL); virtual ~LLThread(); // Warning! You almost NEVER want to destroy a thread unless it's in the STOPPED state. @@ -64,7 +61,7 @@ public: bool isStopped() const { return (STOPPED == mStatus) || (CRASHED == mStatus); } bool isCrashed() const { return (CRASHED == mStatus); } - static U32 currentID(); // Return ID of current thread + static id_t currentID(); // Return ID of current thread static void yield(); // Static because it can be called by the main thread, which doesn't have an LLThread data structure. public: @@ -88,7 +85,7 @@ public: LLVolatileAPRPool* getLocalAPRFilePool() { return mLocalAPRFilePoolp ; } - U32 getID() const { return mID; } + id_t getID() const { return mID; } // Called by threads *not* created via LLThread to register some // internal state used by LLMutex. You must call this once early @@ -109,7 +106,7 @@ protected: std::thread *mThreadp; EThreadStatus mStatus; - U32 mID; + id_t mID; LLTrace::ThreadRecorder* mRecorder; //a local apr_pool for APRFile operations in this thread. If it exists, LLAPRFile::sAPRFilePoolp should not be used. @@ -126,8 +123,8 @@ protected: virtual bool runCondition(void); // Lock/Unlock Run Condition -- use around modification of any variable used in runCondition() - inline void lockData(); - inline void unlockData(); + void lockData(); + void unlockData(); // This is the predicate that decides whether the thread should sleep. // It should only be called with mDataLock locked, since the virtual runCondition() function may need to access @@ -142,17 +139,6 @@ protected: }; -void LLThread::lockData() -{ - mDataLock->lock(); -} - -void LLThread::unlockData() -{ - mDataLock->unlock(); -} - - //============================================================================ // Simple responder for self destructing callbacks @@ -168,5 +154,6 @@ public: //============================================================================ extern LL_COMMON_API void assert_main_thread(); +extern LL_COMMON_API bool on_main_thread(); #endif // LL_LLTHREAD_H diff --git a/indra/llcommon/llthreadlocalstorage.cpp b/indra/llcommon/llthreadlocalstorage.cpp index 8cef05caac..d8a063e8d5 100644 --- a/indra/llcommon/llthreadlocalstorage.cpp +++ b/indra/llcommon/llthreadlocalstorage.cpp @@ -93,11 +93,9 @@ void LLThreadLocalPointerBase::initAllThreadLocalStorage() { if (!sInitialized) { - for (LLInstanceTracker::instance_iter it = beginInstances(), end_it = endInstances(); - it != end_it; - ++it) + for (auto& base : instance_snapshot()) { - (*it).initStorage(); + base.initStorage(); } sInitialized = true; } @@ -108,11 +106,9 @@ void LLThreadLocalPointerBase::destroyAllThreadLocalStorage() { if (sInitialized) { - //for (LLInstanceTracker::instance_iter it = beginInstances(), end_it = endInstances(); - // it != end_it; - // ++it) + //for (auto& base : instance_snapshot()) //{ - // (*it).destroyStorage(); + // base.destroyStorage(); //} sInitialized = false; } diff --git a/indra/llcommon/llthreadsafequeue.h b/indra/llcommon/llthreadsafequeue.h index 9e4548242b..6edffb4187 100644 --- a/indra/llcommon/llthreadsafequeue.h +++ b/indra/llcommon/llthreadsafequeue.h @@ -30,19 +30,9 @@ #include "llexception.h" #include #include - -#if LL_WINDOWS -#pragma warning (push) -#pragma warning (disable:4265) -#endif -// 'std::_Pad' : class has virtual functions, but destructor is not virtual -#include // std::unique_lock +#include "mutex.h" // std::unique_lock #include -#if LL_WINDOWS -#pragma warning (pop) -#endif - // // A general queue exception. // @@ -88,7 +78,7 @@ public: // Add an element to the front of queue (will block if the queue has // reached capacity). // - // This call will raise an interrupt error if the queue is deleted while + // This call will raise an interrupt error if the queue is closed while // the caller is blocked. void pushFront(ElementT const & element); @@ -99,7 +89,7 @@ public: // Pop the element at the end of the queue (will block if the queue is // empty). // - // This call will raise an interrupt error if the queue is deleted while + // This call will raise an interrupt error if the queue is closed while // the caller is blocked. ElementT popBack(void); @@ -110,11 +100,27 @@ public: // Returns the size of the queue. size_t size(); + // closes the queue: + // - every subsequent pushFront() call will throw LLThreadSafeQueueInterrupt + // - every subsequent tryPushFront() call will return false + // - popBack() calls will return normally until the queue is drained, then + // every subsequent popBack() will throw LLThreadSafeQueueInterrupt + // - tryPopBack() calls will return normally until the queue is drained, + // then every subsequent tryPopBack() call will return false + void close(); + + // detect closed state + bool isClosed(); + // inverse of isClosed() + explicit operator bool(); + private: std::deque< ElementT > mStorage; U32 mCapacity; + bool mClosed; boost::fibers::mutex mLock; + typedef std::unique_lock lock_t; boost::fibers::condition_variable mCapacityCond; boost::fibers::condition_variable mEmptyCond; }; @@ -124,7 +130,8 @@ private: template LLThreadSafeQueue::LLThreadSafeQueue(U32 capacity) : -mCapacity(capacity) + mCapacity(capacity), + mClosed(false) { } @@ -132,12 +139,18 @@ mCapacity(capacity) template void LLThreadSafeQueue::pushFront(ElementT const & element) { - std::unique_lock lock1(mLock); + lock_t lock1(mLock); while (true) { + if (mClosed) + { + LLTHROW(LLThreadSafeQueueInterrupt()); + } + if (mStorage.size() < mCapacity) { mStorage.push_front(element); + lock1.unlock(); mEmptyCond.notify_one(); return; } @@ -151,14 +164,18 @@ void LLThreadSafeQueue::pushFront(ElementT const & element) template bool LLThreadSafeQueue::tryPushFront(ElementT const & element) { - std::unique_lock lock1(mLock, std::defer_lock); + lock_t lock1(mLock, std::defer_lock); if (!lock1.try_lock()) return false; + if (mClosed) + return false; + if (mStorage.size() >= mCapacity) return false; mStorage.push_front(element); + lock1.unlock(); mEmptyCond.notify_one(); return true; } @@ -167,17 +184,23 @@ bool LLThreadSafeQueue::tryPushFront(ElementT const & element) template ElementT LLThreadSafeQueue::popBack(void) { - std::unique_lock lock1(mLock); + lock_t lock1(mLock); while (true) { if (!mStorage.empty()) { ElementT value = mStorage.back(); mStorage.pop_back(); + lock1.unlock(); mCapacityCond.notify_one(); return value; } + if (mClosed) + { + LLTHROW(LLThreadSafeQueueInterrupt()); + } + // Storage empty. Wait for signal. mEmptyCond.wait(lock1); } @@ -187,15 +210,18 @@ ElementT LLThreadSafeQueue::popBack(void) template bool LLThreadSafeQueue::tryPopBack(ElementT & element) { - std::unique_lock lock1(mLock, std::defer_lock); + lock_t lock1(mLock, std::defer_lock); if (!lock1.try_lock()) return false; + // no need to check mClosed: tryPopBack() behavior when the queue is + // closed is implemented by simple inability to push any new elements if (mStorage.empty()) return false; element = mStorage.back(); mStorage.pop_back(); + lock1.unlock(); mCapacityCond.notify_one(); return true; } @@ -204,8 +230,34 @@ bool LLThreadSafeQueue::tryPopBack(ElementT & element) template size_t LLThreadSafeQueue::size(void) { - std::lock_guard lock(mLock); + lock_t lock(mLock); return mStorage.size(); } +template +void LLThreadSafeQueue::close() +{ + lock_t lock(mLock); + mClosed = true; + lock.unlock(); + // wake up any blocked popBack() calls + mEmptyCond.notify_all(); + // wake up any blocked pushFront() calls + mCapacityCond.notify_all(); +} + +template +bool LLThreadSafeQueue::isClosed() +{ + lock_t lock(mLock); + return mClosed; +} + +template +LLThreadSafeQueue::operator bool() +{ + lock_t lock(mLock); + return ! mClosed; +} + #endif diff --git a/indra/llcommon/lltrace.h b/indra/llcommon/lltrace.h index ba233d5727..9220a41bcd 100644 --- a/indra/llcommon/lltrace.h +++ b/indra/llcommon/lltrace.h @@ -57,7 +57,7 @@ class StatBase { public: StatBase(const char* name, const char* description); - virtual ~StatBase() LLINSTANCETRACKER_DTOR_NOEXCEPT {} + virtual ~StatBase() {} virtual const char* getUnitLabel() const; const std::string& getName() const { return mName; } diff --git a/indra/llcommon/lltracethreadrecorder.cpp b/indra/llcommon/lltracethreadrecorder.cpp index 181fc2f058..025dc57044 100644 --- a/indra/llcommon/lltracethreadrecorder.cpp +++ b/indra/llcommon/lltracethreadrecorder.cpp @@ -28,6 +28,7 @@ #include "lltracethreadrecorder.h" #include "llfasttimer.h" #include "lltrace.h" +#include "llstl.h" namespace LLTrace { @@ -64,16 +65,15 @@ void ThreadRecorder::init() activate(&mThreadRecordingBuffers); // initialize time block parent pointers - for (BlockTimerStatHandle::instance_tracker_t::instance_iter it = BlockTimerStatHandle::instance_tracker_t::beginInstances(), end_it = BlockTimerStatHandle::instance_tracker_t::endInstances(); - it != end_it; - ++it) + for (auto& base : BlockTimerStatHandle::instance_snapshot()) { - BlockTimerStatHandle& time_block = static_cast(*it); - TimeBlockTreeNode& tree_node = mTimeBlockTreeNodes[it->getIndex()]; + // because of indirect derivation from LLInstanceTracker, have to downcast + BlockTimerStatHandle& time_block = static_cast(base); + TimeBlockTreeNode& tree_node = mTimeBlockTreeNodes[time_block.getIndex()]; tree_node.mBlock = &time_block; tree_node.mParent = &root_time_block; - it->getCurrentAccumulator().mParent = &root_time_block; + time_block.getCurrentAccumulator().mParent = &root_time_block; } mRootTimer = new BlockTimer(root_time_block); diff --git a/indra/llcommon/lluuid.cpp b/indra/llcommon/lluuid.cpp index 7c6da1cc0b..bcff0a8501 100644 --- a/indra/llcommon/lluuid.cpp +++ b/indra/llcommon/lluuid.cpp @@ -43,6 +43,7 @@ #include "llstring.h" #include "lltimer.h" #include "llthread.h" +#include "llmutex.h" const LLUUID LLUUID::null; const LLTransactionID LLTransactionID::tnull; @@ -739,7 +740,7 @@ void LLUUID::getCurrentTime(uuid_time_t *timestamp) getSystemTime(&time_last); uuids_this_tick = uuids_per_tick; init = TRUE; - mMutex = new LLMutex(); + mMutex = new LLMutex(); } uuid_time_t time_now = {0,0}; diff --git a/indra/llcommon/llworkerthread.h b/indra/llcommon/llworkerthread.h index b1a6f61360..0387e75c65 100644 --- a/indra/llcommon/llworkerthread.h +++ b/indra/llcommon/llworkerthread.h @@ -34,6 +34,7 @@ #include "llqueuedthread.h" #include "llatomic.h" +#include "llmutex.h" #define USE_FRAME_CALLBACK_MANAGER 0 diff --git a/indra/llcommon/lockstatic.h b/indra/llcommon/lockstatic.h new file mode 100644 index 0000000000..96c53c6473 --- /dev/null +++ b/indra/llcommon/lockstatic.h @@ -0,0 +1,73 @@ +/** + * @file lockstatic.h + * @author Nat Goodspeed + * @date 2019-12-03 + * @brief LockStatic class provides mutex-guarded access to the specified + * static data. + * + * $LicenseInfo:firstyear=2019&license=viewerlgpl$ + * Copyright (c) 2019, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_LOCKSTATIC_H) +#define LL_LOCKSTATIC_H + +#include "mutex.h" // std::unique_lock + +namespace llthread +{ + +// Instantiate this template to obtain a pointer to the canonical static +// instance of Static while holding a lock on that instance. Use of +// Static::mMutex presumes that Static declares some suitable mMutex. +template +class LockStatic +{ + typedef std::unique_lock lock_t; +public: + LockStatic(): + mData(getStatic()), + mLock(mData->mMutex) + {} + Static* get() const { return mData; } + operator Static*() const { return get(); } + Static* operator->() const { return get(); } + // sometimes we must explicitly unlock... + void unlock() + { + // but once we do, access is no longer permitted + mData = nullptr; + mLock.unlock(); + } +protected: + Static* mData; + lock_t mLock; +private: + Static* getStatic() + { + // Static::mMutex must be function-local static rather than class- + // static. Some of our consumers must function properly (therefore + // lock properly) even when the containing module's static variables + // have not yet been runtime-initialized. A mutex requires + // construction. A static class member might not yet have been + // constructed. + // + // We could store a dumb mutex_t*, notice when it's NULL and allocate a + // heap mutex -- but that's vulnerable to race conditions. And we can't + // defend the dumb pointer with another mutex. + // + // We could store a std::atomic -- but a default-constructed + // std::atomic does not contain a valid T, even a default-constructed + // T! Which means std::atomic, too, requires runtime initialization. + // + // But a function-local static is guaranteed to be initialized exactly + // once: the first time control reaches that declaration. + static Static sData; + return &sData; + } +}; + +} // llthread namespace + +#endif /* ! defined(LL_LOCKSTATIC_H) */ diff --git a/indra/llcommon/mutex.h b/indra/llcommon/mutex.h new file mode 100644 index 0000000000..90d0942270 --- /dev/null +++ b/indra/llcommon/mutex.h @@ -0,0 +1,22 @@ +/** + * @file mutex.h + * @author Nat Goodspeed + * @date 2019-12-03 + * @brief Wrap in odious boilerplate + * + * $LicenseInfo:firstyear=2019&license=viewerlgpl$ + * Copyright (c) 2019, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if LL_WINDOWS +#pragma warning (push) +#pragma warning (disable:4265) +#endif +// warning C4265: 'std::_Pad' : class has virtual functions, but destructor is not virtual + +#include + +#if LL_WINDOWS +#pragma warning (pop) +#endif diff --git a/indra/llcommon/tests/llinstancetracker_test.cpp b/indra/llcommon/tests/llinstancetracker_test.cpp index 7b3ccf9a14..9b89159625 100644 --- a/indra/llcommon/tests/llinstancetracker_test.cpp +++ b/indra/llcommon/tests/llinstancetracker_test.cpp @@ -41,7 +41,6 @@ #include // other Linden headers #include "../test/lltut.h" -#include "wrapllerrs.h" struct Badness: public std::runtime_error { @@ -112,24 +111,22 @@ namespace tut void object::test<2>() { ensure_equals(Unkeyed::instanceCount(), 0); - Unkeyed* dangling = NULL; + std::weak_ptr dangling; { Unkeyed one; ensure_equals(Unkeyed::instanceCount(), 1); - Unkeyed* found = Unkeyed::getInstance(&one); - ensure_equals(found, &one); + std::weak_ptr found = one.getWeak(); + ensure(! found.expired()); { boost::scoped_ptr two(new Unkeyed); ensure_equals(Unkeyed::instanceCount(), 2); - Unkeyed* found = Unkeyed::getInstance(two.get()); - ensure_equals(found, two.get()); } ensure_equals(Unkeyed::instanceCount(), 1); - // store an unwise pointer to a temp Unkeyed instance - dangling = &one; + // store a weak pointer to a temp Unkeyed instance + dangling = found; } // make that instance vanish // check the now-invalid pointer to the destroyed instance - ensure("getInstance(T*) failed to track destruction", ! Unkeyed::getInstance(dangling)); + ensure("weak_ptr failed to track destruction", dangling.expired()); ensure_equals(Unkeyed::instanceCount(), 0); } @@ -142,7 +139,8 @@ namespace tut // reimplement LLInstanceTracker using, say, a hash map instead of a // std::map. We DO insist that every key appear exactly once. typedef std::vector StringVector; - StringVector keys(Keyed::beginKeys(), Keyed::endKeys()); + auto snap = Keyed::key_snapshot(); + StringVector keys(snap.begin(), snap.end()); std::sort(keys.begin(), keys.end()); StringVector::const_iterator ki(keys.begin()); ensure_equals(*ki++, "one"); @@ -153,17 +151,15 @@ namespace tut ensure("didn't reach end", ki == keys.end()); // Use a somewhat different approach to order independence with - // beginInstances(): explicitly capture the instances we know in a + // instance_snapshot(): explicitly capture the instances we know in a // set, and delete them as we iterate through. typedef std::set InstanceSet; InstanceSet instances; instances.insert(&one); instances.insert(&two); instances.insert(&three); - for (Keyed::instance_iter ii(Keyed::beginInstances()), iend(Keyed::endInstances()); - ii != iend; ++ii) + for (auto& ref : Keyed::instance_snapshot()) { - Keyed& ref = *ii; ensure_equals("spurious instance", instances.erase(&ref), 1); } ensure_equals("unreported instance", instances.size(), 0); @@ -180,63 +176,62 @@ namespace tut instances.insert(&two); instances.insert(&three); - for (Unkeyed::instance_iter ii(Unkeyed::beginInstances()), iend(Unkeyed::endInstances()); ii != iend; ++ii) - { - Unkeyed& ref = *ii; - ensure_equals("spurious instance", instances.erase(&ref), 1); - } + for (auto& ref : Unkeyed::instance_snapshot()) + { + ensure_equals("spurious instance", instances.erase(&ref), 1); + } ensure_equals("unreported instance", instances.size(), 0); } - /* + template<> template<> void object::test<5>() { - set_test_name("delete Keyed with outstanding instance_iter"); - std::string what; - Keyed* keyed = new Keyed("delete Keyed with outstanding instance_iter"); - { - WrapLLErrs wrapper; - Keyed::instance_iter i(Keyed::beginInstances()); - what = wrapper.catch_llerrs([&keyed](){ - delete keyed; - }); - } - ensure(! what.empty()); + std::string desc("delete Keyed with outstanding instance_snapshot"); + set_test_name(desc); + Keyed* keyed = new Keyed(desc); + // capture a snapshot but do not yet traverse it + auto snapshot = Keyed::instance_snapshot(); + // delete the one instance + delete keyed; + // traversing the snapshot should reflect the deletion + // avoid ensure_equals() because it requires the ability to stream the + // two values to std::ostream + ensure(snapshot.begin() == snapshot.end()); } - + template<> template<> void object::test<6>() { - set_test_name("delete Keyed with outstanding key_iter"); - std::string what; - Keyed* keyed = new Keyed("delete Keyed with outstanding key_it"); - { - WrapLLErrs wrapper; - Keyed::key_iter i(Keyed::beginKeys()); - what = wrapper.catch_llerrs([&keyed](){ - delete keyed; - }); - } - ensure(! what.empty()); + std::string desc("delete Keyed with outstanding key_snapshot"); + set_test_name(desc); + Keyed* keyed = new Keyed(desc); + // capture a snapshot but do not yet traverse it + auto snapshot = Keyed::key_snapshot(); + // delete the one instance + delete keyed; + // traversing the snapshot should reflect the deletion + // avoid ensure_equals() because it requires the ability to stream the + // two values to std::ostream + ensure(snapshot.begin() == snapshot.end()); } template<> template<> void object::test<7>() { - set_test_name("delete Unkeyed with outstanding instance_iter"); + set_test_name("delete Unkeyed with outstanding instance_snapshot"); std::string what; Unkeyed* unkeyed = new Unkeyed; - { - WrapLLErrs wrapper; - Unkeyed::instance_iter i(Unkeyed::beginInstances()); - what = wrapper.catch_llerrs([&unkeyed](){ - delete unkeyed; - }); - } - ensure(! what.empty()); + // capture a snapshot but do not yet traverse it + auto snapshot = Unkeyed::instance_snapshot(); + // delete the one instance + delete unkeyed; + // traversing the snapshot should reflect the deletion + // avoid ensure_equals() because it requires the ability to stream the + // two values to std::ostream + ensure(snapshot.begin() == snapshot.end()); } - + template<> template<> void object::test<8>() { @@ -246,11 +241,9 @@ namespace tut // We can't use the iterator-range InstanceSet constructor because // beginInstances() returns an iterator that dereferences to an // Unkeyed&, not an Unkeyed*. - for (Unkeyed::instance_iter uki(Unkeyed::beginInstances()), - ukend(Unkeyed::endInstances()); - uki != ukend; ++uki) + for (auto& ref : Unkeyed::instance_snapshot()) { - existing.insert(&*uki); + existing.insert(&ref); } try { @@ -273,11 +266,9 @@ namespace tut // instances was also present in the original set. If that's not true, // it's because our new Unkeyed ended up in the updated set despite // its constructor exception. - for (Unkeyed::instance_iter uki(Unkeyed::beginInstances()), - ukend(Unkeyed::endInstances()); - uki != ukend; ++uki) + for (auto& ref : Unkeyed::instance_snapshot()) { - ensure("failed to remove instance", existing.find(&*uki) != existing.end()); + ensure("failed to remove instance", existing.find(&ref) != existing.end()); } - }*/ + } } // namespace tut diff --git a/indra/llcommon/tests/llleap_test.cpp b/indra/llcommon/tests/llleap_test.cpp index bf0a74d10d..9d71e327d8 100644 --- a/indra/llcommon/tests/llleap_test.cpp +++ b/indra/llcommon/tests/llleap_test.cpp @@ -49,24 +49,28 @@ const size_t BUFFERED_LENGTH = 1023*1024; // try wrangling just under a megabyte #endif -void waitfor(const std::vector& instances, int timeout=60) +// capture std::weak_ptrs to LLLeap instances so we can tell when they expire +typedef std::vector> LLLeapVector; + +void waitfor(const LLLeapVector& instances, int timeout=60) { int i; for (i = 0; i < timeout; ++i) { // Every iteration, test whether any of the passed LLLeap instances // still exist (are still running). - std::vector::const_iterator vli(instances.begin()), vlend(instances.end()); - for ( ; vli != vlend; ++vli) + bool found = false; + for (auto& ptr : instances) { - // getInstance() returns NULL if it's terminated/gone, non-NULL if - // it's still running - if (LLLeap::getInstance(*vli)) + if (! ptr.expired()) + { + found = true; break; + } } // If we made it through all of 'instances' without finding one that's // still running, we're done. - if (vli == vlend) + if (! found) { /*==========================================================================*| std::cout << instances.size() << " LLLeap instances terminated in " @@ -86,8 +90,8 @@ void waitfor(const std::vector& instances, int timeout=60) void waitfor(LLLeap* instance, int timeout=60) { - std::vector instances; - instances.push_back(instance); + LLLeapVector instances; + instances.push_back(instance->getWeak()); waitfor(instances, timeout); } @@ -218,11 +222,11 @@ namespace tut NamedTempFile script("py", "import time\n" "time.sleep(1)\n"); - std::vector instances; + LLLeapVector instances; instances.push_back(LLLeap::create(get_test_name(), - sv(list_of(PYTHON)(script.getName())))); + sv(list_of(PYTHON)(script.getName())))->getWeak()); instances.push_back(LLLeap::create(get_test_name(), - sv(list_of(PYTHON)(script.getName())))); + sv(list_of(PYTHON)(script.getName())))->getWeak()); // In this case we're simply establishing that two LLLeap instances // can coexist without throwing exceptions or bombing in any other // way. Wait for them to terminate. diff --git a/indra/llcommon/tests/llmainthreadtask_test.cpp b/indra/llcommon/tests/llmainthreadtask_test.cpp new file mode 100644 index 0000000000..8178aa629a --- /dev/null +++ b/indra/llcommon/tests/llmainthreadtask_test.cpp @@ -0,0 +1,136 @@ +/** + * @file llmainthreadtask_test.cpp + * @author Nat Goodspeed + * @date 2019-12-05 + * @brief Test for llmainthreadtask. + * + * $LicenseInfo:firstyear=2019&license=viewerlgpl$ + * Copyright (c) 2019, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "llmainthreadtask.h" +// STL headers +// std headers +#include +// external library headers +// other Linden headers +#include "../test/lltut.h" +#include "../test/sync.h" +#include "llthread.h" // on_main_thread() +#include "lleventtimer.h" +#include "lockstatic.h" + +/***************************************************************************** +* TUT +*****************************************************************************/ +namespace tut +{ + struct llmainthreadtask_data + { + // 2-second timeout + Sync mSync{F32Milliseconds(2000.0f)}; + + llmainthreadtask_data() + { + // we're not testing the result; this is just to cache the + // initial thread as the main thread. + on_main_thread(); + } + }; + typedef test_group llmainthreadtask_group; + typedef llmainthreadtask_group::object object; + llmainthreadtask_group llmainthreadtaskgrp("llmainthreadtask"); + + template<> template<> + void object::test<1>() + { + set_test_name("inline"); + bool ran = false; + bool result = LLMainThreadTask::dispatch( + [&ran]()->bool{ + ran = true; + return true; + }); + ensure("didn't run lambda", ran); + ensure("didn't return result", result); + } + + struct StaticData + { + std::mutex mMutex; // LockStatic looks for mMutex + bool ran{false}; + }; + typedef llthread::LockStatic LockStatic; + + template<> template<> + void object::test<2>() + { + set_test_name("cross-thread"); + std::atomic_bool result(false); + // wrapping our thread lambda in a packaged_task will catch any + // exceptions it might throw and deliver them via future + std::packaged_task thread_work( + [this, &result](){ + // unblock test<2>()'s yield_until(1) + mSync.set(1); + // dispatch work to main thread -- should block here + bool on_main( + LLMainThreadTask::dispatch( + []()->bool{ + // have to lock static mutex to set static data + LockStatic()->ran = true; + // indicate whether task was run on the main thread + return on_main_thread(); + })); + // wait for test<2>() to unblock us again + mSync.yield_until(3); + result = on_main; + }); + auto thread_result = thread_work.get_future(); + std::thread thread; + try + { + // run thread_work + thread = std::thread(std::move(thread_work)); + // wait for thread to set(1) + mSync.yield_until(1); + // try to acquire the lock, should block because thread has it + LockStatic lk; + // wake up when dispatch() unlocks the static mutex + ensure("shouldn't have run yet", !lk->ran); + ensure("shouldn't have returned yet", !result); + // unlock so the task can acquire the lock + lk.unlock(); + // run the task -- should unblock thread, which will immediately block + // on mSync + LLEventTimer::updateClass(); + // 'lk', having unlocked, can no longer be used to access; relock with + // a new LockStatic instance + ensure("should now have run", LockStatic()->ran); + ensure("returned too early", !result); + // okay, let thread perform the assignment + mSync.set(3); + } + catch (...) + { + // A test failure exception anywhere in the try block can cause + // the test program to terminate without explanation when + // ~thread() finds that 'thread' is still joinable. We could + // either join() or detach() it -- but since it might be blocked + // waiting for something from the main thread that now can never + // happen, it's safer to detach it. + thread.detach(); + throw; + } + // 'thread' should be all done now + thread.join(); + // deliver any exception thrown by thread_work + thread_result.get(); + ensure("ran changed", LockStatic()->ran); + ensure("didn't run on main thread", result); + } +} // namespace tut diff --git a/indra/llcorehttp/httpcommon.cpp b/indra/llcorehttp/httpcommon.cpp index 7c93c54cdf..e37a38b05f 100644 --- a/indra/llcorehttp/httpcommon.cpp +++ b/indra/llcorehttp/httpcommon.cpp @@ -40,6 +40,7 @@ #include #if SAFE_SSL #include +#include // std::hash #endif @@ -369,7 +370,8 @@ void ssl_locking_callback(int mode, int type, const char *file, int line) //static unsigned long ssl_thread_id(void) { - return LLThread::currentID(); + // std::thread::id is very deliberately opaque, but we can hash it + return std::hash()(LLThread::currentID()); } #endif diff --git a/indra/llkdu/llimagej2ckdu.cpp b/indra/llkdu/llimagej2ckdu.cpp index 4547d17363..dd0d693638 100644 --- a/indra/llkdu/llimagej2ckdu.cpp +++ b/indra/llkdu/llimagej2ckdu.cpp @@ -44,16 +44,19 @@ using namespace kdu_core; #include #include -// stream kdu_dims to std::ostream // Turns out this must NOT be in the anonymous namespace! -// It must also precede #include "stringize.h". +namespace kdu_core +{ +// stream kdu_dims to std::ostream inline std::ostream& operator<<(std::ostream& out, const kdu_dims& dims) { return out << "(" << dims.pos.x << "," << dims.pos.y << ")," "[" << dims.size.x << "x" << dims.size.y << "]"; } +} // namespace kdu_core +// operator<<(std::ostream&, const kdu_dims&) must precede #include "stringize.h" #include "stringize.h" namespace { diff --git a/indra/llmessage/llbuffer.cpp b/indra/llmessage/llbuffer.cpp index 1a0eceba0f..cfe38605ad 100644 --- a/indra/llmessage/llbuffer.cpp +++ b/indra/llmessage/llbuffer.cpp @@ -32,6 +32,7 @@ #include "llmath.h" #include "llstl.h" #include "llthread.h" +#include "llmutex.h" #include #define ASSERT_LLBUFFERARRAY_MUTEX_LOCKED() llassert(!mMutexp || mMutexp->isSelfLocked()) diff --git a/indra/llmessage/llbufferstream.cpp b/indra/llmessage/llbufferstream.cpp index ff1c9993cc..39508c1c52 100644 --- a/indra/llmessage/llbufferstream.cpp +++ b/indra/llmessage/llbufferstream.cpp @@ -31,6 +31,7 @@ #include "llbuffer.h" #include "llthread.h" +#include "llmutex.h" static const S32 DEFAULT_OUTPUT_SEGMENT_SIZE = 1024 * 4; diff --git a/indra/llmessage/llcoproceduremanager.cpp b/indra/llmessage/llcoproceduremanager.cpp index 8d9090a516..5597935500 100644 --- a/indra/llmessage/llcoproceduremanager.cpp +++ b/indra/llmessage/llcoproceduremanager.cpp @@ -291,7 +291,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. - pendingCoprocs->pushFront({}); + pendingCoprocs->close(); } return false; }); @@ -323,7 +323,7 @@ LLUUID LLCoprocedurePool::enqueueCoprocedure(const std::string &name, LLCoproced LL_INFOS("CoProcMgr") << "Coprocedure(" << name << ") enqueuing with id=" << id.asString() << " in pool \"" << mPoolName << "\" at " << mPending << LL_ENDL; auto pushed = mPendingCoprocs->tryPushFront(boost::make_shared(name, id, proc)); // We don't really have a lot of good options if tryPushFront() failed, - // perhaps because the consuming coroutine is gummed up or something. This + // perhaps because the consuming coroutines are gummed up or something. This // method is probably called from code called by mainloop. If we toss an // llcoro::suspend() call here, we'll circle back for another mainloop // iteration, possibly resulting in being re-entered here. Let's avoid that. @@ -341,13 +341,14 @@ void LLCoprocedurePool::coprocedureInvokerCoro( QueuedCoproc::ptr_t coproc; for (;;) { + try { LLCoros::TempStatus st("waiting for work"); coproc = pendingCoprocs->popBack(); } - if (! coproc) + catch (const LLThreadSafeQueueError&) { - // close() pushes an empty pointer to signal done + // queue is closed break; } @@ -369,7 +370,7 @@ void LLCoprocedurePool::coprocedureInvokerCoro( << ") in pool '" << mPoolName << "'")); // must NOT omit this or we deplete the pool mActiveCoprocs.erase(itActive); - throw; + continue; } // Nicky: This is super spammy. Consider using LL_DEBUGS here? @@ -381,5 +382,5 @@ void LLCoprocedurePool::coprocedureInvokerCoro( void LLCoprocedurePool::close() { - mPendingCoprocs->pushFront({}); + mPendingCoprocs->close(); } diff --git a/indra/llmessage/llproxy.h b/indra/llmessage/llproxy.h index 87891901ad..a1ffa9e5d5 100644 --- a/indra/llmessage/llproxy.h +++ b/indra/llmessage/llproxy.h @@ -32,6 +32,7 @@ #include "llmemory.h" #include "llsingleton.h" #include "llthread.h" +#include "llmutex.h" #include #include diff --git a/indra/llplugin/llpluginmessagepipe.h b/indra/llplugin/llpluginmessagepipe.h index c3498beac0..9d5835eb82 100644 --- a/indra/llplugin/llpluginmessagepipe.h +++ b/indra/llplugin/llpluginmessagepipe.h @@ -31,6 +31,7 @@ #include "lliosocket.h" #include "llthread.h" +#include "llmutex.h" class LLPluginMessagePipe; diff --git a/indra/llrender/llgl.cpp b/indra/llrender/llgl.cpp index 2a93f9c534..b9d05e16bf 100644 --- a/indra/llrender/llgl.cpp +++ b/indra/llrender/llgl.cpp @@ -2452,9 +2452,8 @@ void LLGLNamePool::release(GLuint name) //static void LLGLNamePool::upkeepPools() { - for (tracker_t::instance_iter iter = beginInstances(); iter != endInstances(); ++iter) + for (auto& pool : instance_snapshot()) { - LLGLNamePool & pool = *iter; pool.upkeep(); } } @@ -2462,9 +2461,8 @@ void LLGLNamePool::upkeepPools() //static void LLGLNamePool::cleanupPools() { - for (tracker_t::instance_iter iter = beginInstances(); iter != endInstances(); ++iter) + for (auto& pool : instance_snapshot()) { - LLGLNamePool & pool = *iter; pool.cleanup(); } } diff --git a/indra/llui/llconsole.cpp b/indra/llui/llconsole.cpp index b398ecf218..b8f2afbe74 100644 --- a/indra/llui/llconsole.cpp +++ b/indra/llui/llconsole.cpp @@ -723,9 +723,9 @@ void LLConsole::onUrlLabelCallback(const LLUUID& paragraph_id, const std::string // static void LLConsole::updateClass() { - for (instance_iter it = beginInstances(); it != endInstances(); ++it) + for (auto& con : instance_snapshot()) { - it->update(); + con.update(); } } diff --git a/indra/llui/lllayoutstack.cpp b/indra/llui/lllayoutstack.cpp index 7daf36395c..2c8a66145f 100644 --- a/indra/llui/lllayoutstack.cpp +++ b/indra/llui/lllayoutstack.cpp @@ -713,10 +713,10 @@ void LLLayoutStack::createResizeBar(LLLayoutPanel* panelp) //static void LLLayoutStack::updateClass() { - for (instance_iter it = beginInstances(); it != endInstances(); ++it) + for (auto& layout : instance_snapshot()) { - it->updateLayout(); - it->mAnimatedThisFrame = false; + layout.updateLayout(); + layout.mAnimatedThisFrame = false; } } diff --git a/indra/llui/llnotificationslistener.cpp b/indra/llui/llnotificationslistener.cpp index be26416cbb..e73ba1fbe9 100644 --- a/indra/llui/llnotificationslistener.cpp +++ b/indra/llui/llnotificationslistener.cpp @@ -127,18 +127,16 @@ void LLNotificationsListener::listChannels(const LLSD& params) const { LLReqID reqID(params); LLSD response(reqID.makeResponse()); - for (LLNotificationChannel::instance_iter cmi(LLNotificationChannel::beginInstances()), - cmend(LLNotificationChannel::endInstances()); - cmi != cmend; ++cmi) + for (auto& cm : LLNotificationChannel::instance_snapshot()) { LLSD channelInfo, parents; - BOOST_FOREACH(const std::string& parent, cmi->getParents()) + for (const std::string& parent : cm.getParents()) { parents.append(parent); } channelInfo["parents"] = parents; channelInfo["parent"] = parents.size()? parents[0] : ""; - response[cmi->getName()] = channelInfo; + response[cm.getName()] = channelInfo; } LLEventPumps::instance().obtain(params["reply"]).post(response); } diff --git a/indra/llvfs/llvfs.h b/indra/llvfs/llvfs.h index 4ca5273915..9258c77365 100644 --- a/indra/llvfs/llvfs.h +++ b/indra/llvfs/llvfs.h @@ -31,6 +31,7 @@ #include "lluuid.h" #include "llassettype.h" #include "llthread.h" +#include "llmutex.h" enum EVFSValid { diff --git a/indra/llwindow/llwindow.cpp b/indra/llwindow/llwindow.cpp index 5e6868ec0e..f43ca28c13 100644 --- a/indra/llwindow/llwindow.cpp +++ b/indra/llwindow/llwindow.cpp @@ -471,9 +471,8 @@ LLCoordCommon LL_COORD_TYPE_WINDOW::convertToCommon() const { const LLCoordWindow& self = LLCoordWindow::getTypedCoords(*this); - LLWindow* windowp = &(*LLWindow::beginInstances()); LLCoordGL out; - windowp->convertCoords(self, &out); + LLWindow::instance_snapshot().begin()->convertCoords(self, &out); return out.convert(); } @@ -481,18 +480,16 @@ void LL_COORD_TYPE_WINDOW::convertFromCommon(const LLCoordCommon& from) { LLCoordWindow& self = LLCoordWindow::getTypedCoords(*this); - LLWindow* windowp = &(*LLWindow::beginInstances()); LLCoordGL from_gl(from); - windowp->convertCoords(from_gl, &self); + LLWindow::instance_snapshot().begin()->convertCoords(from_gl, &self); } LLCoordCommon LL_COORD_TYPE_SCREEN::convertToCommon() const { const LLCoordScreen& self = LLCoordScreen::getTypedCoords(*this); - LLWindow* windowp = &(*LLWindow::beginInstances()); LLCoordGL out; - windowp->convertCoords(self, &out); + LLWindow::instance_snapshot().begin()->convertCoords(self, &out); return out.convert(); } @@ -500,7 +497,6 @@ void LL_COORD_TYPE_SCREEN::convertFromCommon(const LLCoordCommon& from) { LLCoordScreen& self = LLCoordScreen::getTypedCoords(*this); - LLWindow* windowp = &(*LLWindow::beginInstances()); LLCoordGL from_gl(from); - windowp->convertCoords(from_gl, &self); + LLWindow::instance_snapshot().begin()->convertCoords(from_gl, &self); } diff --git a/indra/llxml/llcontrol.h b/indra/llxml/llcontrol.h index d9b354ed76..e5d8414d3c 100644 --- a/indra/llxml/llcontrol.h +++ b/indra/llxml/llcontrol.h @@ -240,8 +240,6 @@ public: LLControlGroup(const std::string& name); ~LLControlGroup(); void cleanup(); - - typedef LLInstanceTracker::instance_iter instance_iter; LLControlVariablePtr getControl(const std::string& name); diff --git a/indra/media_plugins/cef/windows_volume_catcher.cpp b/indra/media_plugins/cef/windows_volume_catcher.cpp index 6953ad3ab8..7a36123a11 100644 --- a/indra/media_plugins/cef/windows_volume_catcher.cpp +++ b/indra/media_plugins/cef/windows_volume_catcher.cpp @@ -27,8 +27,9 @@ */ #include "volume_catcher.h" -#include #include "llsingleton.h" +#include +#include class VolumeCatcherImpl : public LLSingleton { LLSINGLETON(VolumeCatcherImpl); diff --git a/indra/newview/fschathistory.cpp b/indra/newview/fschathistory.cpp index 93d7526116..50b15a82e0 100644 --- a/indra/newview/fschathistory.cpp +++ b/indra/newview/fschathistory.cpp @@ -1562,14 +1562,12 @@ void FSChatHistory::appendMessage(const LLChat& chat, const LLSD &args, const LL // We don't want multiple friendship offers to appear, this code checks if there are previous offers // by iterating though all panels. // Note: it might be better to simply add a "pending offer" flag somewhere - for (LLToastNotifyPanel::instance_iter ti(LLToastNotifyPanel::beginInstances()) - , tend(LLToastNotifyPanel::endInstances()); ti != tend; ++ti) + for (auto& ti : LLToastNotifyPanel::instance_snapshot()) { - LLToastNotifyPanel& panel = *ti; - LLIMToastNotifyPanel * imtoastp = dynamic_cast(&panel); - const std::string& notification_name = panel.getNotificationName(); + LLIMToastNotifyPanel * imtoastp = dynamic_cast(&ti); + const std::string& notification_name = ti.getNotificationName(); if (notification_name == "OfferFriendship" - && panel.isControlPanelEnabled() + && ti.isControlPanelEnabled() && imtoastp) { create_toast = false; diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index 7dc04a834e..1d874813c1 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -1972,24 +1972,9 @@ bool LLAppViewer::cleanup() gDirUtilp->deleteFilesInDir(logdir, "*-*-*-*-*.dmp"); } - { - // Kill off LLLeap objects. We can find them all because LLLeap is derived - // from LLInstanceTracker. But collect instances first: LLInstanceTracker - // specifically forbids adding/deleting instances while iterating. - std::vector leaps; - leaps.reserve(LLLeap::instanceCount()); - for (LLLeap::instance_iter li(LLLeap::beginInstances()), lend(LLLeap::endInstances()); - li != lend; ++li) - { - leaps.push_back(&*li); - } - // Okay, now trash them all. We don't have to NULL or erase the entry - // in 'leaps' because the whole vector is going away momentarily. - BOOST_FOREACH(LLLeap* leap, leaps) - { - delete leap; - } - } // destroy 'leaps' + // Kill off LLLeap objects. We can find them all because LLLeap is derived + // from LLInstanceTracker. + LLLeap::instance_snapshot().deleteAll(); //flag all elements as needing to be destroyed immediately // to ensure shutdown order @@ -3397,12 +3382,11 @@ bool LLAppViewer::initConfiguration() // Let anyone else who cares know that we've populated our settings // variables. - for (LLControlGroup::key_iter ki(LLControlGroup::beginKeys()), kend(LLControlGroup::endKeys()); - ki != kend; ++ki) + for (const auto& key : LLControlGroup::key_snapshot()) { // For each named instance of LLControlGroup, send an event saying // we've initialized an LLControlGroup instance by that name. - LLEventPumps::instance().obtain("LLControlGroup").post(LLSDMap("init", *ki)); + LLEventPumps::instance().obtain("LLControlGroup").post(LLSDMap("init", key)); } // [RLVa:KB] - Patch: RLVa-2.1.0 diff --git a/indra/newview/llchannelmanager.cpp b/indra/newview/llchannelmanager.cpp index 084fd527ad..74b53ea7ca 100644 --- a/indra/newview/llchannelmanager.cpp +++ b/indra/newview/llchannelmanager.cpp @@ -48,11 +48,18 @@ LLChannelManager::LLChannelManager() LLAppViewer::instance()->setOnLoginCompletedCallback(boost::bind(&LLChannelManager::onLoginCompleted, this)); mChannelList.clear(); mStartUpChannel = NULL; - + if(!gViewerWindow) { LL_ERRS() << "LLChannelManager::LLChannelManager() - viwer window is not initialized yet" << LL_ENDL; } + + // We don't actually need this instance right now, but our + // cleanupSingleton() method deletes LLScreenChannels, which need to + // unregister from LLUI. Calling LLUI::instance() here establishes the + // dependency so LLSingletonBase::deleteAll() calls our deleteSingleton() + // before LLUI::deleteSingleton(). + LLUI::instance(); } //-------------------------------------------------------------------------- diff --git a/indra/newview/llchathistory.cpp b/indra/newview/llchathistory.cpp index 41aac017a1..bd56adf3de 100644 --- a/indra/newview/llchathistory.cpp +++ b/indra/newview/llchathistory.cpp @@ -1410,10 +1410,8 @@ void LLChatHistory::appendMessage(const LLChat& chat, const LLSD &args, const LL // We don't want multiple friendship offers to appear, this code checks if there are previous offers // by iterating though all panels. // Note: it might be better to simply add a "pending offer" flag somewhere - for (LLToastNotifyPanel::instance_iter ti(LLToastNotifyPanel::beginInstances()) - , tend(LLToastNotifyPanel::endInstances()); ti != tend; ++ti) + for (auto& panel : LLToastNotifyPanel::instance_snapshot()) { - LLToastNotifyPanel& panel = *ti; LLIMToastNotifyPanel * imtoastp = dynamic_cast(&panel); const std::string& notification_name = panel.getNotificationName(); if (notification_name == "OfferFriendship" diff --git a/indra/newview/llimprocessing.cpp b/indra/newview/llimprocessing.cpp index 805fb38afb..fc2d33cdd9 100644 --- a/indra/newview/llimprocessing.cpp +++ b/indra/newview/llimprocessing.cpp @@ -2054,10 +2054,8 @@ void LLIMProcessing::processNewMessage(LLUUID from_id, payload["sender"] = sender.getIPandPort(); bool add_notification = true; - for (LLToastNotifyPanel::instance_iter ti(LLToastNotifyPanel::beginInstances()) - , tend(LLToastNotifyPanel::endInstances()); ti != tend; ++ti) + for (auto& panel : LLToastNotifyPanel::instance_snapshot()) { - LLToastNotifyPanel& panel = *ti; const std::string& notification_name = panel.getNotificationName(); if (notification_name == "OfferFriendship" && panel.isControlPanelEnabled()) { diff --git a/indra/newview/llscenemonitor.cpp b/indra/newview/llscenemonitor.cpp index 7261861e80..fe2f94d03e 100644 --- a/indra/newview/llscenemonitor.cpp +++ b/indra/newview/llscenemonitor.cpp @@ -559,16 +559,14 @@ void LLSceneMonitor::dumpToFile(std::string file_name) typedef StatType trace_count; - for (trace_count::instance_iter it = trace_count::beginInstances(), end_it = trace_count::endInstances(); - it != end_it; - ++it) + for (auto& it : trace_count::instance_snapshot()) { std::ostringstream row; row << std::setprecision(10); - row << it->getName(); + row << it.getName(); - const char* unit_label = it->getUnitLabel(); + const char* unit_label = it.getUnitLabel(); if(unit_label[0]) { row << "(" << unit_label << ")"; @@ -579,8 +577,8 @@ void LLSceneMonitor::dumpToFile(std::string file_name) for (S32 frame = 1; frame <= frame_count; frame++) { Recording& recording = scene_load_recording.getPrevRecording(frame_count - frame); - samples += recording.getSampleCount(*it); - row << ", " << recording.getSum(*it); + samples += recording.getSampleCount(it); + row << ", " << recording.getSum(it); } row << '\n'; @@ -593,15 +591,13 @@ void LLSceneMonitor::dumpToFile(std::string file_name) typedef StatType trace_event; - for (trace_event::instance_iter it = trace_event::beginInstances(), end_it = trace_event::endInstances(); - it != end_it; - ++it) + for (auto& it : trace_event::instance_snapshot()) { std::ostringstream row; row << std::setprecision(10); - row << it->getName(); + row << it.getName(); - const char* unit_label = it->getUnitLabel(); + const char* unit_label = it.getUnitLabel(); if(unit_label[0]) { row << "(" << unit_label << ")"; @@ -612,8 +608,8 @@ void LLSceneMonitor::dumpToFile(std::string file_name) for (S32 frame = 1; frame <= frame_count; frame++) { Recording& recording = scene_load_recording.getPrevRecording(frame_count - frame); - samples += recording.getSampleCount(*it); - F64 mean = recording.getMean(*it); + samples += recording.getSampleCount(it); + F64 mean = recording.getMean(it); if (llisnan(mean)) { row << ", n/a"; @@ -634,15 +630,13 @@ void LLSceneMonitor::dumpToFile(std::string file_name) typedef StatType trace_sample; - for (trace_sample::instance_iter it = trace_sample::beginInstances(), end_it = trace_sample::endInstances(); - it != end_it; - ++it) + for (auto& it : trace_sample::instance_snapshot()) { std::ostringstream row; row << std::setprecision(10); - row << it->getName(); + row << it.getName(); - const char* unit_label = it->getUnitLabel(); + const char* unit_label = it.getUnitLabel(); if(unit_label[0]) { row << "(" << unit_label << ")"; @@ -653,8 +647,8 @@ void LLSceneMonitor::dumpToFile(std::string file_name) for (S32 frame = 1; frame <= frame_count; frame++) { Recording& recording = scene_load_recording.getPrevRecording(frame_count - frame); - samples += recording.getSampleCount(*it); - F64 mean = recording.getMean(*it); + samples += recording.getSampleCount(it); + F64 mean = recording.getMean(it); if (llisnan(mean)) { row << ", n/a"; @@ -674,15 +668,13 @@ void LLSceneMonitor::dumpToFile(std::string file_name) } typedef StatType trace_mem; - for (trace_mem::instance_iter it = trace_mem::beginInstances(), end_it = trace_mem::endInstances(); - it != end_it; - ++it) + for (auto& it : trace_mem::instance_snapshot()) { - os << it->getName() << "(KiB)"; + os << it.getName() << "(KiB)"; for (S32 frame = 1; frame <= frame_count; frame++) { - os << ", " << scene_load_recording.getPrevRecording(frame_count - frame).getMax(*it).valueInUnits(); + os << ", " << scene_load_recording.getPrevRecording(frame_count - frame).getMax(it).valueInUnits(); } os << '\n'; diff --git a/indra/newview/lltoast.cpp b/indra/newview/lltoast.cpp index 0c5e8f04f0..a1c793f85a 100644 --- a/indra/newview/lltoast.cpp +++ b/indra/newview/lltoast.cpp @@ -634,16 +634,8 @@ S32 LLToast::notifyParent(const LLSD& info) //static void LLToast::updateClass() { - // Minimize calls to getInstances per frame - //for (LLInstanceTracker::instance_iter iter = LLInstanceTracker::beginInstances(); - // iter != LLInstanceTracker::endInstances(); ) - - LLInstanceTracker::instance_iter end = LLInstanceTracker::endInstances(); - for (LLInstanceTracker::instance_iter iter = LLInstanceTracker::beginInstances(); iter != end; ) - // + for (auto& toast : LLInstanceTracker::instance_snapshot()) { - LLToast& toast = *iter++; - toast.updateHoveredState(); } } @@ -651,22 +643,6 @@ void LLToast::updateClass() // static void LLToast::cleanupToasts() { - LLToast * toastp = NULL; - - while (LLInstanceTracker::instanceCount() > 0) - { - { // Need to scope iter to allow deletion - LLInstanceTracker::instance_iter iter = LLInstanceTracker::beginInstances(); - toastp = &(*iter); - } - - //LL_INFOS() << "Cleaning up toast id " << toastp->getNotificationID() << LL_ENDL; - - // LLToast destructor will remove it from the LLInstanceTracker. - if (!toastp) - break; // Don't get stuck in the loop if a null pointer somehow got on the list - - delete toastp; - } + LLInstanceTracker::instance_snapshot().deleteAll(); } diff --git a/indra/newview/llviewercontrollistener.cpp b/indra/newview/llviewercontrollistener.cpp index d2484b2b23..3443bb644a 100644 --- a/indra/newview/llviewercontrollistener.cpp +++ b/indra/newview/llviewercontrollistener.cpp @@ -50,11 +50,9 @@ LLViewerControlListener::LLViewerControlListener() std::ostringstream groupnames; groupnames << "[\"group\"] is one of "; const char* delim = ""; - for (LLControlGroup::key_iter cgki(LLControlGroup::beginKeys()), - cgkend(LLControlGroup::endKeys()); - cgki != cgkend; ++cgki) + for (const auto& key : LLControlGroup::key_snapshot()) { - groupnames << delim << '"' << *cgki << '"'; + groupnames << delim << '"' << key << '"'; delim = ", "; } groupnames << '\n'; @@ -181,11 +179,9 @@ void LLViewerControlListener::groups(LLSD const & request) { // No Info, we're not looking up either a group or a control name. Response response(LLSD(), request); - for (LLControlGroup::key_iter cgki(LLControlGroup::beginKeys()), - cgkend(LLControlGroup::endKeys()); - cgki != cgkend; ++cgki) + for (const auto& key : LLControlGroup::key_snapshot()) { - response["groups"].append(*cgki); + response["groups"].append(key); } } diff --git a/indra/test/sync.h b/indra/test/sync.h index cafbc034b4..0f0b6cece8 100644 --- a/indra/test/sync.h +++ b/indra/test/sync.h @@ -41,18 +41,49 @@ public: mTimeout(timeout) {} - /// Bump mCond by n steps -- ideally, do this every time a participating - /// coroutine wakes up from any suspension. The choice to bump() after - /// resumption rather than just before suspending is worth calling out: - /// this practice relies on the fact that condition_variable::notify_all() - /// merely marks a suspended coroutine ready to run, rather than - /// immediately resuming it. This way, though, even if a coroutine exits - /// before reaching its next suspend point, the other coroutine isn't - /// left waiting forever. + /** + * Bump mCond by n steps -- ideally, do this every time a participating + * coroutine wakes up from any suspension. The choice to bump() after + * resumption rather than just before suspending is worth calling out: + * this practice relies on the fact that condition_variable::notify_all() + * merely marks a suspended coroutine ready to run, rather than + * immediately resuming it. This way, though, even if a coroutine exits + * before reaching its next suspend point, the other coroutine isn't + * left waiting forever. + */ void bump(int n=1) { - LL_DEBUGS() << llcoro::logname() << " bump(" << n << ") -> " << (mCond.get() + n) << LL_ENDL; - mCond.set_all(mCond.get() + n); + // Calling mCond.set_all(mCond.get() + n) would be great for + // coroutines -- but not so good between kernel threads -- it would be + // racy. Make the increment atomic by calling update_all(), which runs + // the passed lambda within a mutex lock. + int updated; + mCond.update_all( + [&n, &updated](int& data) + { + data += n; + // Capture the new value for possible logging purposes. + updated = data; + }); + // In the multi-threaded case, this log message could be a bit + // misleading, as it will be emitted after waiting threads have + // already awakened. But emitting the log message within the lock + // would seem to hold the lock longer than we really ought. + LL_DEBUGS() << llcoro::logname() << " bump(" << n << ") -> " << updated << LL_ENDL; + } + + /** + * Set mCond to a specific n. Use of bump() and yield() is nicely + * maintainable, since you can insert or delete matching operations in a + * test function and have the rest of the Sync operations continue to + * line up as before. But sometimes you need to get very specific, which + * is where set() and yield_until() come in handy: less maintainable, + * more precise. + */ + void set(int n) + { + LL_DEBUGS() << llcoro::logname() << " set(" << n << ")" << LL_ENDL; + mCond.set_all(n); } /// suspend until "somebody else" has bumped mCond by n steps