DEV-32777: Introduce LLCoros, an LLSingleton registry of named coroutine

instances. LLCoros::launch() intends to address three issues:
- ownership of coroutine instance
- cleanup of coroutine instance when it terminates
- central place to twiddle MSVC optimizations to bypass DEV-32777 crash.
Initially coded on Mac; will address the third bullet on Windows.
Adapt listenerNameForCoro() to consult LLCoros::getName() if applicable.
Change LLLogin::Impl::connect() to use LLCoros::launch().
LLCoros::getName() relies on patch to boost::coroutines::coroutine::self to
introduce get_id().

7 files changed, 350 insertions, 28 deletions

diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt
index 71ec6cb8e4..bbfadbb344 100644
--- a/indra/llcommon/CMakeLists.txt
+++ b/indra/llcommon/CMakeLists.txt
@@ -26,6 +26,7 @@ set(llcommon_SOURCE_FILES
     llbase32.cpp
     llbase64.cpp
     llcommon.cpp
+    llcoros.cpp
     llcrc.cpp
     llcriticaldamp.cpp
     llcursortypes.cpp
@@ -102,6 +103,7 @@ set(llcommon_HEADER_FILES
     llchat.h
     llclickaction.h
     llcommon.h
+    llcoros.h
     llcrc.h
     llcriticaldamp.h
     llcursortypes.h
diff --git a/indra/llcommon/llcoros.cpp b/indra/llcommon/llcoros.cpp
new file mode 100644
index 0000000000..6fa6ae8f1a
--- /dev/null
+++ b/indra/llcommon/llcoros.cpp
@@ -0,0 +1,107 @@
+/**
+ * @file   llcoros.cpp
+ * @author Nat Goodspeed
+ * @date   2009-06-03
+ * @brief  Implementation for llcoros.
+ * 
+ * $LicenseInfo:firstyear=2009&license=viewergpl$
+ * Copyright (c) 2009, Linden Research, Inc.
+ * $/LicenseInfo$
+ */
+
+// Precompiled header
+#include "linden_common.h"
+// associated header
+#include "llcoros.h"
+// STL headers
+// std headers
+// external library headers
+#include <boost/bind.hpp>
+// other Linden headers
+#include "llevents.h"
+#include "llerror.h"
+#include "stringize.h"
+
+LLCoros::LLCoros()
+{
+    // Register our cleanup() method for "mainloop" ticks
+    LLEventPumps::instance().obtain("mainloop").listen(
+        "LLCoros", boost::bind(&LLCoros::cleanup, this, _1));
+}
+
+bool LLCoros::cleanup(const LLSD&)
+{
+    // 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->exited())
+        {
+            LL_INFOS("LLCoros") << "LLCoros: cleaning up coroutine " << mi->first << 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
+{
+    // Allowing empty name would make getName()'s not-found return ambiguous.
+    if (prefix.empty())
+    {
+        LL_ERRS("LLCoros") << "LLCoros::launch(): pass non-empty name string" << LL_ENDL;
+    }
+
+    // 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++))
+    {
+        if (mCoros.find(name) == mCoros.end())
+        {
+            LL_INFOS("LLCoros") << "LLCoros: launching coroutine " << name << LL_ENDL;
+            return name;
+        }
+    }
+}
+
+bool LLCoros::kill(const std::string& name)
+{
+    CoroMap::iterator found = mCoros.find(name);
+    if (found == mCoros.end())
+    {
+        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.
+    mCoros.erase(found);
+    return true;
+}
+
+std::string LLCoros::getNameByID(const void* self_id) const
+{
+    // Walk the existing coroutines, looking for one from which the 'self_id'
+    // 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))
+        {
+            return mi->first;
+        }
+    }
+    return "";
+}
diff --git a/indra/llcommon/llcoros.h b/indra/llcommon/llcoros.h
new file mode 100644
index 0000000000..dfbe76932c
--- /dev/null
+++ b/indra/llcommon/llcoros.h
@@ -0,0 +1,205 @@
+/**
+ * @file   llcoros.h
+ * @author Nat Goodspeed
+ * @date   2009-06-02
+ * @brief  Manage running boost::coroutine instances
+ * 
+ * $LicenseInfo:firstyear=2009&license=viewergpl$
+ * Copyright (c) 2009, Linden Research, Inc.
+ * $/LicenseInfo$
+ */
+
+#if ! defined(LL_LLCOROS_H)
+#define LL_LLCOROS_H
+
+#include <boost/coroutine/coroutine.hpp>
+#include "llsingleton.h"
+#include <boost/ptr_container/ptr_map.hpp>
+#include <string>
+#include <boost/preprocessor/repetition/enum_params.hpp>
+#include <boost/preprocessor/repetition/enum_binary_params.hpp>
+#include <boost/preprocessor/iteration/local.hpp>
+#include <stdexcept>
+
+/// Base class for each coroutine
+struct LLCoroBase
+{
+    LLCoroBase() {}
+    virtual ~LLCoroBase() {}
+
+    virtual bool exited() const = 0;
+    template <typename COROUTINE_SELF>
+    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 <typename COROUTINE>
+struct LLCoro: public LLCoroBase
+{
+    template <typename CALLABLE>
+    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<void*>(coro_private::coroutine_accessor::get_impl(const_cast<COROUTINE&>(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.
+ *
+ * Our typical coroutine usage is a bit 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.
+ * The coroutine initiates some action that will take multiple frames (e.g. a
+ * capability request), waits for its result, processes it and silently steals
+ * away.
+ *
+ * This usage poses two (related) problems:
+ *
+ * # Who should own the coroutine instance? If it's simply local to the
+ *   handler code that launches it, return from the handler will destroy the
+ *   coroutine object, terminating the coroutine.
+ * # Once the coroutine terminates, in whatever way, who's responsible for
+ *   cleaning up the coroutine object?
+ *
+ * LLCoros is a Singleton collection of currently-active coroutine instances.
+ * Each has a name. You ask LLCoros to launch a new coroutine with a suggested
+ * 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
+ * currently-running coroutine.
+ *
+ * Finally, the next frame ("mainloop" event) after the coroutine terminates,
+ * LLCoros will notice its demise and destroy it.
+ */
+class LLCoros: public LLSingleton<LLCoros>
+{
+public:
+    /*------------------------------ launch() ------------------------------*/
+    /**
+     * 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
+     * actual name is returned to you.
+     *
+     * Usage looks like this, for (e.g.) two coroutine parameters:
+     * @code
+     * typedef boost::coroutines::coroutine<void(const std::string&, const LLSD&)> coro_type;
+     * std::string name = LLCoros::instance().launch<coro_type>(
+     *    "mycoro", boost::bind(&MyClass::method, this, _1, _2, _3),
+     *    "somestring", LLSD(17));
+     * @endcode
+     *
+     * In other words, you must specify:
+     *
+     * * the desired <tt>boost::coroutines::coroutine</tt> type, to whose
+     *   signature the initial <tt>coro_type::self&</tt> parameter is
+     *   implicitly added
+     * * the suggested name string for the new coroutine instance
+     * * the callable to be run, e.g. <tt>boost::bind()</tt> expression for a
+     *   class method -- not forgetting to add _1 for the
+     *   <tt>coro_type::self&</tt> parameter
+     * * the actual parameters to be passed to that callable after the
+     *   implicit <tt>coro_type::self&</tt> parameter
+     *
+     * 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 <typename COROUTINE, typename CALLABLE                     \
+              BOOST_PP_COMMA_IF(n)                                      \
+              BOOST_PP_ENUM_PARAMS(n, typename T)>                      \
+    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<COROUTINE>* ptr = new LLCoro<COROUTINE>(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;                                                    \
+    }
+
+#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
+     * cleans it up. This is for use only when you must explicitly interrupt
+     * one prematurely. Returns @c true if the specified name was found and
+     * still running at the time.
+     */
+    bool kill(const std::string& name);
+
+    /**
+     * From within a coroutine, pass its @c self object to look up the
+     * (tweaked) name string by which this coroutine is registered. Returns
+     * the empty string if not found (e.g. if the coroutine was launched by
+     * hand rather than using LLCoros::launch()).
+     */
+    template <typename COROUTINE_SELF>
+    std::string getName(const COROUTINE_SELF& self) const
+    {
+        return getNameByID(self.get_id());
+    }
+
+    /// getName() by self.get_id()
+    std::string getNameByID(const void* self_id) const;
+
+private:
+    friend class LLSingleton<LLCoros>;
+    LLCoros();
+    std::string generateDistinctName(const std::string& prefix) const;
+    bool cleanup(const LLSD&);
+
+    typedef boost::ptr_map<std::string, LLCoroBase> CoroMap;
+    CoroMap mCoros;
+};
+
+#endif /* ! defined(LL_LLCOROS_H) */
diff --git a/indra/llcommon/lleventcoro.cpp b/indra/llcommon/lleventcoro.cpp
index cea5a1eda3..d598f1cc4a 100644
--- a/indra/llcommon/lleventcoro.cpp
+++ b/indra/llcommon/lleventcoro.cpp
@@ -20,20 +20,31 @@
 // other Linden headers
 #include "llsdserialize.h"
 #include "llerror.h"
+#include "llcoros.h"
 
-std::string LLEventDetail::listenerNameForCoro(const void* self)
+std::string LLEventDetail::listenerNameForCoroImpl(const void* self_id)
 {
+    // First, if this coroutine was launched by LLCoros::launch(), find that name.
+    std::string name(LLCoros::instance().getNameByID(self_id));
+    if (! name.empty())
+    {
+        return name;
+    }
+    // Apparently this coroutine wasn't launched by LLCoros::launch(). Check
+    // whether we have a memo for this self_id.
     typedef std::map<const void*, std::string> MapType;
     static MapType memo;
-    MapType::const_iterator found = memo.find(self);
+    MapType::const_iterator found = memo.find(self_id);
     if (found != memo.end())
     {
         // this coroutine instance has called us before, reuse same name
         return found->second;
     }
     // this is the first time we've been called for this coroutine instance
-    std::string name(LLEventPump::inventName("coro"));
-    memo[self] = name;
+    name = LLEventPump::inventName("coro");
+    memo[self_id] = name;
+    LL_INFOS("LLEventCoro") << "listenerNameForCoroImpl(" << self_id << "): inventing coro name '"
+                            << name << "'" << LL_ENDL;
     return name;
 }
 
diff --git a/indra/llcommon/lleventcoro.h b/indra/llcommon/lleventcoro.h
index 5726ea0f65..c6d9de171d 100644
--- a/indra/llcommon/lleventcoro.h
+++ b/indra/llcommon/lleventcoro.h
@@ -106,7 +106,14 @@ namespace LLEventDetail
      * that's okay, since it won't collide with any listener name used by the
      * earlier coroutine since that earlier coroutine no longer exists.
      */
-    LL_COMMON_API std::string listenerNameForCoro(const void* self);
+    template <typename COROUTINE_SELF>
+    std::string listenerNameForCoro(COROUTINE_SELF& self)
+    {
+        return listenerNameForCoroImpl(self.get_id());
+    }
+
+    /// Implementation for listenerNameForCoro()
+    LL_COMMON_API std::string listenerNameForCoroImpl(const void* self_id);
 
     /**
      * Implement behavior described for postAndWait()'s @a replyPumpNamePath
@@ -185,7 +192,7 @@ LLSD postAndWait(SELF& self, const LLSD& event, const LLEventPumpOrPumpName& req
     boost::coroutines::future<LLSD> future(self);
     // make a callback that will assign a value to the future, and listen on
     // the specified LLEventPump with that callback
-    std::string listenerName(LLEventDetail::listenerNameForCoro(&self));
+    std::string listenerName(LLEventDetail::listenerNameForCoro(self));
     LLTempBoundListener connection(
         replyPump.getPump().listen(listenerName,
                                    voidlistener(boost::coroutines::make_callback(future))));
@@ -307,7 +314,7 @@ LLEventWithID postAndWait2(SELF& self, const LLSD& event,
     boost::coroutines::future<LLEventWithID> future(self);
     // either callback will assign a value to this future; listen on
     // each specified LLEventPump with a callback
-    std::string name(LLEventDetail::listenerNameForCoro(&self));
+    std::string name(LLEventDetail::listenerNameForCoro(self));
     LLTempBoundListener connection0(
         replyPump0.getPump().listen(name + "a",
                                LLEventDetail::wfeoh(boost::coroutines::make_callback(future), 0)));
diff --git a/indra/viewer_components/login/lllogin.cpp b/indra/viewer_components/login/lllogin.cpp
index 26b950b618..575a709761 100644
--- a/indra/viewer_components/login/lllogin.cpp
+++ b/indra/viewer_components/login/lllogin.cpp
@@ -44,8 +44,8 @@
 #include "lllogin.h"
 
 #include <boost/bind.hpp>
-#include <boost/scoped_ptr.hpp>
 
+#include "llcoros.h"
 #include "llevents.h"
 #include "lleventfilter.h"
 #include "lleventcoro.h"
@@ -109,35 +109,25 @@ private:
 
     void login_(coroutine_type::self& self, const std::string& uri, const LLSD& credentials);
 
-    boost::scoped_ptr<coroutine_type> mCoro;
     LLEventStream mPump;
 	LLSD mAuthResponse, mValidAuthResponse;
 };
 
 void LLLogin::Impl::connect(const std::string& uri, const LLSD& credentials)
 {
-    // If there's a previous coroutine instance, and that instance is still
-    // active, destroying the instance will terminate the coroutine by
-    // throwing an exception, thus unwinding the stack and destroying all
-    // local objects. It should (!) all Just Work. Nonetheless, it would be
-    // strange, so make a note of it.
-    if (mCoro && *mCoro)
-    {
-        LL_WARNS("LLLogin") << "Previous login attempt interrupted by new request" << LL_ENDL;
-    }
-
-    // Construct a coroutine that will run our login_() method; placeholders
-    // forward the params from the (*mCoro)(etc.) call below. Using scoped_ptr
-    // ensures that if mCoro was already pointing to a previous instance, that
-    // old instance will be destroyed as noted above.
-    mCoro.reset(new coroutine_type(boost::bind(&Impl::login_, this, _1, _2, _3)));
-    // Run the coroutine until its first wait; at that point, return here.
-    (*mCoro)(std::nothrow, uri, credentials);
+    // Launch a coroutine with our login_() method; placeholders forward the
+    // params. Run the coroutine until its first wait; at that point, return
+    // here.
+    std::string coroname = 
+        LLCoros::instance().launch<coroutine_type>("LLLogin::Impl::login_",
+                                                   boost::bind(&Impl::login_, this, _1, _2, _3),
+                                                   uri, credentials);
 }
 
 void LLLogin::Impl::login_(coroutine_type::self& self,
                            const std::string& uri, const LLSD& credentials)
 {
+    LL_INFOS("LLLogin") << "Entering coroutine " << LLCoros::instance().getName(self) << LL_ENDL;
     // Arriving in SRVRequest state
     LLEventStream replyPump("reply", true);
     // Should be an array of one or more uri strings.
diff --git a/install.xml b/install.xml
index 0183193734..cb1ed9aae7 100644
--- a/install.xml
+++ b/install.xml
@@ -207,9 +207,9 @@
           <key>darwin</key>
           <map>
             <key>md5sum</key>
-            <string>95dda5da1fb66b690a03944fca1b2c53</string>
+            <string>10579844b84c3038ceb3706abc60c3b0</string>
             <key>url</key>
-            <uri>http://s3.amazonaws.com/viewer-source-downloads/install_pkgs/boost-1.34.1-darwin-20090427.tar.bz2</uri>
+            <uri>http://s3.amazonaws.com/viewer-source-downloads/install_pkgs/boost-1.34.1-darwin-20090603.tar.bz2</uri>
           </map>
           <key>linux</key>
           <map>