From 820d4a20d1b9c9a3e562b925ba59a6addcffa558 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 4 Jun 2009 16:01:40 +0000 Subject: DEV-32777: Use a canonical boost::coroutines::coroutine signature, relying on boost::bind() to pass any other coroutine arguments. This allows us to remove the LLCoroBase and LLCoro constructs, directly storing a coroutine object in our ptr_map. It also allows us to remove the multiple launch() overloads for multiple arguments. Finally, it lets us move most launch() functionality into a non-template method. --- indra/llcommon/llcoros.cpp | 17 +++- indra/llcommon/llcoros.h | 136 +++++++++--------------------- indra/viewer_components/login/lllogin.cpp | 10 +-- 3 files changed, 58 insertions(+), 105 deletions(-) diff --git a/indra/llcommon/llcoros.cpp b/indra/llcommon/llcoros.cpp index 6fa6ae8f1a..5d23e1d284 100644 --- a/indra/llcommon/llcoros.cpp +++ b/indra/llcommon/llcoros.cpp @@ -54,6 +54,15 @@ bool LLCoros::cleanup(const LLSD&) return false; } +std::string LLCoros::launchImpl(const std::string& prefix, coro* newCoro) +{ + std::string name(generateDistinctName(prefix)); + mCoros.insert(name, newCoro); + /* Run the coroutine until its first wait, then return here */ + (*newCoro)(std::nothrow); + return name; +} + std::string LLCoros::generateDistinctName(const std::string& prefix) const { // Allowing empty name would make getName()'s not-found return ambiguous. @@ -86,8 +95,8 @@ bool LLCoros::kill(const std::string& name) return false; } // Because this is a boost::ptr_map, erasing the map entry also destroys - // the referenced heap object, in this case an LLCoro. That will destroy - // the contained boost::coroutine object, which will terminate the coroutine. + // the referenced heap object, in this case the boost::coroutine object, + // which will terminate the coroutine. mCoros.erase(found); return true; } @@ -98,7 +107,9 @@ std::string LLCoros::getNameByID(const void* self_id) const // passed to us comes. for (CoroMap::const_iterator mi(mCoros.begin()), mend(mCoros.end()); mi != mend; ++mi) { - if (mi->second->owns_self_id(self_id)) + namespace coro_private = boost::coroutines::detail; + if (static_cast(coro_private::coroutine_accessor::get_impl(const_cast(*mi->second)).get()) + == self_id) { return mi->first; } diff --git a/indra/llcommon/llcoros.h b/indra/llcommon/llcoros.h index dfbe76932c..6b07ba4105 100644 --- a/indra/llcommon/llcoros.h +++ b/indra/llcommon/llcoros.h @@ -21,53 +21,16 @@ #include #include -/// Base class for each coroutine -struct LLCoroBase -{ - LLCoroBase() {} - virtual ~LLCoroBase() {} - - virtual bool exited() const = 0; - template - bool owns_self(const COROUTINE_SELF& self) const - { - return owns_self_id(self.get_id()); - } - - virtual bool owns_self_id(const void* self_id) const = 0; -}; - -/// Template subclass to accommodate different boost::coroutine signatures -template -struct LLCoro: public LLCoroBase -{ - template - LLCoro(const CALLABLE& callable): - mCoro(callable) - {} - - virtual bool exited() const { return mCoro.exited(); } - - COROUTINE mCoro; - - virtual bool owns_self_id(const void* self_id) const - { - namespace coro_private = boost::coroutines::detail; - return static_cast(coro_private::coroutine_accessor::get_impl(const_cast(mCoro)).get()) - == self_id; - } -}; - /** * Registry of named Boost.Coroutine instances * - * The Boost.Coroutine library supports the general case of a coroutine accepting - * arbitrary parameters and yielding multiple (sets of) results. For such use - * cases, it's natural for the invoking code to retain the coroutine instance: - * the consumer repeatedly calls back into the coroutine until it yields its - * next result. + * The Boost.Coroutine library supports the general case of a coroutine + * accepting arbitrary parameters and yielding multiple (sets of) results. For + * such use cases, it's natural for the invoking code to retain the coroutine + * instance: the consumer repeatedly calls into the coroutine, perhaps passing + * new parameter values, prompting it to yield its next result. * - * Our typical coroutine usage is a bit different, though. For us, coroutines + * Our typical coroutine usage is different, though. For us, coroutines * provide an alternative to the @c Responder pattern. Our typical coroutine * has @c void return, invoked in fire-and-forget mode: the handler for some * user gesture launches the coroutine and promptly returns to the main loop. @@ -98,7 +61,11 @@ struct LLCoro: public LLCoroBase class LLCoros: public LLSingleton { public: - /*------------------------------ launch() ------------------------------*/ + /// Canonical boost::coroutines::coroutine signature we use + typedef boost::coroutines::coroutine coro; + /// Canonical 'self' type + typedef coro::self self; + /** * Create and start running a new coroutine with specified name. The name * string you pass is a suggestion; it will be tweaked for uniqueness. The @@ -106,68 +73,44 @@ public: * * Usage looks like this, for (e.g.) two coroutine parameters: * @code - * typedef boost::coroutines::coroutine coro_type; - * std::string name = LLCoros::instance().launch( - * "mycoro", boost::bind(&MyClass::method, this, _1, _2, _3), - * "somestring", LLSD(17)); + * class MyClass + * { + * public: + * ... + * // Do NOT NOT NOT accept reference params other than 'self'! + * // Pass by value only! + * void myCoroutineMethod(LLCoros::self& self, std::string, LLSD); + * ... + * }; + * ... + * std::string name = LLCoros::instance().launch( + * "mycoro", boost::bind(&MyClass::myCoroutineMethod, this, _1, + * "somestring", LLSD(17)); * @endcode * - * In other words, you must specify: + * Your function/method must accept LLCoros::self& as its first parameter. + * It can accept any other parameters you want -- but ONLY BY VALUE! + * Other reference parameters are a BAD IDEA! You Have Been Warned. See + * DEV-32777 comments for an explanation. * - * * the desired boost::coroutines::coroutine type, to whose - * signature the initial coro_type::self& parameter is - * implicitly added - * * the suggested name string for the new coroutine instance - * * the callable to be run, e.g. boost::bind() expression for a - * class method -- not forgetting to add _1 for the - * coro_type::self& parameter - * * the actual parameters to be passed to that callable after the - * implicit coro_type::self& parameter + * Pass a callable that accepts the single LLCoros::self& parameter. It + * may work to pass a free function whose only parameter is 'self'; for + * all other cases use boost::bind(). Of course, for a non-static class + * method, the first parameter must be the class instance. Use the + * placeholder _1 for the 'self' parameter. Any other parameters should be + * passed via the bind() expression. * * launch() tweaks the suggested name so it won't collide with any * existing coroutine instance, creates the coroutine instance, registers * it with the tweaked name and runs it until its first wait. At that * point it returns the tweaked name. - * - * Use of a typedef for the coroutine type is recommended, because you - * must restate it for the callable's first parameter. - * - * @note - * launch() only accepts const-reference parameters. Once we can assume - * C++0x features on every platform, we'll have so-called "perfect - * forwarding" and variadic templates and other such ponies, and can - * support an arbitrary number of truly arbitrary parameter types. But for - * now, we'll stick with const reference params. N.B. Passing a non-const - * reference to a local variable into a coroutine seems like a @em really - * bad idea: the local variable will be destroyed during the lifetime of - * the coroutine. */ - // Use the preprocessor to generate launch() overloads accepting 0, 1, - // ..., BOOST_COROUTINE_ARG_MAX const ref params of arbitrary type. -#define BOOST_PP_LOCAL_MACRO(n) \ - template \ - std::string launch(const std::string& prefix, const CALLABLE& callable \ - BOOST_PP_COMMA_IF(n) \ - BOOST_PP_ENUM_BINARY_PARAMS(n, const T, & p)) \ - { \ - std::string name(generateDistinctName(prefix)); \ - LLCoro* ptr = new LLCoro(callable); \ - mCoros.insert(name, ptr); \ - /* Run the coroutine until its first wait, then return here */ \ - ptr->mCoro(std::nothrow \ - BOOST_PP_COMMA_IF(n) \ - BOOST_PP_ENUM_PARAMS(n, p)); \ - return name; \ + template + std::string launch(const std::string& prefix, const CALLABLE& callable) + { + return launchImpl(prefix, new coro(callable)); } -#define BOOST_PP_LOCAL_LIMITS (0, BOOST_COROUTINE_ARG_MAX) -#include BOOST_PP_LOCAL_ITERATE() -#undef BOOST_PP_LOCAL_MACRO -#undef BOOST_PP_LOCAL_LIMITS - /*----------------------- end of launch() family -----------------------*/ - /** * Abort a running coroutine by name. Normally, when a coroutine either * runs to completion or terminates with an exception, LLCoros quietly @@ -195,10 +138,11 @@ public: private: friend class LLSingleton; LLCoros(); + std::string launchImpl(const std::string& prefix, coro* newCoro); std::string generateDistinctName(const std::string& prefix) const; bool cleanup(const LLSD&); - typedef boost::ptr_map CoroMap; + typedef boost::ptr_map CoroMap; CoroMap mCoros; }; diff --git a/indra/viewer_components/login/lllogin.cpp b/indra/viewer_components/login/lllogin.cpp index 92384a1c5b..c0d35f31d3 100644 --- a/indra/viewer_components/login/lllogin.cpp +++ b/indra/viewer_components/login/lllogin.cpp @@ -105,12 +105,10 @@ private: return response; } - typedef boost::coroutines::coroutine coroutine_type; - // In a coroutine's top-level function args, do NOT NOT NOT accept // references (const or otherwise) to anything but the self argument! Pass // by value only! - void login_(coroutine_type::self& self, std::string uri, LLSD credentials); + void login_(LLCoros::self& self, std::string uri, LLSD credentials); LLEventStream mPump; LLSD mAuthResponse, mValidAuthResponse; @@ -121,11 +119,11 @@ void LLLogin::Impl::connect(const std::string& uri, const LLSD& credentials) // Launch a coroutine with our login_() method. Run the coroutine until // its first wait; at that point, return here. std::string coroname = - LLCoros::instance().launch("LLLogin::Impl::login_", - boost::bind(&Impl::login_, this, _1, uri, credentials)); + LLCoros::instance().launch("LLLogin::Impl::login_", + boost::bind(&Impl::login_, this, _1, uri, credentials)); } -void LLLogin::Impl::login_(coroutine_type::self& self, std::string uri, LLSD credentials) +void LLLogin::Impl::login_(LLCoros::self& self, std::string uri, LLSD credentials) { LL_INFOS("LLLogin") << "Entering coroutine " << LLCoros::instance().getName(self) << " with uri '" << uri << "', credentials " << credentials << LL_ENDL; -- cgit v1.2.3