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).

1 files changed, 88 insertions, 209 deletions

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 <boost/bind.hpp>
+#include <boost/fiber/fiber.hpp>
+#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 <boost/fiber/protected_fixedsize_stack.hpp>
+#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 <excpt.h>
 #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<LLCoros::CoroData> 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<CoroData&>(const_cast<const LLCoros*>(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<std::string&>(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);
+}