From 9d5b897600a8f9405ec37a141b9417f34a11c557 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 2 Dec 2019 14:39:24 -0500 Subject: DRTVWR-494: Defend LLInstanceTracker against multi-thread usage. The previous implementation went to some effort to crash if anyone attempted to create or destroy an LLInstanceTracker subclass instance during traversal. That restriction is manageable within a single thread, but becomes unworkable if it's possible that a given subclass might be used on more than one thread. Remove LLInstanceTracker::instance_iter, beginInstances(), endInstances(), also key_iter, beginKeys() and endKeys(). Instead, introduce key_snapshot() and instance_snapshot(), the only means of iterating over LLInstanceTracker instances. (These are intended to resemble functions, but in fact the current implementation simply presents the classes.) Iterating over a captured snapshot defends against container modifications during traversal. The term 'snapshot' reminds the coder that a new instance created during traversal will not be considered. To defend against instance deletion during traversal, a snapshot stores std::weak_ptrs which it lazily dereferences, skipping on the fly any that have expired. Dereferencing instance_snapshot::iterator gets you a reference rather than a pointer. Because some use cases want to delete all existing instances, add an instance_snapshot::deleteAll() method that extracts the pointer. Those cases used to require explicitly copying instance pointers into a separate container; instance_snapshot() now takes care of that. It remains the caller's responsibility to ensure that all instances of that LLInstanceTracker subclass were allocated on the heap. Replace unkeyed static LLInstanceTracker::getInstance(T*) -- which returned nullptr if that instance had been destroyed -- with new getWeak() method returning std::weak_ptr. Caller must detect expiration of that weak_ptr. Adjust tests accordingly. Use of std::weak_ptr to detect expired instances requires engaging std::shared_ptr in the constructor. We now store shared_ptrs in the static containers (std::map for keyed, std::set for unkeyed). Make LLInstanceTrackerBase a template parameterized on the type of the static data it manages. For that reason, hoist static data class declarations out of the class definitions to an LLInstanceTrackerStuff namespace. Remove the static atomic sIterationNestDepth and its methods incrementDepth(), decrementDepth() and getDepth(), since they were used only to forbid creation and destruction during traversal. Add a std::mutex to static data. Introduce an internal LockStatic class that locks the mutex while providing a pointer to static data, making that the only way to access the static data. The LLINSTANCETRACKER_DTOR_NOEXCEPT macro goes away because we no longer expect ~LLInstanceTracker() to throw an exception in test programs. That affects LLTrace::StatBase as well as LLInstanceTracker itself. Adapt consumers to the new LLInstanceTracker API. --- indra/llui/llconsole.cpp | 4 ++-- indra/llui/lllayoutstack.cpp | 6 +++--- indra/llui/llnotificationslistener.cpp | 8 +++----- 3 files changed, 8 insertions(+), 10 deletions(-) (limited to 'indra/llui') diff --git a/indra/llui/llconsole.cpp b/indra/llui/llconsole.cpp index 5f50e46233..7817d99aef 100644 --- a/indra/llui/llconsole.cpp +++ b/indra/llui/llconsole.cpp @@ -369,9 +369,9 @@ LLConsole::Paragraph::Paragraph (LLWString str, const LLColor4 &color, F32 add_t // 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 4a464b3507..4aae1e374b 100644 --- a/indra/llui/lllayoutstack.cpp +++ b/indra/llui/lllayoutstack.cpp @@ -636,10 +636,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); } -- cgit v1.3 From 66981fab0b3c8dcc3a031d50710a2b24ec6b0603 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 10 May 2018 21:46:07 -0400 Subject: SL-793: Use Boost.Fiber instead of the "dcoroutine" library. Longtime fans will remember that the "dcoroutine" library is a Google Summer of Code project by Giovanni P. Deretta. He originally called it "Boost.Coroutine," and we originally added it to our 3p-boost autobuild package as such. But when the official Boost.Coroutine library came along (with a very different API), and we still needed the API of the GSoC project, we renamed the unofficial one "dcoroutine" to allow coexistence. The "dcoroutine" library had an internal low-level API more or less analogous to Boost.Context. We later introduced an implementation of that internal API based on Boost.Context, a step towards eliminating the GSoC code in favor of official, supported Boost code. However, recent versions of Boost.Context no longer support the API on which we built the shim for "dcoroutine." We started down the path of reimplementing that shim using the current Boost.Context API -- then realized that it's time to bite the bullet and replace the "dcoroutine" API with the Boost.Fiber API, which we've been itching to do for literally years now. Naturally, most of the heavy lifting is in llcoros.{h,cpp} and lleventcoro.{h,cpp} -- which is good: the LLCoros layer abstracts away most of the differences between "dcoroutine" and Boost.Fiber. The one feature Boost.Fiber does not provide is the ability to forcibly terminate some other fiber. Accordingly, disable LLCoros::kill() and LLCoprocedureManager::shutdown(). The only known shutdown() call was in LLCoprocedurePool's destructor. We also took the opportunity to remove postAndSuspend2() and its associated machinery: FutureListener2, LLErrorEvent, errorException(), errorLog(), LLCoroEventPumps. All that dual-LLEventPump stuff was introduced at a time when the Responder pattern was king, and we assumed we'd want to listen on one LLEventPump with the success handler and on another with the error handler. We have never actually used that in practice. Remove associated tests, of course. There is one other semantic difference that necessitates patching a number of tests: with "dcoroutine," fulfilling a future IMMEDIATELY resumes the waiting coroutine. With Boost.Fiber, fulfilling a future merely marks the fiber as ready to resume next time the scheduler gets around to it. To observe the test side effects, we've inserted a number of llcoro::suspend() calls -- also in the main loop. For a long time we retained a single unit test exercising the raw "dcoroutine" API. Remove that. Eliminate llcoro_get_id.{h,cpp}, which provided llcoro::get_id(), which was a hack to emulate fiber-local variables. Since Boost.Fiber has an actual API for that, remove the hack. In fact, use (new alias) LLCoros::local_ptr for LLSingleton's dependency tracking in place of llcoro::get_id(). In CMake land, replace BOOST_COROUTINE_LIBRARY with BOOST_FIBER_LIBRARY. We don't actually use the Boost.Coroutine for anything (though there exist plausible use cases). --- indra/cmake/Boost.cmake | 20 +- indra/cmake/LLAddBuildTest.cmake | 4 +- indra/cmake/LLAppearance.cmake | 2 +- indra/cmake/LLCommon.cmake | 4 +- indra/cmake/LLCoreHttp.cmake | 2 +- indra/linux_crash_logger/CMakeLists.txt | 2 +- indra/llcommon/CMakeLists.txt | 6 +- indra/llcommon/llcoro_get_id.cpp | 32 -- indra/llcommon/llcoro_get_id.h | 30 -- indra/llcommon/llcoros.cpp | 297 +++------- indra/llcommon/llcoros.h | 184 ++----- indra/llcommon/lleventcoro.cpp | 268 +++------ indra/llcommon/lleventcoro.h | 204 +------ indra/llcommon/llsingleton.cpp | 41 +- indra/llcommon/tests/lleventcoro_test.cpp | 598 ++------------------- indra/llmessage/CMakeLists.txt | 8 +- indra/llmessage/llcoproceduremanager.cpp | 6 + indra/llmessage/llcoproceduremanager.h | 2 + indra/llprimitive/CMakeLists.txt | 2 +- indra/llui/CMakeLists.txt | 2 +- indra/mac_crash_logger/CMakeLists.txt | 2 +- indra/newview/CMakeLists.txt | 4 +- indra/newview/llappviewer.cpp | 2 + indra/test/CMakeLists.txt | 2 +- indra/viewer_components/login/CMakeLists.txt | 4 +- .../viewer_components/login/tests/lllogin_test.cpp | 5 + indra/win_crash_logger/CMakeLists.txt | 2 +- 27 files changed, 331 insertions(+), 1404 deletions(-) delete mode 100644 indra/llcommon/llcoro_get_id.cpp delete mode 100644 indra/llcommon/llcoro_get_id.h (limited to 'indra/llui') diff --git a/indra/cmake/Boost.cmake b/indra/cmake/Boost.cmake index 180a84dbcf..e05e3ca0e5 100644 --- a/indra/cmake/Boost.cmake +++ b/indra/cmake/Boost.cmake @@ -8,7 +8,7 @@ if (USESYSTEMLIBS) include(FindBoost) set(BOOST_CONTEXT_LIBRARY boost_context-mt) - set(BOOST_COROUTINE_LIBRARY boost_coroutine-mt) + set(BOOST_FIBER_LIBRARY boost_fiber-mt) set(BOOST_FILESYSTEM_LIBRARY boost_filesystem-mt) set(BOOST_PROGRAM_OPTIONS_LIBRARY boost_program_options-mt) set(BOOST_REGEX_LIBRARY boost_regex-mt) @@ -49,9 +49,9 @@ else (USESYSTEMLIBS) set(BOOST_CONTEXT_LIBRARY optimized libboost_context-mt debug libboost_context-mt-gd) - set(BOOST_COROUTINE_LIBRARY - optimized libboost_coroutine-mt - debug libboost_coroutine-mt-gd) + set(BOOST_FIBER_LIBRARY + optimized libboost_fiber-mt + debug libboost_fiber-mt-gd) set(BOOST_FILESYSTEM_LIBRARY optimized libboost_filesystem-mt debug libboost_filesystem-mt-gd) @@ -75,9 +75,9 @@ else (USESYSTEMLIBS) set(BOOST_CONTEXT_LIBRARY optimized boost_context-mt debug boost_context-mt-d) - set(BOOST_COROUTINE_LIBRARY - optimized boost_coroutine-mt - debug boost_coroutine-mt-d) + set(BOOST_FIBER_LIBRARY + optimized boost_fiber-mt + debug boost_fiber-mt-d) set(BOOST_FILESYSTEM_LIBRARY optimized boost_filesystem-mt debug boost_filesystem-mt-d) @@ -100,9 +100,9 @@ else (USESYSTEMLIBS) set(BOOST_CONTEXT_LIBRARY optimized boost_context-mt debug boost_context-mt-d) - set(BOOST_COROUTINE_LIBRARY - optimized boost_coroutine-mt - debug boost_coroutine-mt-d) + set(BOOST_FIBER_LIBRARY + optimized boost_fiber-mt + debug boost_fiber-mt-d) set(BOOST_FILESYSTEM_LIBRARY optimized boost_filesystem-mt debug boost_filesystem-mt-d) diff --git a/indra/cmake/LLAddBuildTest.cmake b/indra/cmake/LLAddBuildTest.cmake index b3f42c1a5e..ee6396e473 100644 --- a/indra/cmake/LLAddBuildTest.cmake +++ b/indra/cmake/LLAddBuildTest.cmake @@ -53,7 +53,7 @@ INCLUDE(GoogleMock) ${GOOGLEMOCK_INCLUDE_DIRS} ) SET(alltest_LIBRARIES - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY} ${GOOGLEMOCK_LIBRARIES} @@ -201,7 +201,7 @@ FUNCTION(LL_ADD_INTEGRATION_TEST SET(libraries ${library_dependencies} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY} ${GOOGLEMOCK_LIBRARIES} diff --git a/indra/cmake/LLAppearance.cmake b/indra/cmake/LLAppearance.cmake index ae265d07e3..675330ec72 100644 --- a/indra/cmake/LLAppearance.cmake +++ b/indra/cmake/LLAppearance.cmake @@ -18,7 +18,7 @@ endif (BUILD_HEADLESS) set(LLAPPEARANCE_LIBRARIES llappearance llmessage llcorehttp - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY} ) diff --git a/indra/cmake/LLCommon.cmake b/indra/cmake/LLCommon.cmake index 3e29297c58..8900419f9b 100644 --- a/indra/cmake/LLCommon.cmake +++ b/indra/cmake/LLCommon.cmake @@ -19,7 +19,7 @@ if (LINUX) # specify all libraries that llcommon uses. # llcommon uses `clock_gettime' which is provided by librt on linux. set(LLCOMMON_LIBRARIES llcommon - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_THREAD_LIBRARY} ${BOOST_SYSTEM_LIBRARY} @@ -27,7 +27,7 @@ if (LINUX) ) else (LINUX) set(LLCOMMON_LIBRARIES llcommon - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_THREAD_LIBRARY} ${BOOST_SYSTEM_LIBRARY} ) diff --git a/indra/cmake/LLCoreHttp.cmake b/indra/cmake/LLCoreHttp.cmake index 379ae207de..613453ab5d 100644 --- a/indra/cmake/LLCoreHttp.cmake +++ b/indra/cmake/LLCoreHttp.cmake @@ -12,6 +12,6 @@ set(LLCOREHTTP_INCLUDE_DIRS ) set(LLCOREHTTP_LIBRARIES llcorehttp - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY}) diff --git a/indra/linux_crash_logger/CMakeLists.txt b/indra/linux_crash_logger/CMakeLists.txt index 315aed8d11..d789c850a0 100644 --- a/indra/linux_crash_logger/CMakeLists.txt +++ b/indra/linux_crash_logger/CMakeLists.txt @@ -69,7 +69,7 @@ target_link_libraries(linux-crash-logger ${LLMATH_LIBRARIES} ${LLCOREHTTP_LIBRARIES} ${LLCOMMON_LIBRARIES} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${UI_LIBRARIES} ${DB_LIBRARIES} diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 55c44446b4..2f263cd830 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -44,7 +44,6 @@ set(llcommon_SOURCE_FILES llcleanup.cpp llcommon.cpp llcommonutils.cpp - llcoro_get_id.cpp llcoros.cpp llcrc.cpp llcriticaldamp.cpp @@ -146,7 +145,6 @@ set(llcommon_HEADER_FILES llcleanup.h llcommon.h llcommonutils.h - llcoro_get_id.h llcoros.h llcrc.h llcriticaldamp.h @@ -293,7 +291,7 @@ target_link_libraries( ${JSONCPP_LIBRARIES} ${ZLIB_LIBRARIES} ${WINDOWS_LIBRARIES} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_PROGRAM_OPTIONS_LIBRARY} ${BOOST_REGEX_LIBRARY} @@ -322,7 +320,7 @@ if (LL_TESTS) ${LLCOMMON_LIBRARIES} ${WINDOWS_LIBRARIES} ${GOOGLEMOCK_LIBRARIES} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_THREAD_LIBRARY} ${BOOST_SYSTEM_LIBRARY}) diff --git a/indra/llcommon/llcoro_get_id.cpp b/indra/llcommon/llcoro_get_id.cpp deleted file mode 100644 index 24ed1fe0c9..0000000000 --- a/indra/llcommon/llcoro_get_id.cpp +++ /dev/null @@ -1,32 +0,0 @@ -/** - * @file llcoro_get_id.cpp - * @author Nat Goodspeed - * @date 2016-09-03 - * @brief Implementation for llcoro_get_id. - * - * $LicenseInfo:firstyear=2016&license=viewerlgpl$ - * Copyright (c) 2016, Linden Research, Inc. - * $/LicenseInfo$ - */ - -// Precompiled header -#include "linden_common.h" -// associated header -#include "llcoro_get_id.h" -// STL headers -// std headers -// external library headers -// other Linden headers -#include "llcoros.h" - -namespace llcoro -{ - -id get_id() -{ - // An instance of Current can convert to LLCoros::CoroData*, which can - // implicitly convert to void*, which is an llcoro::id. - return LLCoros::Current(); -} - -} // llcoro diff --git a/indra/llcommon/llcoro_get_id.h b/indra/llcommon/llcoro_get_id.h deleted file mode 100644 index 4c1dca6f19..0000000000 --- a/indra/llcommon/llcoro_get_id.h +++ /dev/null @@ -1,30 +0,0 @@ -/** - * @file llcoro_get_id.h - * @author Nat Goodspeed - * @date 2016-09-03 - * @brief Supplement the functionality in llcoro.h. - * - * This is broken out as a separate header file to resolve - * circularity: LLCoros isa LLSingleton, yet LLSingleton machinery - * requires llcoro::get_id(). - * - * Be very suspicious of anyone else #including this header. - * - * $LicenseInfo:firstyear=2016&license=viewerlgpl$ - * Copyright (c) 2016, Linden Research, Inc. - * $/LicenseInfo$ - */ - -#if ! defined(LL_LLCORO_GET_ID_H) -#define LL_LLCORO_GET_ID_H - -namespace llcoro -{ - -/// Get an opaque, distinct token for the running coroutine (or main). -typedef void* id; -id get_id(); - -} // llcoro - -#endif /* ! defined(LL_LLCORO_GET_ID_H) */ diff --git a/indra/llcommon/llcoros.cpp b/indra/llcommon/llcoros.cpp index cc775775bf..f5ffd96cec 100644 --- a/indra/llcommon/llcoros.cpp +++ b/indra/llcommon/llcoros.cpp @@ -34,6 +34,17 @@ // std headers // external library headers #include +#include +#ifndef BOOST_DISABLE_ASSERTS +#define UNDO_BOOST_DISABLE_ASSERTS +// with Boost 1.65.1, needed for Mac with this specific header +#define BOOST_DISABLE_ASSERTS +#endif +#include +#ifdef UNDO_BOOST_DISABLE_ASSERTS +#undef UNDO_BOOST_DISABLE_ASSERTS +#undef BOOST_DISABLE_ASSERTS +#endif // other Linden headers #include "lltimer.h" #include "llevents.h" @@ -45,176 +56,69 @@ #include #endif -namespace { -void no_op() {} -} // anonymous namespace -// Do nothing, when we need nothing done. This is a static member of LLCoros -// because CoroData is a private nested class. -void LLCoros::no_cleanup(CoroData*) {} - -// CoroData for the currently-running coroutine. Use a thread_specific_ptr -// because each thread potentially has its own distinct pool of coroutines. -LLCoros::Current::Current() +const LLCoros::CoroData& LLCoros::get_CoroData(const std::string& caller) const { - // Use a function-static instance so this thread_specific_ptr is - // instantiated on demand. Since we happen to know it's consumed by - // LLSingleton, this is likely to happen before the runtime has finished - // initializing module-static data. For the same reason, we can't package - // this pointer in an LLSingleton. - - // This thread_specific_ptr does NOT own the CoroData object! That's owned - // by LLCoros::mCoros. It merely identifies it. For this reason we - // instantiate it with a no-op cleanup function. - static boost::thread_specific_ptr sCurrent(LLCoros::no_cleanup); - - // If this is the first time we're accessing sCurrent for the running - // thread, its get() will be NULL. This could be a problem, in that - // llcoro::get_id() would return the same (NULL) token value for the "main - // coroutine" in every thread, whereas what we really want is a distinct - // value for every distinct stack in the process. So if get() is NULL, - // give it a heap CoroData: this ensures that llcoro::get_id() will return - // distinct values. - // This tactic is "leaky": sCurrent explicitly does not destroy any - // CoroData to which it points, and we do NOT enter these "main coroutine" - // CoroData instances in the LLCoros::mCoros map. They are dummy entries, - // and they will leak at process shutdown: one CoroData per thread. - if (! sCurrent.get()) + CoroData* current = mCurrent.get(); + // For the main() coroutine, the one NOT explicitly launched by launch(), + // we never explicitly set mCurrent. Use a static CoroData instance with + // canonical values. + 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 -- - // and truthfully the additional (thread-safe!) machinery to ensure - // uniqueness just doesn't feel worth the trouble. - // We use a no-op callable and a minimal stack size because, although - // CoroData's constructor in fact initializes its mCoro with a - // coroutine with that stack size, no one ever actually enters it by - // calling mCoro(). - sCurrent.reset(new CoroData(0, // no prev - "", // not a named coroutine - no_op, // no-op callable - 1024)); // stacksize moot + // to mean "not in a coroutine," empty string should suffice here. + static CoroData sMain(""); + // We need not reset() the local_ptr to this read-only data: reuse the + // same instance for every thread's main coroutine. + current = &sMain; } - - mCurrent = &sCurrent; + return *current; } -//static LLCoros::CoroData& LLCoros::get_CoroData(const std::string& caller) { - CoroData* current = Current(); - // With the dummy CoroData set in LLCoros::Current::Current(), this - // pointer should never be NULL. - llassert_always(current); - return *current; + // reuse const implementation, just cast away const-ness of result + return const_cast(const_cast(this)->get_CoroData(caller)); } //static -LLCoros::coro::self& LLCoros::get_self() +LLCoros::coro::id LLCoros::get_self() { - CoroData& current = get_CoroData("get_self()"); - if (! current.mSelf) - { - LL_ERRS("LLCoros") << "Calling get_self() from non-coroutine context!" << LL_ENDL; - } - return *current.mSelf; + return boost::this_fiber::get_id(); } //static void LLCoros::set_consuming(bool consuming) { - get_CoroData("set_consuming()").mConsuming = consuming; + CoroData& data(LLCoros::instance().get_CoroData("set_consuming()")); + // DO NOT call this on the main() coroutine. + llassert_always(! data.mName.empty()); + data.mConsuming = consuming; } //static bool LLCoros::get_consuming() { - return get_CoroData("get_consuming()").mConsuming; -} - -llcoro::Suspending::Suspending() -{ - LLCoros::Current current; - // Remember currently-running coroutine: we're about to suspend it. - mSuspended = current; - // Revert Current to the value it had at the moment we last switched - // into this coroutine. - current.reset(mSuspended->mPrev); -} - -llcoro::Suspending::~Suspending() -{ - LLCoros::Current current; - // Okay, we're back, update our mPrev - mSuspended->mPrev = current; - // and reinstate our Current. - current.reset(mSuspended); + return LLCoros::instance().get_CoroData("get_consuming()").mConsuming; } LLCoros::LLCoros(): // MAINT-2724: default coroutine stack size too small on Windows. // Previously we used // boost::context::guarded_stack_allocator::default_stacksize(); - // empirically this is 64KB on Windows and Linux. Try quadrupling. + // empirically this is insufficient. #if ADDRESS_SIZE == 64 mStackSize(512*1024) #else mStackSize(256*1024) #endif { - // Register our cleanup() method for "mainloop" ticks - LLEventPumps::instance().obtain("mainloop").listen( - "LLCoros", boost::bind(&LLCoros::cleanup, this, _1)); -} - -bool LLCoros::cleanup(const LLSD&) -{ - static std::string previousName; - static int previousCount = 0; - // Walk the mCoros map, checking and removing completed coroutines. - for (CoroMap::iterator mi(mCoros.begin()), mend(mCoros.end()); mi != mend; ) - { - // Has this coroutine exited (normal return, exception, exit() call) - // since last tick? - if (mi->second->mCoro.exited()) - { - if (previousName != mi->first) - { - previousName = mi->first; - previousCount = 1; - } - else - { - ++previousCount; - } - - if ((previousCount < 5) || !(previousCount % 50)) - { - if (previousCount < 5) - LL_DEBUGS("LLCoros") << "LLCoros: cleaning up coroutine " << mi->first << LL_ENDL; - else - LL_DEBUGS("LLCoros") << "LLCoros: cleaning up coroutine " << mi->first << "("<< previousCount << ")" << LL_ENDL; - - } - // The erase() call will invalidate its passed iterator value -- - // so increment mi FIRST -- but pass its original value to - // erase(). This is what postincrement is all about. - mCoros.erase(mi++); - } - else - { - // Still live, just skip this entry as if incrementing at the top - // of the loop as usual. - ++mi; - } - } - return false; } std::string LLCoros::generateDistinctName(const std::string& prefix) const { - static std::string previousName; - static int previousCount = 0; + static int unique = 0; // Allowing empty name would make getName()'s not-found return ambiguous. if (prefix.empty()) @@ -225,37 +129,15 @@ std::string LLCoros::generateDistinctName(const std::string& prefix) const // If the specified name isn't already in the map, just use that. std::string name(prefix); - // Find the lowest numeric suffix that doesn't collide with an existing - // entry. Start with 2 just to make it more intuitive for any interested - // parties: e.g. "joe", "joe2", "joe3"... - for (int i = 2; ; name = STRINGIZE(prefix << i++)) + // Until we find an unused name, append a numeric suffix for uniqueness. + while (mCoros.find(name) != mCoros.end()) { - if (mCoros.find(name) == mCoros.end()) - { - if (previousName != name) - { - previousName = name; - previousCount = 1; - } - else - { - ++previousCount; - } - - if ((previousCount < 5) || !(previousCount % 50)) - { - if (previousCount < 5) - LL_DEBUGS("LLCoros") << "LLCoros: launching coroutine " << name << LL_ENDL; - else - LL_DEBUGS("LLCoros") << "LLCoros: launching coroutine " << name << "(" << previousCount << ")" << LL_ENDL; - - } - - return name; - } + name = STRINGIZE(prefix << unique++); } + return name; } +/*==========================================================================*| bool LLCoros::kill(const std::string& name) { CoroMap::iterator found = mCoros.find(name); @@ -269,10 +151,11 @@ bool LLCoros::kill(const std::string& name) mCoros.erase(found); return true; } +|*==========================================================================*/ std::string LLCoros::getName() const { - return Current()->mName; + return get_CoroData("getName()").mName; } void LLCoros::setStackSize(S32 stacksize) @@ -300,6 +183,27 @@ void LLCoros::printActiveCoroutines() } } +std::string LLCoros::launch(const std::string& prefix, const callable_t& callable) +{ + std::string name(generateDistinctName(prefix)); + // 'dispatch' means: enter the new fiber immediately, returning here only + // when the fiber yields for whatever reason. + // std::allocator_arg is a flag to indicate that the following argument is + // a StackAllocator. + // protected_fixedsize_stack sets a guard page past the end of the new + // stack so that stack underflow will result in an access violation + // instead of weird, subtle, possibly undiagnosed memory stomps. + boost::fibers::fiber newCoro(boost::fibers::launch::dispatch, + std::allocator_arg, + boost::fibers::protected_fixedsize_stack(mStackSize), + [this, &name, &callable](){ toplevel(name, callable); }); + // You have two choices with a fiber instance: you can join() it or you + // can detach() it. If you try to destroy the instance before doing + // either, the program silently terminates. We don't need this handle. + newCoro.detach(); + return name; +} + #if LL_WINDOWS static const U32 STATUS_MSC_EXCEPTION = 0xE06D7363; // compiler specific @@ -340,10 +244,14 @@ void LLCoros::winlevel(const callable_t& callable) // Top-level wrapper around caller's coroutine callable. This function accepts // the coroutine library's implicit coro::self& parameter and saves it, but // does not pass it down to the caller's callable. -void LLCoros::toplevel(coro::self& self, CoroData* data, const callable_t& callable) +void LLCoros::toplevel(const std::string& name, const callable_t& callable) { - // capture the 'self' param in CoroData - data->mSelf = &self; + CoroData* corodata = new CoroData(name); + // Store it in our pointer map. Oddly, must cast away const-ness of key. + mCoros.insert(const_cast(name), corodata); + // also set it as current + mCurrent.reset(corodata); + // run the code the caller actually wants in the coroutine try { @@ -358,70 +266,41 @@ void LLCoros::toplevel(coro::self& self, CoroData* data, const callable_t& calla // Any uncaught exception derived from LLContinueError will be caught // here and logged. This coroutine will terminate but the rest of the // viewer will carry on. - LOG_UNHANDLED_EXCEPTION(STRINGIZE("coroutine " << data->mName)); + LOG_UNHANDLED_EXCEPTION(STRINGIZE("coroutine " << corodata->mName)); } catch (...) { // Any OTHER kind of uncaught exception will cause the viewer to // crash, hopefully informatively. - CRASH_ON_UNHANDLED_EXCEPTION(STRINGIZE("coroutine " << data->mName)); + CRASH_ON_UNHANDLED_EXCEPTION(STRINGIZE("coroutine " << corodata->mName)); } - // This cleanup isn't perfectly symmetrical with the way we initially set - // data->mPrev, but this is our last chance to reset Current. - Current().reset(data->mPrev); } -/***************************************************************************** -* MUST BE LAST -*****************************************************************************/ -// Turn off MSVC optimizations for just LLCoros::launch() -- see -// DEV-32777. But MSVC doesn't support push/pop for optimization flags as it -// does for warning suppression, and we really don't want to force -// optimization ON for other code even in Debug or RelWithDebInfo builds. - -#if LL_MSVC -// work around broken optimizations -#pragma warning(disable: 4748) -#pragma warning(disable: 4355) // 'this' used in initializer list: yes, intentionally -#pragma optimize("", off) -#endif // LL_MSVC - -LLCoros::CoroData::CoroData(CoroData* prev, const std::string& name, - const callable_t& callable, S32 stacksize): - mPrev(prev), +LLCoros::CoroData::CoroData(const std::string& name): mName(name), - // Wrap the caller's callable in our toplevel() function so we can manage - // Current appropriately at startup and shutdown of each coroutine. - mCoro(boost::bind(toplevel, _1, this, callable), stacksize), // don't consume events unless specifically directed mConsuming(false), - mSelf(0), mCreationTime(LLTimer::getTotalSeconds()) { } -std::string LLCoros::launch(const std::string& prefix, const callable_t& callable) +void LLCoros::delete_CoroData(CoroData* cdptr) { - std::string name(generateDistinctName(prefix)); - Current current; - // pass the current value of Current as previous context - CoroData* newCoro = new(std::nothrow) CoroData(current, name, callable, mStackSize); - if (newCoro == NULL) + // This custom cleanup function is necessarily static. Find and bind the + // LLCoros instance. + LLCoros& self(LLCoros::instance()); + // We set mCurrent on entry to a new fiber, expecting that the + // corresponding entry has already been stored in mCoros. It is an + // error if we do not find that entry. + CoroMap::iterator found = self.mCoros.find(cdptr->mName); + if (found == self.mCoros.end()) { - // Out of memory? - printActiveCoroutines(); - LL_ERRS("LLCoros") << "Failed to start coroutine: " << name << " Stacksize: " << mStackSize << " Total coroutines: " << mCoros.size() << LL_ENDL; + LL_ERRS("LLCoros") << "Coroutine '" << cdptr->mName << "' terminated " + << "without being stored in LLCoros::mCoros" + << LL_ENDL; } - // Store it in our pointer map - mCoros.insert(name, newCoro); - // also set it as current - current.reset(newCoro); - /* Run the coroutine until its first wait, then return here */ - (newCoro->mCoro)(std::nothrow); - return name; -} -#if LL_MSVC -// reenable optimizations -#pragma optimize("", on) -#endif // LL_MSVC + // Oh good, we found the mCoros entry. Erase it. Because it's a ptr_map, + // that will implicitly delete this CoroData. + self.mCoros.erase(found); +} diff --git a/indra/llcommon/llcoros.h b/indra/llcommon/llcoros.h index c551413811..678633497d 100644 --- a/indra/llcommon/llcoros.h +++ b/indra/llcommon/llcoros.h @@ -29,22 +29,13 @@ #if ! defined(LL_LLCOROS_H) #define LL_LLCOROS_H -#include -#include +#include +#include +#include #include "llsingleton.h" #include #include -#include -#include #include -#include -#include "llcoro_get_id.h" // for friend declaration - -// forward-declare helper class -namespace llcoro -{ -class Suspending; -} /** * Registry of named Boost.Coroutine instances @@ -76,19 +67,20 @@ class Suspending; * name prefix; from your prefix it generates a distinct name, registers the * new coroutine and returns the actual name. * - * The name can be used to kill off the coroutine prematurely, if needed. It - * can also provide diagnostic info: we can look up the name of the + * The name + * can provide diagnostic info: we can look up the name of the * currently-running coroutine. - * - * Finally, the next frame ("mainloop" event) after the coroutine terminates, - * LLCoros will notice its demise and destroy it. */ class LL_COMMON_API LLCoros: public LLSingleton { LLSINGLETON(LLCoros); public: - /// Canonical boost::dcoroutines::coroutine signature we use - typedef boost::dcoroutines::coroutine coro; + /// The viewer's use of the term "coroutine" became deeply embedded before + /// the industry term "fiber" emerged to distinguish userland threads from + /// simpler, more transient kinds of coroutines. Semantically they've + /// always been fibers. But at this point in history, we're pretty much + /// stuck with the term "coroutine." + typedef boost::fibers::fiber coro; /// Canonical callable type typedef boost::function callable_t; @@ -119,10 +111,10 @@ public: * DEV-32777 comments for an explanation. * * Pass a nullary callable. It works to directly pass a nullary free - * function (or static method); for all other cases use boost::bind(). Of - * course, for a non-static class method, the first parameter must be the - * class instance. Any other parameters should be passed via the bind() - * expression. + * function (or static method); for other cases use a lambda expression, + * std::bind() or boost::bind(). Of course, for a non-static class method, + * the first parameter must be the class instance. Any other parameters + * should be passed via the enclosing expression. * * launch() tweaks the suggested name so it won't collide with any * existing coroutine instance, creates the coroutine instance, registers @@ -138,7 +130,7 @@ public: * one prematurely. Returns @c true if the specified name was found and * still running at the time. */ - bool kill(const std::string& name); +// bool kill(const std::string& name); /** * From within a coroutine, look up the (tweaked) name string by which @@ -148,14 +140,18 @@ public: */ std::string getName() const; - /// for delayed initialization + /** + * For delayed initialization. To be clear, this will only affect + * coroutines launched @em after this point. The underlying facility + * provides no way to alter the stack size of any running coroutine. + */ void setStackSize(S32 stacksize); /// for delayed initialization void printActiveCoroutines(); - /// get the current coro::self& for those who really really care - static coro::self& get_self(); + /// get the current coro::id for those who really really care + static coro::id get_self(); /** * Most coroutines, most of the time, don't "consume" the events for which @@ -190,141 +186,57 @@ public: }; /** - * Please do NOT directly use boost::dcoroutines::future! It is essential - * to maintain the "current" coroutine at every context switch. This - * Future wraps the essential boost::dcoroutines::future functionality - * with that maintenance. + * Aliases for promise and future. An older underlying future implementation + * required us to wrap future; that's no longer needed. However -- if it's + * important to restore kill() functionality, we might need to provide a + * proxy, so continue using the aliases. */ template - class Future; + using Promise = boost::fibers::promise; + template + using Future = boost::fibers::future; + template + static Future getFuture(Promise& promise) { return promise.get_future(); } + + /// for data local to each running coroutine + template + using local_ptr = boost::fibers::fiber_specific_ptr; private: - friend class llcoro::Suspending; - friend llcoro::id llcoro::get_id(); std::string generateDistinctName(const std::string& prefix) const; - bool cleanup(const LLSD&); + void toplevel(const std::string& name, const callable_t& callable); struct CoroData; - static void no_cleanup(CoroData*); #if LL_WINDOWS static void winlevel(const callable_t& callable); #endif - static void toplevel(coro::self& self, CoroData* data, const callable_t& callable); - static CoroData& get_CoroData(const std::string& caller); + CoroData& get_CoroData(const std::string& caller); + const CoroData& get_CoroData(const std::string& caller) const; S32 mStackSize; // coroutine-local storage, as it were: one per coro we track struct CoroData { - CoroData(CoroData* prev, const std::string& name, - const callable_t& callable, S32 stacksize); + CoroData(const std::string& name); - // The boost::dcoroutines library supports asymmetric coroutines. Every - // time we context switch out of a coroutine, we pass control to the - // previously-active one (or to the non-coroutine stack owned by the - // thread). So our management of the "current" coroutine must be able to - // restore the previous value when we're about to switch away. - CoroData* mPrev; // tweaked name of the current coroutine const std::string mName; - // the actual coroutine instance - LLCoros::coro mCoro; // set_consuming() state bool mConsuming; - // When the dcoroutine library calls a top-level callable, it implicitly - // passes coro::self& as the first parameter. All our consumer code used - // to explicitly pass coro::self& down through all levels of call stack, - // because at the leaf level we need it for context-switching. But since - // coroutines are based on cooperative switching, we can cause the - // top-level entry point to stash a pointer to the currently-running - // coroutine, and manage it appropriately as we switch out and back in. - // That eliminates the need to pass it as an explicit parameter down - // through every level, which is unfortunately viral in nature. Finding it - // implicitly rather than explicitly allows minor maintenance in which a - // leaf-level function adds a new async I/O call that suspends the calling - // coroutine, WITHOUT having to propagate coro::self& through every - // function signature down to that point -- and of course through every - // other caller of every such function. - LLCoros::coro::self* mSelf; F64 mCreationTime; // since epoch }; typedef boost::ptr_map CoroMap; CoroMap mCoros; - // Identify the current coroutine's CoroData. Use a little helper class so - // a caller can either use a temporary instance, or instantiate a named - // variable and access it multiple times. - class Current - { - public: - Current(); - - operator LLCoros::CoroData*() { return get(); } - LLCoros::CoroData* operator->() { return get(); } - LLCoros::CoroData* get() { return mCurrent->get(); } - void reset(LLCoros::CoroData* ptr) { mCurrent->reset(ptr); } - - private: - boost::thread_specific_ptr* mCurrent; - }; -}; - -namespace llcoro -{ + // Identify the current coroutine's CoroData. This local_ptr isn't static + // because it's a member of an LLSingleton, and we rely on it being + // cleaned up in proper dependency order. + // As each coroutine terminates, use our custom cleanup function to remove + // the corresponding entry from mCoros. + local_ptr mCurrent{delete_CoroData}; -/// Instantiate one of these in a block surrounding any leaf point when -/// control literally switches away from this coroutine. -class Suspending: boost::noncopyable -{ -public: - Suspending(); - ~Suspending(); - -private: - LLCoros::CoroData* mSuspended; -}; - -} // namespace llcoro - -template -class LLCoros::Future -{ - typedef boost::dcoroutines::future dfuture; - -public: - Future(): - mFuture(get_self()) - {} - - typedef typename boost::dcoroutines::make_callback_result::type callback_t; - - callback_t make_callback() - { - return boost::dcoroutines::make_callback(mFuture); - } - -#ifndef LL_LINUX - explicit -#endif - operator bool() const - { - return bool(mFuture); - } - - bool operator!() const - { - return ! mFuture; - } - - T get() - { - // instantiate Suspending to manage the "current" coroutine - llcoro::Suspending suspended; - return *mFuture; - } - -private: - dfuture mFuture; + // Cleanup function for each fiber's instance of mCurrent. + static void delete_CoroData(CoroData* cdptr); }; #endif /* ! defined(LL_LLCOROS_H) */ diff --git a/indra/llcommon/lleventcoro.cpp b/indra/llcommon/lleventcoro.cpp index 43e41f250d..47d99f0050 100644 --- a/indra/llcommon/lleventcoro.cpp +++ b/indra/llcommon/lleventcoro.cpp @@ -31,18 +31,15 @@ // associated header #include "lleventcoro.h" // STL headers -#include +#include // std headers // external library headers +#include // other Linden headers #include "llsdserialize.h" #include "llsdutil.h" #include "llerror.h" #include "llcoros.h" -#include "llmake.h" -#include "llexception.h" - -#include "lleventfilter.h" namespace { @@ -105,65 +102,47 @@ void storeToLLSDPath(LLSD& dest, const LLSD& path, const LLSD& value) llsd::drill(dest, path) = value; } -/// For LLCoros::Future::make_callback(), the callback has a signature -/// like void callback(LLSD), which isn't a valid LLEventPump listener: such -/// listeners must return bool. -template -class FutureListener -{ -public: - // FutureListener is instantiated on the coroutine stack: the stack, in - // other words, that wants to suspend. - FutureListener(const LISTENER& listener): - mListener(listener), - // Capture the suspending coroutine's flag as a consuming or - // non-consuming listener. - mConsume(LLCoros::get_consuming()) - {} - - // operator()() is called on the main stack: the stack on which the - // expected event is fired. - bool operator()(const LLSD& event) - { - mListener(event); - // tell upstream LLEventPump whether listener consumed - return mConsume; - } - -protected: - LISTENER mListener; - bool mConsume; -}; - } // anonymous void llcoro::suspend() { - // By viewer convention, we post an event on the "mainloop" LLEventPump - // each iteration of the main event-handling loop. So waiting for a single - // event on "mainloop" gives us a one-frame suspend. - suspendUntilEventOn("mainloop"); + boost::this_fiber::yield(); } void llcoro::suspendUntilTimeout(float seconds) { - LLEventTimeout timeout; - - timeout.eventAfter(seconds, LLSD()); - llcoro::suspendUntilEventOn(timeout); + // The fact that we accept non-integer seconds means we should probably + // use granularity finer than one second. However, given the overhead of + // the rest of our processing, it seems silly to use granularity finer + // than a millisecond. + boost::this_fiber::sleep_for(std::chrono::milliseconds(long(seconds * 1000))); } -LLSD llcoro::postAndSuspend(const LLSD& event, const LLEventPumpOrPumpName& requestPump, - const LLEventPumpOrPumpName& replyPump, const LLSD& replyPumpNamePath) +namespace { - // declare the future - LLCoros::Future future; + +LLBoundListener postAndSuspendSetup(const std::string& callerName, + const std::string& listenerName, + LLCoros::Promise& promise, + const LLSD& event, + const LLEventPumpOrPumpName& requestPump, + const LLEventPumpOrPumpName& replyPump, + const LLSD& replyPumpNamePath) +{ + // Get the consuming attribute for THIS coroutine, the one that's about to + // suspend. Don't call get_consuming() in the lambda body: that would + // return the consuming attribute for some other coroutine, most likely + // the main routine. + bool consuming(LLCoros::get_consuming()); // make a callback that will assign a value to the future, and listen on // the specified LLEventPump with that callback - std::string listenerName(listenerNameForCoro()); - LLTempBoundListener connection( + LLBoundListener connection( replyPump.getPump().listen(listenerName, - llmake(future.make_callback()))); + [&promise, consuming](const LLSD& result) + { + promise.set_value(result); + return consuming; + })); // skip the "post" part if requestPump is default-constructed if (requestPump) { @@ -171,7 +150,7 @@ LLSD llcoro::postAndSuspend(const LLSD& event, const LLEventPumpOrPumpName& requ // request event. LLSD modevent(event); storeToLLSDPath(modevent, replyPumpNamePath, replyPump.getPump().getName()); - LL_DEBUGS("lleventcoro") << "postAndSuspend(): coroutine " << listenerName + LL_DEBUGS("lleventcoro") << callerName << ": coroutine " << listenerName << " posting to " << requestPump.getPump().getName() << LL_ENDL; @@ -179,158 +158,73 @@ LLSD llcoro::postAndSuspend(const LLSD& event, const LLEventPumpOrPumpName& requ // << ": " << modevent << LL_ENDL; requestPump.getPump().post(modevent); } - LL_DEBUGS("lleventcoro") << "postAndSuspend(): coroutine " << listenerName + LL_DEBUGS("lleventcoro") << callerName << ": coroutine " << listenerName << " about to wait on LLEventPump " << replyPump.getPump().getName() << LL_ENDL; - // calling get() on the future makes us wait for it - LLSD value(future.get()); - LL_DEBUGS("lleventcoro") << "postAndSuspend(): coroutine " << listenerName - << " resuming with " << value << LL_ENDL; - // returning should disconnect the connection - return value; -} - -LLSD llcoro::suspendUntilEventOnWithTimeout(const LLEventPumpOrPumpName& suspendPumpOrName, - F32 timeoutin, const LLSD &timeoutResult) -{ - /** - * The timeout pump is attached upstream of of the waiting pump and will - * pass the timeout event through it. We CAN NOT attach downstream since - * doing so will cause the suspendPump to fire any waiting events immediately - * and they will be lost. This becomes especially problematic with the - * LLEventTimeout(pump) constructor which will also attempt to fire those - * events using the virtual listen_impl method in the not yet fully constructed - * timeoutPump. - */ - LLEventTimeout timeoutPump; - LLEventPump &suspendPump = suspendPumpOrName.getPump(); - - LLTempBoundListener timeoutListener(timeoutPump.listen(suspendPump.getName(), - boost::bind(&LLEventPump::post, &suspendPump, _1))); - - timeoutPump.eventAfter(timeoutin, timeoutResult); - return llcoro::suspendUntilEventOn(suspendPump); + return connection; } -namespace -{ - -/** - * This helper is specifically for postAndSuspend2(). We use a single future - * object, but we want to listen on two pumps with it. Since we must still - * adapt from the callable constructed by boost::dcoroutines::make_callback() - * (void return) to provide an event listener (bool return), we've adapted - * FutureListener for the purpose. The basic idea is that we construct a - * distinct instance of FutureListener2 -- binding different instance data -- - * for each of the pumps. Then, when a pump delivers an LLSD value to either - * FutureListener2, it can combine that LLSD with its discriminator to feed - * the future object. - * - * DISCRIM is a template argument so we can use llmake() rather than - * having to write our own argument-deducing helper function. - */ -template -class FutureListener2: public FutureListener -{ - typedef FutureListener super; - -public: - // instantiated on coroutine stack: the stack about to suspend - FutureListener2(const LISTENER& listener, DISCRIM discriminator): - super(listener), - mDiscrim(discriminator) - {} - - // called on main stack: the stack on which event is fired - bool operator()(const LLSD& event) - { - // our future object is defined to accept LLEventWithID - super::mListener(LLEventWithID(event, mDiscrim)); - // tell LLEventPump whether or not event was consumed - return super::mConsume; - } - -private: - const DISCRIM mDiscrim; -}; - } // anonymous -namespace llcoro +LLSD llcoro::postAndSuspend(const LLSD& event, const LLEventPumpOrPumpName& requestPump, + const LLEventPumpOrPumpName& replyPump, const LLSD& replyPumpNamePath) { + LLCoros::Promise promise; + std::string listenerName(listenerNameForCoro()); + + // Store connection into an LLTempBoundListener so we implicitly + // disconnect on return from this function. + LLTempBoundListener connection = + postAndSuspendSetup("postAndSuspend()", listenerName, promise, + event, requestPump, replyPump, replyPumpNamePath); -LLEventWithID postAndSuspend2(const LLSD& event, - const LLEventPumpOrPumpName& requestPump, - const LLEventPumpOrPumpName& replyPump0, - const LLEventPumpOrPumpName& replyPump1, - const LLSD& replyPump0NamePath, - const LLSD& replyPump1NamePath) -{ // declare the future - LLCoros::Future future; - // either callback will assign a value to this future; listen on - // each specified LLEventPump with a callback - std::string name(listenerNameForCoro()); - LLTempBoundListener connection0( - replyPump0.getPump().listen( - name + "a", - llmake(future.make_callback(), 0))); - LLTempBoundListener connection1( - replyPump1.getPump().listen( - name + "b", - llmake(future.make_callback(), 1))); - // skip the "post" part if requestPump is default-constructed - if (requestPump) - { - // If either replyPumpNamePath is non-empty, store the corresponding - // replyPump name in the request event. - LLSD modevent(event); - storeToLLSDPath(modevent, replyPump0NamePath, - replyPump0.getPump().getName()); - storeToLLSDPath(modevent, replyPump1NamePath, - replyPump1.getPump().getName()); - LL_DEBUGS("lleventcoro") << "postAndSuspend2(): coroutine " << name - << " posting to " << requestPump.getPump().getName() - << ": " << modevent << LL_ENDL; - requestPump.getPump().post(modevent); - } - LL_DEBUGS("lleventcoro") << "postAndSuspend2(): coroutine " << name - << " about to wait on LLEventPumps " << replyPump0.getPump().getName() - << ", " << replyPump1.getPump().getName() << LL_ENDL; + LLCoros::Future future = LLCoros::getFuture(promise); // calling get() on the future makes us wait for it - LLEventWithID value(future.get()); - LL_DEBUGS("lleventcoro") << "postAndSuspend(): coroutine " << name - << " resuming with (" << value.first << ", " << value.second << ")" - << LL_ENDL; - // returning should disconnect both connections + LLSD value(future.get()); + LL_DEBUGS("lleventcoro") << "postAndSuspend(): coroutine " << listenerName + << " resuming with " << value << LL_ENDL; + // returning should disconnect the connection return value; } -LLSD errorException(const LLEventWithID& result, const std::string& desc) +LLSD llcoro::postAndSuspendWithTimeout(const LLSD& event, + const LLEventPumpOrPumpName& requestPump, + const LLEventPumpOrPumpName& replyPump, + const LLSD& replyPumpNamePath, + F32 timeout, const LLSD& timeoutResult) { - // If the result arrived on the error pump (pump 1), instead of - // returning it, deliver it via exception. - if (result.second) + LLCoros::Promise promise; + std::string listenerName(listenerNameForCoro()); + + // Store connection into an LLTempBoundListener so we implicitly + // disconnect on return from this function. + LLTempBoundListener connection = + postAndSuspendSetup("postAndSuspendWithTimeout()", listenerName, promise, + event, requestPump, replyPump, replyPumpNamePath); + + // declare the future + LLCoros::Future future = LLCoros::getFuture(promise); + // wait for specified timeout + boost::fibers::future_status status = + future.wait_for(std::chrono::milliseconds(long(timeout * 1000))); + // if the future is NOT yet ready, return timeoutResult instead + if (status == boost::fibers::future_status::timeout) { - LLTHROW(LLErrorEvent(desc, result.first)); + LL_DEBUGS("lleventcoro") << "postAndSuspendWithTimeout(): coroutine " << listenerName + << " timed out after " << timeout << " seconds," + << " resuming with " << timeoutResult << LL_ENDL; + return timeoutResult; } - // That way, our caller knows a simple return must be from the reply - // pump (pump 0). - return result.first; -} - -LLSD errorLog(const LLEventWithID& result, const std::string& desc) -{ - // If the result arrived on the error pump (pump 1), log it as a fatal - // error. - if (result.second) + else { - LL_ERRS("errorLog") << desc << ":" << std::endl; - LLSDSerialize::toPrettyXML(result.first, LL_CONT); - LL_CONT << LL_ENDL; + llassert_always(status == boost::fibers::future_status::ready); + + // future is now ready, no more waiting + LLSD value(future.get()); + LL_DEBUGS("lleventcoro") << "postAndSuspendWithTimeout(): coroutine " << listenerName + << " resuming with " << value << LL_ENDL; + // returning should disconnect the connection + return value; } - // A simple return must therefore be from the reply pump (pump 0). - return result.first; } - -} // namespace llcoro diff --git a/indra/llcommon/lleventcoro.h b/indra/llcommon/lleventcoro.h index 84827aab4a..c0fe8b094f 100644 --- a/indra/llcommon/lleventcoro.h +++ b/indra/llcommon/lleventcoro.h @@ -29,12 +29,8 @@ #if ! defined(LL_LLEVENTCORO_H) #define LL_LLEVENTCORO_H -#include #include -#include // std::pair #include "llevents.h" -#include "llerror.h" -#include "llexception.h" /** * Like LLListenerOrPumpName, this is a class intended for parameter lists: @@ -147,117 +143,29 @@ LLSD suspendUntilEventOn(const LLEventPumpOrPumpName& pump) return postAndSuspend(LLSD(), LLEventPumpOrPumpName(), pump); } +/// Like postAndSuspend(), but if we wait longer than @a timeout seconds, +/// stop waiting and return @a timeoutResult instead. +LLSD postAndSuspendWithTimeout(const LLSD& event, + const LLEventPumpOrPumpName& requestPump, + const LLEventPumpOrPumpName& replyPump, + const LLSD& replyPumpNamePath, + F32 timeout, const LLSD& timeoutResult); + /// Suspend the coroutine until an event is fired on the identified pump /// or the timeout duration has elapsed. If the timeout duration /// elapses the specified LLSD is returned. -LLSD suspendUntilEventOnWithTimeout(const LLEventPumpOrPumpName& suspendPumpOrName, F32 timeoutin, const LLSD &timeoutResult); - -} // namespace llcoro - -/// return type for two-pump variant of suspendUntilEventOn() -typedef std::pair LLEventWithID; - -namespace llcoro -{ - -/** - * This function waits for a reply on either of two specified LLEventPumps. - * Otherwise, it closely resembles postAndSuspend(); please see the documentation - * for that function for detailed parameter info. - * - * While we could have implemented the single-pump variant in terms of this - * one, there's enough added complexity here to make it worthwhile to give the - * single-pump variant its own straightforward implementation. Conversely, - * though we could use preprocessor logic to generate n-pump overloads up to - * BOOST_COROUTINE_WAIT_MAX, we don't foresee a use case. This two-pump - * overload exists because certain event APIs are defined in terms of a reply - * LLEventPump and an error LLEventPump. - * - * The LLEventWithID return value provides not only the received event, but - * the index of the pump on which it arrived (0 or 1). - * - * @note - * I'd have preferred to overload the name postAndSuspend() for both signatures. - * But consider the following ambiguous call: - * @code - * postAndSuspend(LLSD(), requestPump, replyPump, "someString"); - * @endcode - * "someString" could be converted to either LLSD (@a replyPumpNamePath for - * the single-pump function) or LLEventOrPumpName (@a replyPump1 for two-pump - * function). - * - * It seems less burdensome to write postAndSuspend2() than to write either - * LLSD("someString") or LLEventOrPumpName("someString"). - */ -LLEventWithID postAndSuspend2(const LLSD& event, - const LLEventPumpOrPumpName& requestPump, - const LLEventPumpOrPumpName& replyPump0, - const LLEventPumpOrPumpName& replyPump1, - const LLSD& replyPump0NamePath=LLSD(), - const LLSD& replyPump1NamePath=LLSD()); - -/** - * Wait for the next event on either of two specified LLEventPumps. - */ inline -LLEventWithID -suspendUntilEventOn(const LLEventPumpOrPumpName& pump0, const LLEventPumpOrPumpName& pump1) +LLSD suspendUntilEventOnWithTimeout(const LLEventPumpOrPumpName& suspendPumpOrName, + F32 timeoutin, const LLSD &timeoutResult) { - // This is now a convenience wrapper for postAndSuspend2(). - return postAndSuspend2(LLSD(), LLEventPumpOrPumpName(), pump0, pump1); + return postAndSuspendWithTimeout(LLSD(), // event + LLEventPumpOrPumpName(), // requestPump + suspendPumpOrName, // replyPump + LLSD(), // replyPumpNamePath + timeoutin, + timeoutResult); } -/** - * Helper for the two-pump variant of suspendUntilEventOn(), e.g.: - * - * @code - * LLSD reply = errorException(suspendUntilEventOn(replyPump, errorPump), - * "error response from login.cgi"); - * @endcode - * - * Examines an LLEventWithID, assuming that the second pump (pump 1) is - * listening for an error indication. If the incoming data arrived on pump 1, - * throw an LLErrorEvent exception. If the incoming data arrived on pump 0, - * just return it. Since a normal return can only be from pump 0, we no longer - * need the LLEventWithID's discriminator int; we can just return the LLSD. - * - * @note I'm not worried about introducing the (fairly generic) name - * errorException() into global namespace, because how many other overloads of - * the same name are going to accept an LLEventWithID parameter? - */ -LLSD errorException(const LLEventWithID& result, const std::string& desc); - -} // namespace llcoro - -/** - * Exception thrown by errorException(). We don't call this LLEventError - * because it's not an error in event processing: rather, this exception - * announces an event that bears error information (for some other API). - */ -class LL_COMMON_API LLErrorEvent: public LLException -{ -public: - LLErrorEvent(const std::string& what, const LLSD& data): - LLException(what), - mData(data) - {} - virtual ~LLErrorEvent() throw() {} - - LLSD getData() const { return mData; } - -private: - LLSD mData; -}; - -namespace llcoro -{ - -/** - * Like errorException(), save that this trips a fatal error using LL_ERRS - * rather than throwing an exception. - */ -LL_COMMON_API LLSD errorLog(const LLEventWithID& result, const std::string& desc); - } // namespace llcoro /** @@ -304,84 +212,4 @@ private: LLEventStream mPump; }; -/** - * Other event APIs require the names of two different LLEventPumps: one for - * success response, the other for error response. Extend LLCoroEventPump - * for the two-pump use case. - */ -class LL_COMMON_API LLCoroEventPumps -{ -public: - LLCoroEventPumps(const std::string& name="coro", - const std::string& suff0="Reply", - const std::string& suff1="Error"): - mPump0(name + suff0, true), // allow tweaking the pump instance name - mPump1(name + suff1, true) - {} - /// request pump 0's name - std::string getName0() const { return mPump0.getName(); } - /// request pump 1's name - std::string getName1() const { return mPump1.getName(); } - /// request both names - std::pair getNames() const - { - return std::pair(mPump0.getName(), mPump1.getName()); - } - - /// request pump 0 - LLEventPump& getPump0() { return mPump0; } - /// request pump 1 - LLEventPump& getPump1() { return mPump1; } - - /// suspendUntilEventOn(either of our two LLEventPumps) - LLEventWithID suspend() - { - return llcoro::suspendUntilEventOn(mPump0, mPump1); - } - - /// errorException(suspend()) - LLSD suspendWithException() - { - return llcoro::errorException(suspend(), std::string("Error event on ") + getName1()); - } - - /// errorLog(suspend()) - LLSD suspendWithLog() - { - return llcoro::errorLog(suspend(), std::string("Error event on ") + getName1()); - } - - LLEventWithID postAndSuspend(const LLSD& event, - const LLEventPumpOrPumpName& requestPump, - const LLSD& replyPump0NamePath=LLSD(), - const LLSD& replyPump1NamePath=LLSD()) - { - return llcoro::postAndSuspend2(event, requestPump, mPump0, mPump1, - replyPump0NamePath, replyPump1NamePath); - } - - LLSD postAndSuspendWithException(const LLSD& event, - const LLEventPumpOrPumpName& requestPump, - const LLSD& replyPump0NamePath=LLSD(), - const LLSD& replyPump1NamePath=LLSD()) - { - return llcoro::errorException(postAndSuspend(event, requestPump, - replyPump0NamePath, replyPump1NamePath), - std::string("Error event on ") + getName1()); - } - - LLSD postAndSuspendWithLog(const LLSD& event, - const LLEventPumpOrPumpName& requestPump, - const LLSD& replyPump0NamePath=LLSD(), - const LLSD& replyPump1NamePath=LLSD()) - { - return llcoro::errorLog(postAndSuspend(event, requestPump, - replyPump0NamePath, replyPump1NamePath), - std::string("Error event on ") + getName1()); - } - -private: - LLEventStream mPump0, mPump1; -}; - #endif /* ! defined(LL_LLEVENTCORO_H) */ diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index f5f3aec270..356b896163 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -30,10 +30,9 @@ #include "llerror.h" #include "llerrorcontrol.h" // LLError::is_available() #include "lldependencies.h" -#include "llcoro_get_id.h" #include "llexception.h" +#include "llcoros.h" #include -#include #include #include // std::cerr in dire emergency #include @@ -115,19 +114,10 @@ private: // initialized, either in the constructor or in initSingleton(). However, // managing that as a stack depends on having a DISTINCT 'initializing' // stack for every C++ stack in the process! And we have a distinct C++ - // stack for every running coroutine. It would be interesting and cool to - // implement a generic coroutine-local-storage mechanism and use that - // here. The trouble is that LLCoros is itself an LLSingleton, so - // depending on LLCoros functionality could dig us into infinite - // recursion. (Moreover, when we reimplement LLCoros on top of - // Boost.Fiber, that library already provides fiber_specific_ptr -- so - // it's not worth a great deal of time and energy implementing a generic - // equivalent on top of boost::dcoroutine, which is on its way out.) - // Instead, use a map of llcoro::id to select the appropriate - // coro-specific 'initializing' stack. llcoro::get_id() is carefully - // implemented to avoid requiring LLCoros. - typedef boost::unordered_map InitializingMap; - InitializingMap mInitializing; + // 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; public: // Instantiate this to obtain a reference to the coroutine-specific @@ -166,18 +156,23 @@ public: private: list_t& get_initializing_() { - // map::operator[] has find-or-create semantics, exactly what we need - // here. It returns a reference to the selected mapped_type instance. - return mInitializing[llcoro::get_id()]; + LLSingletonBase::list_t* current = mInitializing.get(); + if (! current) + { + // If the running coroutine doesn't already have an initializing + // stack, allocate a new one and save it for future reference. + current = new LLSingletonBase::list_t(); + mInitializing.reset(current); + } + return *current; } + // By the time mInitializing is destroyed, its value for every coroutine + // except the running one must have been reset() to nullptr. So every time + // we pop the list to empty, reset() the running coroutine's local_ptr. void cleanup_initializing_() { - InitializingMap::iterator found = mInitializing.find(llcoro::get_id()); - if (found != mInitializing.end()) - { - mInitializing.erase(found); - } + mInitializing.reset(nullptr); } }; diff --git a/indra/llcommon/tests/lleventcoro_test.cpp b/indra/llcommon/tests/lleventcoro_test.cpp index fa02d2bb1a..2e4b6ba823 100644 --- a/indra/llcommon/tests/lleventcoro_test.cpp +++ b/indra/llcommon/tests/lleventcoro_test.cpp @@ -26,50 +26,12 @@ * $/LicenseInfo$ */ -/*****************************************************************************/ -// test<1>() is cloned from a Boost.Coroutine example program whose copyright -// info is reproduced here: -/*---------------------------------------------------------------------------*/ -// Copyright (c) 2006, Giovanni P. Deretta -// -// This code may be used under either of the following two licences: -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL -// THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE. OF SUCH DAMAGE. -// -// Or: -// -// Distributed under the Boost Software License, Version 1.0. -// (See accompanying file LICENSE_1_0.txt or copy at -// http://www.boost.org/LICENSE_1_0.txt) -/*****************************************************************************/ - #define BOOST_RESULT_OF_USE_TR1 1 -// On some platforms, Boost.Coroutine must #define magic symbols before -// #including platform-API headers. Naturally, that's ineffective unless the -// Boost.Coroutine #include is the *first* #include of the platform header. -// That means that client code must generally #include Boost.Coroutine headers -// before anything else. -#include #include #include #include #include +#include #include "linden_common.h" @@ -80,47 +42,12 @@ #include "llsd.h" #include "llsdutil.h" #include "llevents.h" -#include "tests/wrapllerrs.h" -#include "stringize.h" #include "llcoros.h" #include "lleventcoro.h" #include "../test/debug.h" using namespace llcoro; -/***************************************************************************** -* from the banana.cpp example program borrowed for test<1>() -*****************************************************************************/ -namespace coroutines = boost::dcoroutines; -using coroutines::coroutine; - -template -bool match(Iter first, Iter last, std::string match) { - std::string::iterator i = match.begin(); - for(; (first != last) && (i != match.end()); ++i) { - if (*first != *i) - return false; - ++first; - } - return i == match.end(); -} - -template -BidirectionalIterator -match_substring(BidirectionalIterator begin, - BidirectionalIterator end, - std::string xmatch, - BOOST_DEDUCED_TYPENAME coroutine::self& self) { -//BidirectionalIterator begin_ = begin; - for(; begin != end; ++begin) - if(match(begin, end, xmatch)) { - self.yield(begin); - } - return end; -} - -typedef coroutine match_coroutine_type; - /***************************************************************************** * Test helpers *****************************************************************************/ @@ -150,6 +77,8 @@ public: LLSD::Integer value(event["value"]); LLSD::String replyPumpName(event.has("fail")? "error" : "reply"); LLEventPumps::instance().obtain(event[replyPumpName]).post(value + 1); + // give listener a chance to process + llcoro::suspend(); return false; } @@ -167,51 +96,6 @@ namespace tut typedef coroutine_group::object object; coroutine_group coroutinegrp("coroutine"); - template<> template<> - void object::test<1>() - { - set_test_name("From banana.cpp example program in Boost.Coroutine distro"); - std::string buffer = "banananana"; - std::string match = "nana"; - std::string::iterator begin = buffer.begin(); - std::string::iterator end = buffer.end(); - -#if defined(BOOST_CORO_POSIX_IMPL) -// std::cout << "Using Boost.Coroutine " << BOOST_CORO_POSIX_IMPL << '\n'; -#else -// std::cout << "Using non-Posix Boost.Coroutine implementation" << std::endl; -#endif - - typedef std::string::iterator signature(std::string::iterator, - std::string::iterator, - std::string, - match_coroutine_type::self&); - - coroutine matcher - (boost::bind(static_cast(match_substring), - begin, - end, - match, - _1)); - - std::string::iterator i = matcher(); -/*==========================================================================*| - while(matcher && i != buffer.end()) { - std::cout <<"Match at: "<< std::distance(buffer.begin(), i)<<'\n'; - i = matcher(); - } -|*==========================================================================*/ - size_t matches[] = { 2, 4, 6 }; - for (size_t *mi(boost::begin(matches)), *mend(boost::end(matches)); - mi != mend; ++mi, i = matcher()) - { - ensure("more", matcher); - ensure("found", i != buffer.end()); - ensure_equals("value", std::distance(buffer.begin(), i), *mi); - } - ensure("done", ! matcher); - } - // use static data so we can intersperse coroutine functions with the // tests that engage them ImmediateAPI immediateAPI; @@ -231,7 +115,7 @@ namespace tut which = 0; } - void explicit_wait(boost::shared_ptr::callback_t>& cbp) + void explicit_wait(boost::shared_ptr>& cbp) { BEGIN { @@ -241,44 +125,40 @@ namespace tut // provides a callback-style notification (and prove that it // works). - LLCoros::Future future; - // get the callback from that future - LLCoros::Future::callback_t callback(future.make_callback()); - // Perhaps we would send a request to a remote server and arrange - // for 'callback' to be called on response. Of course that might - // involve an adapter object from the actual callback signature to - // the signature of 'callback' -- in this case, void(std::string). - // For test purposes, instead of handing 'callback' (or the + // for cbp->set_value() to be called on response. + // For test purposes, instead of handing 'callback' (or an // adapter) off to some I/O subsystem, we'll just pass it back to // our caller. - cbp.reset(new LLCoros::Future::callback_t(callback)); + cbp = boost::make_shared>(); + LLCoros::Future future = LLCoros::getFuture(*cbp); - ensure("Not yet", ! future); // calling get() on the future causes us to suspend debug("about to suspend"); stringdata = future.get(); - ensure("Got it", bool(future)); + ensure_equals("Got it", stringdata, "received"); } END } template<> template<> - void object::test<2>() + void object::test<1>() { clear(); set_test_name("explicit_wait"); DEBUG; // Construct the coroutine instance that will run explicit_wait. - boost::shared_ptr::callback_t> respond; - LLCoros::instance().launch("test<2>", + boost::shared_ptr> respond; + LLCoros::instance().launch("test<1>", boost::bind(explicit_wait, boost::ref(respond))); // When the coroutine waits for the future, it returns here. debug("about to respond"); - // Now we're the I/O subsystem delivering a result. This immediately - // transfers control back to the coroutine. - (*respond)("received"); + // Now we're the I/O subsystem delivering a result. This should make + // the coroutine ready. + respond->set_value("received"); + // but give it a chance to wake up + llcoro::suspend(); // ensure the coroutine ran and woke up again with the intended result ensure_equals(stringdata, "received"); } @@ -293,60 +173,20 @@ namespace tut } template<> template<> - void object::test<3>() + void object::test<2>() { clear(); set_test_name("waitForEventOn1"); DEBUG; - LLCoros::instance().launch("test<3>", waitForEventOn1); + LLCoros::instance().launch("test<2>", waitForEventOn1); debug("about to send"); LLEventPumps::instance().obtain("source").post("received"); + // give waitForEventOn1() a chance to run + llcoro::suspend(); debug("back from send"); ensure_equals(result.asString(), "received"); } - void waitForEventOn2() - { - BEGIN - { - LLEventWithID pair = suspendUntilEventOn("reply", "error"); - result = pair.first; - which = pair.second; - debug(STRINGIZE("result = " << result << ", which = " << which)); - } - END - } - - template<> template<> - void object::test<4>() - { - clear(); - set_test_name("waitForEventOn2 reply"); - { - DEBUG; - LLCoros::instance().launch("test<4>", waitForEventOn2); - debug("about to send"); - LLEventPumps::instance().obtain("reply").post("received"); - debug("back from send"); - } - ensure_equals(result.asString(), "received"); - ensure_equals("which pump", which, 0); - } - - template<> template<> - void object::test<5>() - { - clear(); - set_test_name("waitForEventOn2 error"); - DEBUG; - LLCoros::instance().launch("test<5>", waitForEventOn2); - debug("about to send"); - LLEventPumps::instance().obtain("error").post("badness"); - debug("back from send"); - ensure_equals(result.asString(), "badness"); - ensure_equals("which pump", which, 1); - } - void coroPump() { BEGIN @@ -359,175 +199,20 @@ namespace tut } template<> template<> - void object::test<6>() + void object::test<3>() { clear(); set_test_name("coroPump"); DEBUG; - LLCoros::instance().launch("test<6>", coroPump); - debug("about to send"); - LLEventPumps::instance().obtain(replyName).post("received"); - debug("back from send"); - ensure_equals(result.asString(), "received"); - } - - void coroPumps() - { - BEGIN - { - LLCoroEventPumps waiter; - replyName = waiter.getName0(); - errorName = waiter.getName1(); - LLEventWithID pair(waiter.suspend()); - result = pair.first; - which = pair.second; - } - END - } - - template<> template<> - void object::test<7>() - { - clear(); - set_test_name("coroPumps reply"); - DEBUG; - LLCoros::instance().launch("test<7>", coroPumps); - debug("about to send"); - LLEventPumps::instance().obtain(replyName).post("received"); - debug("back from send"); - ensure_equals(result.asString(), "received"); - ensure_equals("which pump", which, 0); - } - - template<> template<> - void object::test<8>() - { - clear(); - set_test_name("coroPumps error"); - DEBUG; - LLCoros::instance().launch("test<8>", coroPumps); - debug("about to send"); - LLEventPumps::instance().obtain(errorName).post("badness"); - debug("back from send"); - ensure_equals(result.asString(), "badness"); - ensure_equals("which pump", which, 1); - } - - void coroPumpsNoEx() - { - BEGIN - { - LLCoroEventPumps waiter; - replyName = waiter.getName0(); - errorName = waiter.getName1(); - result = waiter.suspendWithException(); - } - END - } - - template<> template<> - void object::test<9>() - { - clear(); - set_test_name("coroPumpsNoEx"); - DEBUG; - LLCoros::instance().launch("test<9>", coroPumpsNoEx); - debug("about to send"); - LLEventPumps::instance().obtain(replyName).post("received"); - debug("back from send"); - ensure_equals(result.asString(), "received"); - } - - void coroPumpsEx() - { - BEGIN - { - LLCoroEventPumps waiter; - replyName = waiter.getName0(); - errorName = waiter.getName1(); - try - { - result = waiter.suspendWithException(); - debug("no exception"); - } - catch (const LLErrorEvent& e) - { - debug(STRINGIZE("exception " << e.what())); - errordata = e.getData(); - } - } - END - } - - template<> template<> - void object::test<10>() - { - clear(); - set_test_name("coroPumpsEx"); - DEBUG; - LLCoros::instance().launch("test<10>", coroPumpsEx); - debug("about to send"); - LLEventPumps::instance().obtain(errorName).post("badness"); - debug("back from send"); - ensure("no result", result.isUndefined()); - ensure_equals("got error", errordata.asString(), "badness"); - } - - void coroPumpsNoLog() - { - BEGIN - { - LLCoroEventPumps waiter; - replyName = waiter.getName0(); - errorName = waiter.getName1(); - result = waiter.suspendWithLog(); - } - END - } - - template<> template<> - void object::test<11>() - { - clear(); - set_test_name("coroPumpsNoLog"); - DEBUG; - LLCoros::instance().launch("test<11>", coroPumpsNoLog); + LLCoros::instance().launch("test<3>", coroPump); debug("about to send"); LLEventPumps::instance().obtain(replyName).post("received"); + // give coroPump() a chance to run + llcoro::suspend(); debug("back from send"); ensure_equals(result.asString(), "received"); } - void coroPumpsLog() - { - BEGIN - { - LLCoroEventPumps waiter; - replyName = waiter.getName0(); - errorName = waiter.getName1(); - WrapLLErrs capture; - threw = capture.catch_llerrs([&waiter, &debug](){ - result = waiter.suspendWithLog(); - debug("no exception"); - }); - } - END - } - - template<> template<> - void object::test<12>() - { - clear(); - set_test_name("coroPumpsLog"); - DEBUG; - LLCoros::instance().launch("test<12>", coroPumpsLog); - debug("about to send"); - LLEventPumps::instance().obtain(errorName).post("badness"); - debug("back from send"); - ensure("no result", result.isUndefined()); - ensure_contains("got error", threw, "badness"); - } - void postAndWait1() { BEGIN @@ -541,71 +226,17 @@ namespace tut } template<> template<> - void object::test<13>() + void object::test<4>() { clear(); set_test_name("postAndWait1"); DEBUG; - LLCoros::instance().launch("test<13>", postAndWait1); + LLCoros::instance().launch("test<4>", postAndWait1); + // give postAndWait1() a chance to run + llcoro::suspend(); ensure_equals(result.asInteger(), 18); } - void postAndWait2() - { - BEGIN - { - LLEventWithID pair = ::postAndSuspend2(LLSDMap("value", 18), - immediateAPI.getPump(), - "reply2", - "error2", - "reply", - "error"); - result = pair.first; - which = pair.second; - debug(STRINGIZE("result = " << result << ", which = " << which)); - } - END - } - - template<> template<> - void object::test<14>() - { - clear(); - set_test_name("postAndWait2"); - DEBUG; - LLCoros::instance().launch("test<14>", postAndWait2); - ensure_equals(result.asInteger(), 19); - ensure_equals(which, 0); - } - - void postAndWait2_1() - { - BEGIN - { - LLEventWithID pair = ::postAndSuspend2(LLSDMap("value", 18)("fail", LLSD()), - immediateAPI.getPump(), - "reply2", - "error2", - "reply", - "error"); - result = pair.first; - which = pair.second; - debug(STRINGIZE("result = " << result << ", which = " << which)); - } - END - } - - template<> template<> - void object::test<15>() - { - clear(); - set_test_name("postAndWait2_1"); - DEBUG; - LLCoros::instance().launch("test<15>", postAndWait2_1); - ensure_equals(result.asInteger(), 19); - ensure_equals(which, 1); - } - void coroPumpPost() { BEGIN @@ -618,177 +249,14 @@ namespace tut } template<> template<> - void object::test<16>() + void object::test<5>() { clear(); set_test_name("coroPumpPost"); DEBUG; - LLCoros::instance().launch("test<16>", coroPumpPost); + LLCoros::instance().launch("test<5>", coroPumpPost); + // give coroPumpPost() a chance to run + llcoro::suspend(); ensure_equals(result.asInteger(), 18); } - - void coroPumpsPost() - { - BEGIN - { - LLCoroEventPumps waiter; - LLEventWithID pair(waiter.postAndSuspend(LLSDMap("value", 23), - immediateAPI.getPump(), "reply", "error")); - result = pair.first; - which = pair.second; - } - END - } - - template<> template<> - void object::test<17>() - { - clear(); - set_test_name("coroPumpsPost reply"); - DEBUG; - LLCoros::instance().launch("test<17>", coroPumpsPost); - ensure_equals(result.asInteger(), 24); - ensure_equals("which pump", which, 0); - } - - void coroPumpsPost_1() - { - BEGIN - { - LLCoroEventPumps waiter; - LLEventWithID pair( - waiter.postAndSuspend(LLSDMap("value", 23)("fail", LLSD()), - immediateAPI.getPump(), "reply", "error")); - result = pair.first; - which = pair.second; - } - END - } - - template<> template<> - void object::test<18>() - { - clear(); - set_test_name("coroPumpsPost error"); - DEBUG; - LLCoros::instance().launch("test<18>", coroPumpsPost_1); - ensure_equals(result.asInteger(), 24); - ensure_equals("which pump", which, 1); - } - - void coroPumpsPostNoEx() - { - BEGIN - { - LLCoroEventPumps waiter; - result = waiter.postAndSuspendWithException(LLSDMap("value", 8), - immediateAPI.getPump(), "reply", "error"); - } - END - } - - template<> template<> - void object::test<19>() - { - clear(); - set_test_name("coroPumpsPostNoEx"); - DEBUG; - LLCoros::instance().launch("test<19>", coroPumpsPostNoEx); - ensure_equals(result.asInteger(), 9); - } - - void coroPumpsPostEx() - { - BEGIN - { - LLCoroEventPumps waiter; - try - { - result = waiter.postAndSuspendWithException( - LLSDMap("value", 9)("fail", LLSD()), - immediateAPI.getPump(), "reply", "error"); - debug("no exception"); - } - catch (const LLErrorEvent& e) - { - debug(STRINGIZE("exception " << e.what())); - errordata = e.getData(); - } - } - END - } - - template<> template<> - void object::test<20>() - { - clear(); - set_test_name("coroPumpsPostEx"); - DEBUG; - LLCoros::instance().launch("test<20>", coroPumpsPostEx); - ensure("no result", result.isUndefined()); - ensure_equals("got error", errordata.asInteger(), 10); - } - - void coroPumpsPostNoLog() - { - BEGIN - { - LLCoroEventPumps waiter; - result = waiter.postAndSuspendWithLog(LLSDMap("value", 30), - immediateAPI.getPump(), "reply", "error"); - } - END - } - - template<> template<> - void object::test<21>() - { - clear(); - set_test_name("coroPumpsPostNoLog"); - DEBUG; - LLCoros::instance().launch("test<21>", coroPumpsPostNoLog); - ensure_equals(result.asInteger(), 31); - } - - void coroPumpsPostLog() - { - BEGIN - { - LLCoroEventPumps waiter; - WrapLLErrs capture; - threw = capture.catch_llerrs( - [&waiter, &debug](){ - result = waiter.postAndSuspendWithLog( - LLSDMap("value", 31)("fail", LLSD()), - immediateAPI.getPump(), "reply", "error"); - debug("no exception"); - }); - } - END - } - - template<> template<> - void object::test<22>() - { - clear(); - set_test_name("coroPumpsPostLog"); - DEBUG; - LLCoros::instance().launch("test<22>", coroPumpsPostLog); - ensure("no result", result.isUndefined()); - ensure_contains("got error", threw, "32"); - } } - -/*==========================================================================*| -#include - -namespace tut -{ - template<> template<> - void object::test<23>() - { - set_test_name("stacksize"); - std::cout << "default_stacksize: " << boost::context::guarded_stack_allocator::default_stacksize() << '\n'; - } -} // namespace tut -|*==========================================================================*/ diff --git a/indra/llmessage/CMakeLists.txt b/indra/llmessage/CMakeLists.txt index e0922c0667..a2a57ad740 100644 --- a/indra/llmessage/CMakeLists.txt +++ b/indra/llmessage/CMakeLists.txt @@ -217,7 +217,7 @@ target_link_libraries( ${NGHTTP2_LIBRARIES} ${XMLRPCEPI_LIBRARIES} ${LLCOREHTTP_LIBRARIES} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY} rt @@ -235,7 +235,7 @@ target_link_libraries( ${NGHTTP2_LIBRARIES} ${XMLRPCEPI_LIBRARIES} ${LLCOREHTTP_LIBRARIES} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY} ) @@ -264,7 +264,7 @@ if (LINUX) ${LLMESSAGE_LIBRARIES} ${LLCOREHTTP_LIBRARIES} ${JSONCPP_LIBRARIES} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} rt ${GOOGLEMOCK_LIBRARIES} @@ -280,7 +280,7 @@ else (LINUX) ${LLMESSAGE_LIBRARIES} ${LLCOREHTTP_LIBRARIES} ${JSONCPP_LIBRARIES} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${GOOGLEMOCK_LIBRARIES} ) diff --git a/indra/llmessage/llcoproceduremanager.cpp b/indra/llmessage/llcoproceduremanager.cpp index 74cdff2b00..4c85dd999a 100644 --- a/indra/llmessage/llcoproceduremanager.cpp +++ b/indra/llmessage/llcoproceduremanager.cpp @@ -203,6 +203,7 @@ void LLCoprocedureManager::cancelCoprocedure(const LLUUID &id) LL_INFOS() << "Coprocedure not found." << LL_ENDL; } +/*==========================================================================*| void LLCoprocedureManager::shutdown(bool hardShutdown) { for (poolMap_t::const_iterator it = mPoolMap.begin(); it != mPoolMap.end(); ++it) @@ -211,6 +212,7 @@ void LLCoprocedureManager::shutdown(bool hardShutdown) } mPoolMap.clear(); } +|*==========================================================================*/ void LLCoprocedureManager::setPropertyMethods(SettingQuery_t queryfn, SettingUpdate_t updatefn) { @@ -303,10 +305,13 @@ LLCoprocedurePool::LLCoprocedurePool(const std::string &poolName, size_t size): LLCoprocedurePool::~LLCoprocedurePool() { +/*==========================================================================*| shutdown(); +|*==========================================================================*/ } //------------------------------------------------------------------------- +/*==========================================================================*| void LLCoprocedurePool::shutdown(bool hardShutdown) { CoroAdapterMap_t::iterator it; @@ -327,6 +332,7 @@ void LLCoprocedurePool::shutdown(bool hardShutdown) mCoroMapping.clear(); mPendingCoprocs.clear(); } +|*==========================================================================*/ //------------------------------------------------------------------------- LLUUID LLCoprocedurePool::enqueueCoprocedure(const std::string &name, LLCoprocedurePool::CoProcedure_t proc) diff --git a/indra/llmessage/llcoproceduremanager.h b/indra/llmessage/llcoproceduremanager.h index 7d0e83180c..ba6f97355c 100644 --- a/indra/llmessage/llcoproceduremanager.h +++ b/indra/llmessage/llcoproceduremanager.h @@ -59,9 +59,11 @@ public: /// If it has not yet been dequeued it is simply removed from the queue. void cancelCoprocedure(const LLUUID &id); +/*==========================================================================*| /// Requests a shutdown of the upload manager. Passing 'true' will perform /// an immediate kill on the upload coroutine. void shutdown(bool hardShutdown = false); +|*==========================================================================*/ void setPropertyMethods(SettingQuery_t queryfn, SettingUpdate_t updatefn); diff --git a/indra/llprimitive/CMakeLists.txt b/indra/llprimitive/CMakeLists.txt index dd2e806dda..7b6d04b096 100644 --- a/indra/llprimitive/CMakeLists.txt +++ b/indra/llprimitive/CMakeLists.txt @@ -80,7 +80,7 @@ target_link_libraries(llprimitive ${LLXML_LIBRARIES} ${LLPHYSICSEXTENSIONS_LIBRARIES} ${LLCHARACTER_LIBRARIES} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ) diff --git a/indra/llui/CMakeLists.txt b/indra/llui/CMakeLists.txt index e44f57fa9f..2d2fa6588f 100644 --- a/indra/llui/CMakeLists.txt +++ b/indra/llui/CMakeLists.txt @@ -299,7 +299,7 @@ if(LL_TESTS) set(test_libs llui llmessage llcorehttp llcommon ${HUNSPELL_LIBRARY} ${LLCOMMON_LIBRARIES} - ${BOOST_COROUTINE_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY} ${WINDOWS_LIBRARIES}) if(NOT LINUX) LL_ADD_INTEGRATION_TEST(llurlentry llurlentry.cpp "${test_libs}") diff --git a/indra/mac_crash_logger/CMakeLists.txt b/indra/mac_crash_logger/CMakeLists.txt index f6c4dfb59d..95637c9a28 100644 --- a/indra/mac_crash_logger/CMakeLists.txt +++ b/indra/mac_crash_logger/CMakeLists.txt @@ -77,7 +77,7 @@ target_link_libraries(mac-crash-logger ${LLCOREHTTP_LIBRARIES} ${LLCOMMON_LIBRARIES} ${BOOST_CONTEXT_LIBRARY} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ) add_custom_command( diff --git a/indra/newview/CMakeLists.txt b/indra/newview/CMakeLists.txt index dc0d737540..45f4cb269c 100644 --- a/indra/newview/CMakeLists.txt +++ b/indra/newview/CMakeLists.txt @@ -2004,7 +2004,7 @@ target_link_libraries(${VIEWER_BINARY_NAME} ${viewer_LIBRARIES} ${BOOST_PROGRAM_OPTIONS_LIBRARY} ${BOOST_REGEX_LIBRARY} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${DBUSGLIB_LIBRARIES} ${OPENGL_LIBRARIES} @@ -2484,7 +2484,7 @@ if (LL_TESTS) ${OPENSSL_LIBRARIES} ${CRYPTO_LIBRARIES} ${LIBRT_LIBRARY} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ) diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index af70751b37..db2db43ee1 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -1422,6 +1422,8 @@ bool LLAppViewer::doFrame() // canonical per-frame event mainloop.post(newFrame); + // give listeners a chance to run + llcoro::suspend(); if (!LLApp::isExiting()) { diff --git a/indra/test/CMakeLists.txt b/indra/test/CMakeLists.txt index 8344cead57..4187076030 100644 --- a/indra/test/CMakeLists.txt +++ b/indra/test/CMakeLists.txt @@ -98,7 +98,7 @@ target_link_libraries(lltest ${WINDOWS_LIBRARIES} ${BOOST_PROGRAM_OPTIONS_LIBRARY} ${BOOST_REGEX_LIBRARY} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY} ${DL_LIBRARY} diff --git a/indra/viewer_components/login/CMakeLists.txt b/indra/viewer_components/login/CMakeLists.txt index 3bedeb7292..23518b791c 100644 --- a/indra/viewer_components/login/CMakeLists.txt +++ b/indra/viewer_components/login/CMakeLists.txt @@ -50,7 +50,7 @@ target_link_libraries(lllogin ${LLMATH_LIBRARIES} ${LLXML_LIBRARIES} ${BOOST_THREAD_LIBRARY} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY} ) @@ -62,7 +62,7 @@ if(LL_TESTS) set_source_files_properties( lllogin.cpp PROPERTIES - LL_TEST_ADDITIONAL_LIBRARIES "${LLMESSAGE_LIBRARIES};${LLCOREHTTP_LIBRARIES};${LLCOMMON_LIBRARIES};${BOOST_COROUTINE_LIBRARY};${BOOST_CONTEXT_LIBRARY};${BOOST_THREAD_LIBRARY};${BOOST_SYSTEM_LIBRARY}" + LL_TEST_ADDITIONAL_LIBRARIES "${LLMESSAGE_LIBRARIES};${LLCOREHTTP_LIBRARIES};${LLCOMMON_LIBRARIES};${BOOST_FIBER_LIBRARY};${BOOST_CONTEXT_LIBRARY};${BOOST_THREAD_LIBRARY};${BOOST_SYSTEM_LIBRARY}" ) LL_ADD_PROJECT_UNIT_TESTS(lllogin "${lllogin_TEST_SOURCE_FILES}") diff --git a/indra/viewer_components/login/tests/lllogin_test.cpp b/indra/viewer_components/login/tests/lllogin_test.cpp index e96c495446..774823d735 100644 --- a/indra/viewer_components/login/tests/lllogin_test.cpp +++ b/indra/viewer_components/login/tests/lllogin_test.cpp @@ -44,6 +44,7 @@ //#define DEBUG_ON #include "../../../test/debug.h" #include "llevents.h" +#include "lleventcoro.h" #include "stringize.h" #if LL_WINDOWS @@ -199,6 +200,7 @@ namespace tut credentials["passwd"] = "secret"; login.connect("login.bar.com", credentials); + llcoro::suspend(); ensure_equals("Online state", listener.lastEvent()["state"].asString(), "online"); } @@ -226,6 +228,7 @@ namespace tut credentials["passwd"] = "badpasswd"; login.connect("login.bar.com", credentials); + llcoro::suspend(); ensure_equals("Auth state", listener.lastEvent()["change"].asString(), "authenticating"); @@ -265,6 +268,7 @@ namespace tut credentials["passwd"] = "matter"; login.connect("login.bar.com", credentials); + llcoro::suspend(); ensure_equals("Auth state", listener.lastEvent()["change"].asString(), "authenticating"); @@ -300,6 +304,7 @@ namespace tut credentials["cfg_srv_timeout"] = 0.0f; login.connect("login.bar.com", credentials); + llcoro::suspend(); // Get the mainloop eventpump, which needs a pinging in order to drive the // SRV timeout. diff --git a/indra/win_crash_logger/CMakeLists.txt b/indra/win_crash_logger/CMakeLists.txt index 4fba26ab2f..1c3479bf69 100644 --- a/indra/win_crash_logger/CMakeLists.txt +++ b/indra/win_crash_logger/CMakeLists.txt @@ -83,7 +83,7 @@ target_link_libraries(windows-crash-logger ${LLCOREHTTP_LIBRARIES} ${LLCOMMON_LIBRARIES} ${BOOST_CONTEXT_LIBRARY} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${WINDOWS_LIBRARIES} ${DXGUID_LIBRARY} ${GOOGLE_PERFTOOLS_LIBRARIES} -- cgit v1.3 From 32f1dfa531062071ccf090b9c3d391b274caf02b Mon Sep 17 00:00:00 2001 From: Anchor Date: Mon, 10 Jun 2019 15:56:44 -0700 Subject: [DRTVWR-476] - fix compiler errors 32 bit windows build --- indra/llcommon/tests/lleventdispatcher_test.cpp | 4 ++-- indra/llimage/llimagejpeg.cpp | 1 - indra/llmessage/llproxy.cpp | 4 ++-- indra/llprimitive/llmodel.cpp | 2 +- indra/llui/llaccordionctrl.cpp | 2 +- indra/llvfs/lldir.cpp | 2 +- indra/newview/llpaneleditwearable.cpp | 6 +++--- indra/newview/llvovolume.cpp | 2 +- 8 files changed, 11 insertions(+), 12 deletions(-) (limited to 'indra/llui') diff --git a/indra/llcommon/tests/lleventdispatcher_test.cpp b/indra/llcommon/tests/lleventdispatcher_test.cpp index a181d5c941..efb75951be 100644 --- a/indra/llcommon/tests/lleventdispatcher_test.cpp +++ b/indra/llcommon/tests/lleventdispatcher_test.cpp @@ -436,7 +436,7 @@ namespace tut std::vector binary; for (size_t ix = 0, h = 0xaa; ix < 6; ++ix, h += 0x11) { - binary.push_back(h); + binary.push_back((U8)h); } // Full defaults arrays. We actually don't care what the LLUUID or // LLDate values are, as long as they're different from the @@ -1145,7 +1145,7 @@ namespace tut std::vector binary; for (size_t h(0x01), i(0); i < 5; h+= 0x22, ++i) { - binary.push_back(h); + binary.push_back((U8)h); } LLSD args(LLSDMap("a", LLSDArray(true)(17)(3.14)(123.456)("char*")) ("b", LLSDArray("string") diff --git a/indra/llimage/llimagejpeg.cpp b/indra/llimage/llimagejpeg.cpp index 3b1b060c02..8c89b8a705 100644 --- a/indra/llimage/llimagejpeg.cpp +++ b/indra/llimage/llimagejpeg.cpp @@ -386,7 +386,6 @@ boolean LLImageJPEG::encodeEmptyOutputBuffer( j_compress_ptr cinfo ) { self->setLastError("Out of memory in LLImageJPEG::encodeEmptyOutputBuffer( j_compress_ptr cinfo )"); LLTHROW(LLContinueError("Out of memory in LLImageJPEG::encodeEmptyOutputBuffer( j_compress_ptr cinfo )")); - return false; } memcpy( new_buffer, self->mOutputBuffer, self->mOutputBufferSize ); /* Flawfinder: ignore */ delete[] self->mOutputBuffer; diff --git a/indra/llmessage/llproxy.cpp b/indra/llmessage/llproxy.cpp index 950599217f..86bcfe6881 100644 --- a/indra/llmessage/llproxy.cpp +++ b/indra/llmessage/llproxy.cpp @@ -115,9 +115,9 @@ S32 LLProxy::proxyHandshake(LLHost proxy) U32 request_size = socks_username.size() + socks_password.size() + 3; char * password_auth = new char[request_size]; password_auth[0] = 0x01; - password_auth[1] = socks_username.size(); + password_auth[1] = (char)(socks_username.size()); memcpy(&password_auth[2], socks_username.c_str(), socks_username.size()); - password_auth[socks_username.size() + 2] = socks_password.size(); + password_auth[socks_username.size() + 2] = (char)(socks_password.size()); memcpy(&password_auth[socks_username.size() + 3], socks_password.c_str(), socks_password.size()); authmethod_password_reply_t password_reply; diff --git a/indra/llprimitive/llmodel.cpp b/indra/llprimitive/llmodel.cpp index 37548e3fe3..a2d9b4cd9b 100644 --- a/indra/llprimitive/llmodel.cpp +++ b/indra/llprimitive/llmodel.cpp @@ -1579,7 +1579,7 @@ void LLModel::Decomposition::fromLLSD(LLSD& decomp) range = max-min; - U16 count = position.size()/6; + U16 count = (U16)(position.size()/6); for (U32 j = 0; j < count; ++j) { diff --git a/indra/llui/llaccordionctrl.cpp b/indra/llui/llaccordionctrl.cpp index 623f570cef..edcbc3fbb7 100644 --- a/indra/llui/llaccordionctrl.cpp +++ b/indra/llui/llaccordionctrl.cpp @@ -338,7 +338,7 @@ void LLAccordionCtrl::addCollapsibleCtrl(LLView* view) addChild(accordion_tab); mAccordionTabs.push_back(accordion_tab); - accordion_tab->setDropDownStateChangedCallback( boost::bind(&LLAccordionCtrl::onCollapseCtrlCloseOpen, this, mAccordionTabs.size() - 1) ); + accordion_tab->setDropDownStateChangedCallback( boost::bind(&LLAccordionCtrl::onCollapseCtrlCloseOpen, this, (S16)(mAccordionTabs.size() - 1)) ); arrange(); } diff --git a/indra/llvfs/lldir.cpp b/indra/llvfs/lldir.cpp index 2076ce334e..10fbc06c61 100644 --- a/indra/llvfs/lldir.cpp +++ b/indra/llvfs/lldir.cpp @@ -1090,7 +1090,7 @@ LLDir::SepOff LLDir::needSep(const std::string& path, const std::string& name) c { // But if BOTH path and name bring a separator, we need not add one. // Moreover, we should actually skip the leading separator of 'name'. - return SepOff(false, seplen); + return SepOff(false, (unsigned short)seplen); } // Here we know that either path_ends_sep or name_starts_sep is true -- // but not both. So don't add a separator, and don't skip any characters: diff --git a/indra/newview/llpaneleditwearable.cpp b/indra/newview/llpaneleditwearable.cpp index 6573be0aaf..c601a6c210 100644 --- a/indra/newview/llpaneleditwearable.cpp +++ b/indra/newview/llpaneleditwearable.cpp @@ -778,7 +778,7 @@ BOOL LLPanelEditWearable::postBuild() LL_WARNS() << "could not get wearable dictionary entry for wearable of type: " << type << LL_ENDL; continue; } - U8 num_subparts = wearable_entry->mSubparts.size(); + U8 num_subparts = (U8)(wearable_entry->mSubparts.size()); for (U8 index = 0; index < num_subparts; ++index) { @@ -1181,7 +1181,7 @@ void LLPanelEditWearable::showWearable(LLViewerWearable* wearable, BOOL show, BO updatePanelPickerControls(type); // clear and rebuild visual param list - U8 num_subparts = wearable_entry->mSubparts.size(); + U8 num_subparts = (U8)(wearable_entry->mSubparts.size()); for (U8 index = 0; index < num_subparts; ++index) { @@ -1372,7 +1372,7 @@ void LLPanelEditWearable::updateScrollingPanelUI() const LLEditWearableDictionary::WearableEntry *wearable_entry = LLEditWearableDictionary::getInstance()->getWearable(type); llassert(wearable_entry); if (!wearable_entry) return; - U8 num_subparts = wearable_entry->mSubparts.size(); + U8 num_subparts = (U8)(wearable_entry->mSubparts.size()); LLScrollingPanelParam::sUpdateDelayFrames = 0; for (U8 index = 0; index < num_subparts; ++index) diff --git a/indra/newview/llvovolume.cpp b/indra/newview/llvovolume.cpp index 0a1efd564f..fa5938df04 100644 --- a/indra/newview/llvovolume.cpp +++ b/indra/newview/llvovolume.cpp @@ -2063,7 +2063,7 @@ void LLVOVolume::setNumTEs(const U8 num_tes) } else if(old_num_tes > num_tes && mMediaImplList.size() > num_tes) //old faces removed { - U8 end = mMediaImplList.size() ; + U8 end = (U8)(mMediaImplList.size()) ; for(U8 i = num_tes; i < end ; i++) { removeMediaImpl(i) ; -- cgit v1.3 From afaad3cef749e926cb63b6aed0c14f25d3495d55 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 15 Oct 2019 23:06:04 -0400 Subject: DRTVWR-476: Remove special case for listen(boost::bind(weak_ptr)). LLEventDetail::visit_and_connect() promised special treatment for the specific case when an LLEventPump::listen() listener was composed of (possibly nested) boost::bind() objects storing boost::weak_ptr values -- specifically boost::bind() rather than std::bind or lambdas, specifically boost::weak_ptr rather than std::weak_ptr. Outside of self-tests, it does not appear that anyone actually uses that support. There is good reason not to: it's a silent side effect of a complicated compile-time inspection that could be silently derailed by use of std::bind() or a lambda or a std::weak_ptr. Can you be sure you've engaged that promise? How? A more robust guarantee can be achieved by storing an LLTempBoundConnection in the transient object itself. When the object is destroyed, the listener is disconnected. Normal C++ rules around object destruction guarantee it. This idiom is widely used. There are a couple good reasons to remove the visit_and_connect() machinery: * boost::bind() and boost::weak_ptr do not constitute the wave of the future. Preferring those constructs to lambdas and std::weak_ptr penalizes new code, whether by silently failing or by discouraging use of modern idioms. * The visit_and_connect() machinery was always complicated, and apparently never very robust. Most of its promised features have been commented out over the years. Making the code base simpler, clearer and more maintainable is always a useful effect. LLEventDetail::visit_and_connect() was also used by the four LLNotificationChannelBase::connectMumble() methods. Streamline those as well. Of course, remove related test code. --- indra/llcommon/llevents.h | 343 +------------------------------------------ indra/llui/llnotifications.h | 34 +---- indra/test/llevents_tut.cpp | 222 ++++++---------------------- 3 files changed, 54 insertions(+), 545 deletions(-) (limited to 'indra/llui') diff --git a/indra/llcommon/llevents.h b/indra/llcommon/llevents.h index 253592256d..3c388bf176 100644 --- a/indra/llcommon/llevents.h +++ b/indra/llcommon/llevents.h @@ -309,37 +309,6 @@ testable: PumpNames mQueueNames; }; -/***************************************************************************** -* details -*****************************************************************************/ -namespace LLEventDetail -{ - /// Any callable capable of connecting an LLEventListener to an - /// LLStandardSignal to produce an LLBoundListener can be mapped to this - /// signature. - typedef boost::function ConnectFunc; - - /// overload of visit_and_connect() when we have a string identifier available - template - LLBoundListener visit_and_connect(const std::string& name, - const LISTENER& listener, - const ConnectFunc& connect_func); - /** - * Utility template function to use Visitor appropriately - * - * @param listener Callable to connect, typically a boost::bind() - * expression. This will be visited by Visitor using boost::visit_each(). - * @param connect_func Callable that will connect() @a listener to an - * LLStandardSignal, returning LLBoundListener. - */ - template - LLBoundListener visit_and_connect(const LISTENER& listener, - const ConnectFunc& connect_func) - { - return visit_and_connect("", listener, connect_func); - } -} // namespace LLEventDetail - /***************************************************************************** * LLEventTrackable *****************************************************************************/ @@ -374,11 +343,6 @@ namespace LLEventDetail * instance, it attempts to dereference the Foo* pointer that was * deleted but not zeroed.) * - Undefined behavior results. - * If you suspect you may encounter any such scenario, you're better off - * managing the lifespan of your object with boost::shared_ptr. - * Passing LLEventPump::listen() a boost::bind() expression - * involving a boost::weak_ptr is recognized specially, engaging - * thread-safe Boost.Signals2 machinery. */ typedef boost::signals2::trackable LLEventTrackable; @@ -517,44 +481,13 @@ public: * the result be assigned to a LLTempBoundListener or the listener is * manually disconnected when no longer needed since there will be no * way to later find and disconnect this listener manually. - * - * If (as is typical) you pass a boost::bind() expression as @a - * listener, listen() will inspect the components of that expression. If a - * bound object matches any of several cases, the connection will - * automatically be disconnected when that object is destroyed. - * - * * You bind a boost::weak_ptr. - * * Binding a boost::shared_ptr that way would ensure that the - * referenced object would @em never be destroyed, since the @c - * shared_ptr stored in the LLEventPump would remain an outstanding - * reference. Use the weaken() function to convert your @c shared_ptr to - * @c weak_ptr. Because this is easy to forget, binding a @c shared_ptr - * will produce a compile error (@c BOOST_STATIC_ASSERT failure). - * * You bind a simple pointer or reference to an object derived from - * boost::enable_shared_from_this. (UNDER CONSTRUCTION) - * * You bind a simple pointer or reference to an object derived from - * LLEventTrackable. Unlike the cases described above, though, this is - * vulnerable to a couple of cross-thread race conditions, as described - * in the LLEventTrackable documentation. */ - template - LLBoundListener listen(const std::string& name, const LISTENER& listener, + LLBoundListener listen(const std::string& name, + const LLEventListener& listener, const NameList& after=NameList(), const NameList& before=NameList()) { - // Examine listener, using our listen_impl() method to make the - // actual connection. - // This is why listen() is a template. Conversion from boost::bind() - // to LLEventListener performs type erasure, so it's important to look - // at the boost::bind object itself before that happens. - return LLEventDetail::visit_and_connect(name, - listener, - boost::bind(&LLEventPump::listen_invoke, - this, - name, - _1, - after, - before)); + return listen_impl(name, listener, after, before); } /// Get the LLBoundListener associated with the passed name (dummy @@ -598,13 +531,6 @@ private: private: - LLBoundListener listen_invoke(const std::string& name, const LLEventListener& listener, - const NameList& after, - const NameList& before) - { - return this->listen_impl(name, listener, after, before); - } - // must precede mName; see LLEventPump::LLEventPump() LLHandle mRegistry; @@ -814,261 +740,6 @@ private: LL_COMMON_API bool sendReply(const LLSD& reply, const LLSD& request, const std::string& replyKey="reply"); -/***************************************************************************** -* Underpinnings -*****************************************************************************/ -/** - * We originally provided a suite of overloaded - * LLEventTrackable::listenTo(LLEventPump&, ...) methods that would call - * LLEventPump::listen(...) and then pass the returned LLBoundListener to - * LLEventTrackable::track(). This was workable but error-prone: the coder - * must remember to call listenTo() rather than the more straightforward - * listen() method. - * - * Now we publish only the single canonical listen() method, so there's a - * uniform mechanism. Having a single way to do this is good, in that there's - * no question in the coder's mind which of several alternatives to choose. - * - * To support automatic connection management, we use boost::visit_each - * (http://www.boost.org/doc/libs/1_37_0/doc/html/boost/visit_each.html) to - * inspect each argument of a boost::bind expression. (Although the visit_each - * mechanism was first introduced with the original Boost.Signals library, it - * was only later documented.) - * - * Cases: - * * At least one of the function's arguments is a boost::weak_ptr. Pass - * the corresponding shared_ptr to slot_type::track(). Ideally that would be - * the object whose method we want to call, but in fact we do the same for - * any weak_ptr we might find among the bound arguments. If we're passing - * our bound method a weak_ptr to some object, wouldn't the destruction of - * that object invalidate the call? So we disconnect automatically when any - * such object is destroyed. This is the mechanism preferred by boost:: - * signals2. - * * One of the functions's arguments is a boost::shared_ptr. This produces - * a compile error: the bound copy of the shared_ptr stored in the - * boost_bind object stored in the signal object would make the referenced - * T object immortal. We provide a weaken() function. Pass - * weaken(your_shared_ptr) instead. (We can inspect, but not modify, the - * boost::bind object. Otherwise we'd replace the shared_ptr with weak_ptr - * implicitly and just proceed.) - * * One of the function's arguments is a plain pointer/reference to an object - * derived from boost::enable_shared_from_this. We assume that this object - * is managed using boost::shared_ptr, so we implicitly extract a shared_ptr - * and track that. (UNDER CONSTRUCTION) - * * One of the function's arguments is derived from LLEventTrackable. Pass - * the LLBoundListener to its LLEventTrackable::track(). This is vulnerable - * to a couple different race conditions, as described in LLEventTrackable - * documentation. (NOTE: Now that LLEventTrackable is a typedef for - * boost::signals2::trackable, the Signals2 library handles this itself, so - * our visitor needs no special logic for this case.) - * * Any other argument type is irrelevant to automatic connection management. - */ - -namespace LLEventDetail -{ - template - const F& unwrap(const F& f) { return f; } - - template - const F& unwrap(const boost::reference_wrapper& f) { return f.get(); } - - // Most of the following is lifted from the Boost.Signals use of - // visit_each. - template struct truth {}; - - /** - * boost::visit_each() Visitor, used on a template argument const F& - * f as follows (see visit_and_connect()): - * @code - * LLEventListener listener(f); - * Visitor visitor(listener); // bind listener so it can track() shared_ptrs - * using boost::visit_each; // allow unqualified visit_each() call for ADL - * visit_each(visitor, unwrap(f)); - * @endcode - */ - class Visitor - { - public: - /** - * Visitor binds a reference to LLEventListener so we can track() any - * shared_ptrs we find in the argument list. - */ - Visitor(LLEventListener& listener): - mListener(listener) - { - } - - /** - * boost::visit_each() calls this method for each component of a - * boost::bind() expression. - */ - template - void operator()(const T& t) const - { - decode(t, 0); - } - - private: - // decode() decides between a reference wrapper and anything else - // boost::ref() variant - template - void decode(const boost::reference_wrapper& t, int) const - { -// add_if_trackable(t.get_pointer()); - } - - // decode() anything else - template - void decode(const T& t, long) const - { - typedef truth<(boost::is_pointer::value)> is_a_pointer; - maybe_get_pointer(t, is_a_pointer()); - } - - // maybe_get_pointer() decides between a pointer and a non-pointer - // plain pointer variant - template - void maybe_get_pointer(const T& t, truth) const - { -// add_if_trackable(t); - } - - // shared_ptr variant - template - void maybe_get_pointer(const boost::shared_ptr& t, truth) const - { - // If we have a shared_ptr to this object, it doesn't matter - // whether the object is derived from LLEventTrackable, so no - // further analysis of T is needed. -// mListener.track(t); - - // Make this case illegal. Passing a bound shared_ptr to - // slot_type::track() is useless, since the bound shared_ptr will - // keep the object alive anyway! Force the coder to cast to weak_ptr. - - // Trivial as it is, make the BOOST_STATIC_ASSERT() condition - // dependent on template param so the macro is only evaluated if - // this method is in fact instantiated, as described here: - // http://www.boost.org/doc/libs/1_34_1/doc/html/boost_staticassert.html - - // ATTENTION: Don't bind a shared_ptr using - // LLEventPump::listen(boost::bind()). Doing so captures a copy of - // the shared_ptr, making the referenced object effectively - // immortal. Use the weaken() function, e.g.: - // somepump.listen(boost::bind(...weaken(my_shared_ptr)...)); - // This lets us automatically disconnect when the referenced - // object is destroyed. - BOOST_STATIC_ASSERT(sizeof(T) == 0); - } - - // weak_ptr variant - template - void maybe_get_pointer(const boost::weak_ptr& t, truth) const - { - // If we have a weak_ptr to this object, it doesn't matter - // whether the object is derived from LLEventTrackable, so no - // further analysis of T is needed. - mListener.track(t); -// std::cout << "Found weak_ptr<" << typeid(T).name() << ">!\n"; - } - -#if 0 - // reference to anything derived from boost::enable_shared_from_this - template - inline void maybe_get_pointer(const boost::enable_shared_from_this& ct, - truth) const - { - // Use the slot_type::track(shared_ptr) mechanism. Cast away - // const-ness because (in our code base anyway) it's unusual - // to find shared_ptr. - boost::enable_shared_from_this& - t(const_cast&>(ct)); - std::cout << "Capturing shared_from_this()" << std::endl; - boost::shared_ptr sp(t.shared_from_this()); -/*==========================================================================*| - std::cout << "Capturing weak_ptr" << std::endl; - boost::weak_ptr wp(sp); -|*==========================================================================*/ - std::cout << "Tracking shared__ptr" << std::endl; - mListener.track(sp); - } -#endif - - // non-pointer variant - template - void maybe_get_pointer(const T& t, truth) const - { - // Take the address of this object, because the object itself may be - // trackable -// add_if_trackable(boost::addressof(t)); - } - -/*==========================================================================*| - // add_if_trackable() adds LLEventTrackable objects to mTrackables - inline void add_if_trackable(const LLEventTrackable* t) const - { - if (t) - { - } - } - - // pointer to anything not an LLEventTrackable subclass - inline void add_if_trackable(const void*) const - { - } - - // pointer to free function - // The following construct uses the preprocessor to generate - // add_if_trackable() overloads accepting pointer-to-function taking - // 0, 1, ..., LLEVENTS_LISTENER_ARITY parameters of arbitrary type. -#define BOOST_PP_LOCAL_MACRO(n) \ - template \ - inline void \ - add_if_trackable(R (*)(BOOST_PP_ENUM_PARAMS(n, T))) const \ - { \ - } -#define BOOST_PP_LOCAL_LIMITS (0, LLEVENTS_LISTENER_ARITY) -#include BOOST_PP_LOCAL_ITERATE() -#undef BOOST_PP_LOCAL_MACRO -#undef BOOST_PP_LOCAL_LIMITS -|*==========================================================================*/ - - /// Bind a reference to the LLEventListener to call its track() method. - LLEventListener& mListener; - }; - - /** - * Utility template function to use Visitor appropriately - * - * @param raw_listener Callable to connect, typically a boost::bind() - * expression. This will be visited by Visitor using boost::visit_each(). - * @param connect_funct Callable that will connect() @a raw_listener to an - * LLStandardSignal, returning LLBoundListener. - */ - template - LLBoundListener visit_and_connect(const std::string& name, - const LISTENER& raw_listener, - const ConnectFunc& connect_func) - { - // Capture the listener - LLEventListener listener(raw_listener); - // Define our Visitor, binding the listener so we can call - // listener.track() if we discover any shared_ptr. - LLEventDetail::Visitor visitor(listener); - // Allow unqualified visit_each() call for ADL - using boost::visit_each; - // Visit each component of a boost::bind() expression. Pass - // 'raw_listener', our template argument, rather than 'listener' from - // which type details have been erased. unwrap() comes from - // Boost.Signals, in case we were passed a boost::ref(). - visit_each(visitor, LLEventDetail::unwrap(raw_listener)); - // Make the connection using passed function. - return connect_func(listener); - } -} // namespace LLEventDetail - // Somewhat to my surprise, passing boost::bind(...boost::weak_ptr...) to // listen() fails in Boost code trying to instantiate LLEventListener (i.e. // LLStandardSignal::slot_type) because the boost::get_pointer() utility function isn't @@ -1079,12 +750,4 @@ namespace boost T* get_pointer(const weak_ptr& ptr) { return shared_ptr(ptr).get(); } } -/// Since we forbid use of listen(boost::bind(...shared_ptr...)), provide an -/// easy way to cast to the corresponding weak_ptr. -template -boost::weak_ptr weaken(const boost::shared_ptr& ptr) -{ - return boost::weak_ptr(ptr); -} - #endif /* ! defined(LL_LLEVENTS_H) */ diff --git a/indra/llui/llnotifications.h b/indra/llui/llnotifications.h index 62cf41256b..cac687f53d 100644 --- a/indra/llui/llnotifications.h +++ b/indra/llui/llnotifications.h @@ -746,42 +746,24 @@ public: virtual ~LLNotificationChannelBase() {} // you can also connect to a Channel, so you can be notified of // changes to this channel - template - LLBoundListener connectChanged(const LISTENER& slot) + LLBoundListener connectChanged(const LLEventListener& slot) { - // Examine slot to see if it binds an LLEventTrackable subclass, or a - // boost::shared_ptr to something, or a boost::weak_ptr to something. // Call this->connectChangedImpl() to actually connect it. - return LLEventDetail::visit_and_connect(slot, - boost::bind(&LLNotificationChannelBase::connectChangedImpl, - this, - _1)); + return connectChangedImpl(slot); } - template - LLBoundListener connectAtFrontChanged(const LISTENER& slot) + LLBoundListener connectAtFrontChanged(const LLEventListener& slot) { - return LLEventDetail::visit_and_connect(slot, - boost::bind(&LLNotificationChannelBase::connectAtFrontChangedImpl, - this, - _1)); + return connectAtFrontChangedImpl(slot); } - template - LLBoundListener connectPassedFilter(const LISTENER& slot) + LLBoundListener connectPassedFilter(const LLEventListener& slot) { // see comments in connectChanged() - return LLEventDetail::visit_and_connect(slot, - boost::bind(&LLNotificationChannelBase::connectPassedFilterImpl, - this, - _1)); + return connectPassedFilterImpl(slot); } - template - LLBoundListener connectFailedFilter(const LISTENER& slot) + LLBoundListener connectFailedFilter(const LLEventListener& slot) { // see comments in connectChanged() - return LLEventDetail::visit_and_connect(slot, - boost::bind(&LLNotificationChannelBase::connectFailedFilterImpl, - this, - _1)); + return connectFailedFilterImpl(slot); } // use this when items change or to add a new one diff --git a/indra/test/llevents_tut.cpp b/indra/test/llevents_tut.cpp index 9ff0f5086d..a8a3188249 100644 --- a/indra/test/llevents_tut.cpp +++ b/indra/test/llevents_tut.cpp @@ -492,196 +492,60 @@ void events_object::test<10>() heaptest.stopListening("temp"); } -template<> template<> -void events_object::test<11>() -{ - set_test_name("listen(boost::bind(...weak_ptr...))"); - // listen() detecting weak_ptr in boost::bind() object - bool live = false; - LLEventPump& heaptest(pumps.obtain("heaptest")); - LLBoundListener connection; - ensure("default state", !connection.connected()); - { - boost::shared_ptr newListener(new TempListener("heap", live)); - newListener->reset(); - ensure("TempListener constructed", live); - connection = heaptest.listen(newListener->getName(), - boost::bind(&Listener::call, - weaken(newListener), - _1)); - ensure("new connection", connection.connected()); - heaptest.post(1); - check_listener("received", *newListener, 1); - } // presumably this will make newListener go away? - // verify that - ensure("TempListener destroyed", !live); - ensure("implicit disconnect", !connection.connected()); - // now just make sure we don't blow up trying to access a freed object! - heaptest.post(2); -} - -template<> template<> -void events_object::test<12>() -{ - set_test_name("listen(boost::bind(...shared_ptr...))"); - /*==========================================================================*| - // DISABLED because I've made this case produce a compile error. - // Following the error leads the disappointed dev to a comment - // instructing her to use the weaken() function to bind a weak_ptr - // instead of binding a shared_ptr, and explaining why. I know of - // no way to use TUT to code a repeatable test in which the expected - // outcome is a compile error. The interested reader is invited to - // uncomment this block and build to see for herself. - - // listen() detecting shared_ptr in boost::bind() object - bool live = false; - LLEventPump& heaptest(pumps.obtain("heaptest")); - LLBoundListener connection; - std::string listenerName("heap"); - ensure("default state", !connection.connected()); - { - boost::shared_ptr newListener(new TempListener(listenerName, live)); - ensure_equals("use_count", newListener.use_count(), 1); - newListener->reset(); - ensure("TempListener constructed", live); - connection = heaptest.listen(newListener->getName(), - boost::bind(&Listener::call, newListener, _1)); - ensure("new connection", connection.connected()); - ensure_equals("use_count", newListener.use_count(), 2); - heaptest.post(1); - check_listener("received", *newListener, 1); - } // this should make newListener go away... - // Unfortunately, the fact that we've bound a shared_ptr by value into - // our LLEventPump means that copy will keep the referenced object alive. - ensure("TempListener still alive", live); - ensure("still connected", connection.connected()); - // disconnecting explicitly should delete the TempListener... - heaptest.stopListening(listenerName); -#if 0 // however, in my experience, it does not. I don't know why not. - // Ah: on 2009-02-19, Frank Mori Hess, author of the Boost.Signals2 - // library, stated on the boost-users mailing list: - // http://www.nabble.com/Re%3A--signals2--review--The-review-of-the-signals2-library-(formerly-thread_safe_signals)-begins-today%2C-Nov-1st-p22102367.html - // "It will get destroyed eventually. The signal cleans up its slot - // list little by little during connect/invoke. It doesn't immediately - // remove disconnected slots from the slot list since other threads - // might be using the same slot list concurrently. It might be - // possible to make it immediately reset the shared_ptr owning the - // slot though, leaving an empty shared_ptr in the slot list, since - // that wouldn't invalidate any iterators." - ensure("TempListener destroyed", ! live); - ensure("implicit disconnect", ! connection.connected()); -#endif // 0 - // now just make sure we don't blow up trying to access a freed object! - heaptest.post(2); -|*==========================================================================*/ -} - class TempTrackableListener: public TempListener, public LLEventTrackable { public: -TempTrackableListener(const std::string& name, bool& liveFlag): - TempListener(name, liveFlag) -{} + TempTrackableListener(const std::string& name, bool& liveFlag): + TempListener(name, liveFlag) + {} }; template<> template<> -void events_object::test<13>() -{ -set_test_name("listen(boost::bind(...TempTrackableListener ref...))"); -bool live = false; -LLEventPump& heaptest(pumps.obtain("heaptest")); -LLBoundListener connection; +void events_object::test<11>() { - TempTrackableListener tempListener("temp", live); - ensure("TempTrackableListener constructed", live); - connection = heaptest.listen(tempListener.getName(), - boost::bind(&TempTrackableListener::call, - boost::ref(tempListener), _1)); - heaptest.post(1); - check_listener("received", tempListener, 1); -} // presumably this will make tempListener go away? -// verify that -ensure("TempTrackableListener destroyed", ! live); -ensure("implicit disconnect", ! connection.connected()); -// now just make sure we don't blow up trying to access a freed object! -heaptest.post(2); + set_test_name("listen(boost::bind(...TempTrackableListener ref...))"); + bool live = false; + LLEventPump& heaptest(pumps.obtain("heaptest")); + LLBoundListener connection; + { + TempTrackableListener tempListener("temp", live); + ensure("TempTrackableListener constructed", live); + connection = heaptest.listen(tempListener.getName(), + boost::bind(&TempTrackableListener::call, + boost::ref(tempListener), _1)); + heaptest.post(1); + check_listener("received", tempListener, 1); + } // presumably this will make tempListener go away? + // verify that + ensure("TempTrackableListener destroyed", ! live); + ensure("implicit disconnect", ! connection.connected()); + // now just make sure we don't blow up trying to access a freed object! + heaptest.post(2); } template<> template<> -void events_object::test<14>() -{ -set_test_name("listen(boost::bind(...TempTrackableListener pointer...))"); -bool live = false; -LLEventPump& heaptest(pumps.obtain("heaptest")); -LLBoundListener connection; +void events_object::test<12>() { - TempTrackableListener* newListener(new TempTrackableListener("temp", live)); - ensure("TempTrackableListener constructed", live); - connection = heaptest.listen(newListener->getName(), - boost::bind(&TempTrackableListener::call, - newListener, _1)); - heaptest.post(1); - check_listener("received", *newListener, 1); - // explicitly destroy newListener - delete newListener; -} -// verify that -ensure("TempTrackableListener destroyed", ! live); -ensure("implicit disconnect", ! connection.connected()); -// now just make sure we don't blow up trying to access a freed object! -heaptest.post(2); + set_test_name("listen(boost::bind(...TempTrackableListener pointer...))"); + bool live = false; + LLEventPump& heaptest(pumps.obtain("heaptest")); + LLBoundListener connection; + { + TempTrackableListener* newListener(new TempTrackableListener("temp", live)); + ensure("TempTrackableListener constructed", live); + connection = heaptest.listen(newListener->getName(), + boost::bind(&TempTrackableListener::call, + newListener, _1)); + heaptest.post(1); + check_listener("received", *newListener, 1); + // explicitly destroy newListener + delete newListener; + } + // verify that + ensure("TempTrackableListener destroyed", ! live); + ensure("implicit disconnect", ! connection.connected()); + // now just make sure we don't blow up trying to access a freed object! + heaptest.post(2); } -class TempSharedListener: public TempListener, -public boost::enable_shared_from_this -{ -public: -TempSharedListener(const std::string& name, bool& liveFlag): - TempListener(name, liveFlag) -{} -}; - -template<> template<> -void events_object::test<15>() -{ - set_test_name("listen(boost::bind(...TempSharedListener ref...))"); -#if 0 -bool live = false; -LLEventPump& heaptest(pumps.obtain("heaptest")); -LLBoundListener connection; -{ - // We MUST have at least one shared_ptr to an - // enable_shared_from_this subclass object before - // shared_from_this() can work. - boost::shared_ptr - tempListener(new TempSharedListener("temp", live)); - ensure("TempSharedListener constructed", live); - // However, we're not passing either the shared_ptr or its - // corresponding weak_ptr -- instead, we're passing a reference to - // the TempSharedListener. -/*==========================================================================*| - std::cout << "Capturing const ref" << std::endl; - const boost::enable_shared_from_this& cref(*tempListener); - std::cout << "Capturing const ptr" << std::endl; - const boost::enable_shared_from_this* cp(&cref); - std::cout << "Capturing non-const ptr" << std::endl; - boost::enable_shared_from_this* p(const_cast*>(cp)); - std::cout << "Capturing shared_from_this()" << std::endl; - boost::shared_ptr sp(p->shared_from_this()); - std::cout << "Capturing weak_ptr" << std::endl; - boost::weak_ptr wp(weaken(sp)); - std::cout << "Binding weak_ptr" << std::endl; -|*==========================================================================*/ - connection = heaptest.listen(tempListener->getName(), - boost::bind(&TempSharedListener::call, *tempListener, _1)); - heaptest.post(1); - check_listener("received", *tempListener, 1); -} // presumably this will make tempListener go away? -// verify that -ensure("TempSharedListener destroyed", ! live); -ensure("implicit disconnect", ! connection.connected()); -// now just make sure we don't blow up trying to access a freed object! -heaptest.post(2); -#endif // 0 -} } // namespace tut -- cgit v1.3