Merged in nat_linden/viewer-azumarill-vivox (pull request #2)

MAINT-5976: Fix bug in LLCoros::set_consuming() mechanism.

5 files changed, 219 insertions, 119 deletions

diff --git a/indra/llcommon/llcoros.h b/indra/llcommon/llcoros.h
index 0b1f58f48e..f2fb7f1e70 100755
--- a/indra/llcommon/llcoros.h
+++ b/indra/llcommon/llcoros.h
@@ -30,6 +30,7 @@
 #define LL_LLCOROS_H
 
 #include <boost/dcoroutine/coroutine.hpp>
+#include <boost/dcoroutine/future.hpp>
 #include "llsingleton.h"
 #include <boost/ptr_container/ptr_map.hpp>
 #include <boost/function.hpp>
@@ -162,6 +163,15 @@ public:
     static void set_consuming(bool consuming);
     static bool get_consuming();
 
+    /**
+     * 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.
+     */
+    template <typename T>
+    class Future;
+
 private:
     LLCoros();
     friend class LLSingleton<LLCoros>;
@@ -233,4 +243,42 @@ private:
 
 } // namespace llcoro
 
+template <typename T>
+class LLCoros::Future
+{
+    typedef boost::dcoroutines::future<T> dfuture;
+
+public:
+    Future():
+        mFuture(get_self())
+    {}
+
+    typedef typename boost::dcoroutines::make_callback_result<dfuture>::type callback_t;
+
+    callback_t make_callback()
+    {
+        return boost::dcoroutines::make_callback(mFuture);
+    }
+
+    explicit 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;
+};
+
 #endif /* ! defined(LL_LLCOROS_H) */
diff --git a/indra/llcommon/lleventcoro.cpp b/indra/llcommon/lleventcoro.cpp
index 9b7e8fb65f..578a2b62c8 100755
--- a/indra/llcommon/lleventcoro.cpp
+++ b/indra/llcommon/lleventcoro.cpp
@@ -34,12 +34,11 @@
 #include <map>
 // std headers
 // external library headers
-#include <boost/dcoroutine/coroutine.hpp>
-#include <boost/dcoroutine/future.hpp>
 // other Linden headers
 #include "llsdserialize.h"
 #include "llerror.h"
 #include "llcoros.h"
+#include "llmake.h"
 
 #include "lleventfilter.h"
 
@@ -145,6 +144,36 @@ void storeToLLSDPath(LLSD& dest, const LLSD& rawPath, const LLSD& value)
     *pdest = value;
 }
 
+/// For LLCoros::Future<LLSD>::make_callback(), the callback has a signature
+/// like void callback(LLSD), which isn't a valid LLEventPump listener: such
+/// listeners must return bool.
+template <typename LISTENER>
+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()
@@ -167,13 +196,13 @@ LLSD llcoro::postAndSuspend(const LLSD& event, const LLEventPumpOrPumpName& requ
                  const LLEventPumpOrPumpName& replyPump, const LLSD& replyPumpNamePath)
 {
     // declare the future
-    boost::dcoroutines::future<LLSD_consumed> future(LLCoros::get_self());
+    LLCoros::Future<LLSD> future;
     // 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(
         replyPump.getPump().listen(listenerName,
-                                   voidlistener(boost::dcoroutines::make_callback(future))));
+                                   llmake<FutureListener>(future.make_callback())));
     // skip the "post" part if requestPump is default-constructed
     if (requestPump)
     {
@@ -192,67 +221,56 @@ LLSD llcoro::postAndSuspend(const LLSD& event, const LLEventPumpOrPumpName& requ
     LL_DEBUGS("lleventcoro") << "postAndSuspend(): coroutine " << listenerName
                              << " about to wait on LLEventPump " << replyPump.getPump().getName()
                              << LL_ENDL;
-    // trying to dereference ("resolve") the future makes us wait for it
-    LLSD_consumed value;
-    {
-        // instantiate Suspending to manage the "current" coroutine
-        llcoro::Suspending suspended;
-        value = *future;
-    } // destroy Suspending as soon as we're back
+    // calling get() on the future makes us wait for it
+    LLSD value(future.get());
     LL_DEBUGS("lleventcoro") << "postAndSuspend(): coroutine " << listenerName
-                             << " resuming with " << value.first << LL_ENDL;
-    // immediately set consumed according to consuming
-    *value.second = LLCoros::get_consuming();
+                             << " resuming with " << value << LL_ENDL;
     // returning should disconnect the connection
-    return value.first;
+    return value;
 }
 
 namespace
 {
 
-typedef std::pair<LLEventWithID, bool*> LLEventWithID_consumed;
-
 /**
- * This helper is specifically for the two-pump version of suspendUntilEventOn().
- * 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 VoidListener for the purpose. The
- * basic idea is that we construct a distinct instance of WaitForEventOnHelper
- * -- binding different instance data -- for each of the pumps. Then, when a
- * pump delivers an LLSD value to either WaitForEventOnHelper, it can combine
- * that LLSD with its discriminator to feed the future object.
+ * 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 <typename LISTENER>
-class WaitForEventOnHelper
+template <typename LISTENER, typename DISCRIM>
+class FutureListener2: public FutureListener<LISTENER>
 {
+    typedef FutureListener<LISTENER> super;
+
 public:
-    WaitForEventOnHelper(const LISTENER& listener, int discriminator):
-        mListener(listener),
+    // instantiated on coroutine stack: the stack about to suspend
+    FutureListener2(const LISTENER& listener, DISCRIM discriminator):
+        super(listener),
         mDiscrim(discriminator)
     {}
 
-    // this signature is required for an LLEventPump listener
+    // called on main stack: the stack on which event is fired
     bool operator()(const LLSD& event)
     {
-        bool consumed = false;
-        // our future object is defined to accept LLEventWithID_consumed
-        mListener(LLEventWithID_consumed(LLEventWithID(event, mDiscrim), &consumed));
+        // our future object is defined to accept LLEventWithID
+        super::mListener(LLEventWithID(event, mDiscrim));
         // tell LLEventPump whether or not event was consumed
-        return consumed;
+        return super::mConsume;
     }
+
 private:
-    LISTENER mListener;
-    const int mDiscrim;
+    const DISCRIM mDiscrim;
 };
 
-/// WaitForEventOnHelper type-inference helper
-template <typename LISTENER>
-WaitForEventOnHelper<LISTENER> wfeoh(const LISTENER& listener, int discriminator)
-{
-    return WaitForEventOnHelper<LISTENER>(listener, discriminator);
-}
-
 } // anonymous
 
 namespace llcoro
@@ -266,16 +284,18 @@ LLEventWithID postAndSuspend2(const LLSD& event,
                            const LLSD& replyPump1NamePath)
 {
     // declare the future
-    boost::dcoroutines::future<LLEventWithID_consumed> future(LLCoros::get_self());
+    LLCoros::Future<LLEventWithID> 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",
-                                    wfeoh(boost::dcoroutines::make_callback(future), 0)));
+        replyPump0.getPump().listen(
+            name + "a",
+            llmake<FutureListener2>(future.make_callback(), 0)));
     LLTempBoundListener connection1(
-        replyPump1.getPump().listen(name + "b",
-                                    wfeoh(boost::dcoroutines::make_callback(future), 1)));
+        replyPump1.getPump().listen(
+            name + "b",
+            llmake<FutureListener2>(future.make_callback(), 1)));
     // skip the "post" part if requestPump is default-constructed
     if (requestPump)
     {
@@ -294,20 +314,13 @@ LLEventWithID postAndSuspend2(const LLSD& event,
     LL_DEBUGS("lleventcoro") << "postAndSuspend2(): coroutine " << name
                              << " about to wait on LLEventPumps " << replyPump0.getPump().getName()
                              << ", " << replyPump1.getPump().getName() << LL_ENDL;
-    // trying to dereference ("resolve") the future makes us wait for it
-    LLEventWithID_consumed value;
-    {
-        // instantiate Suspending to manage "current" coroutine
-        llcoro::Suspending suspended;
-        value = *future;
-    } // destroy Suspending as soon as we're back
+    // calling get() on the future makes us wait for it
+    LLEventWithID value(future.get());
     LL_DEBUGS("lleventcoro") << "postAndSuspend(): coroutine " << name
-                             << " resuming with (" << value.first.first
-                             << ", " << value.first.second << ")" << LL_ENDL;
-    // tell LLEventPump whether we're consuming
-    *value.second = LLCoros::get_consuming();
+                             << " resuming with (" << value.first << ", " << value.second << ")"
+                             << LL_ENDL;
     // returning should disconnect both connections
-    return value.first;
+    return value;
 }
 
 LLSD errorException(const LLEventWithID& result, const std::string& desc)
diff --git a/indra/llcommon/lleventcoro.h b/indra/llcommon/lleventcoro.h
index acf2ad24a4..2105faf861 100755
--- a/indra/llcommon/lleventcoro.h
+++ b/indra/llcommon/lleventcoro.h
@@ -76,36 +76,6 @@ private:
 namespace llcoro
 {
 
-typedef std::pair<LLSD, bool*> LLSD_consumed;
-
-/// This is an adapter for a signature like void LISTENER(const LLSD&), which
-/// isn't a valid LLEventPump listener: such listeners should return bool.
-template <typename LISTENER>
-class VoidListener
-{
-public:
-    VoidListener(const LISTENER& listener):
-        mListener(listener)
-    {}
-
-    bool operator()(const LLSD& event)
-    {
-        bool consumed = false;
-        mListener(LLSD_consumed(event, &consumed));
-        // tell upstream LLEventPump whether listener consumed
-        return consumed;
-    }
-private:
-    LISTENER mListener;
-};
-
-/// VoidListener helper function to infer the type of the LISTENER
-template <typename LISTENER>
-VoidListener<LISTENER> voidlistener(const LISTENER& listener)
-{
-    return VoidListener<LISTENER>(listener);
-}
-
 /**
  * Yield control from a coroutine for one "mainloop" tick. If your coroutine
  * runs without suspending for nontrivial time, sprinkle in calls to this
diff --git a/indra/llcommon/llmake.h b/indra/llcommon/llmake.h
new file mode 100644
index 0000000000..9a662a0640
--- /dev/null
+++ b/indra/llcommon/llmake.h
@@ -0,0 +1,63 @@
+/**
+ * @file   llmake.h
+ * @author Nat Goodspeed
+ * @date   2015-12-18
+ * @brief  Generic llmake<Template>(arg) function to instantiate
+ *         Template<decltype(arg)>(arg).
+ *
+ *         Many of our class templates have an accompanying helper function to
+ *         make an instance with arguments of arbitrary type. llmake()
+ *         eliminates the need to declare a new helper function for every such
+ *         class template.
+ * 
+ *         also relevant:
+ *
+ *         Template parameter deduction for constructors
+ *         http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0091r0.html
+ *
+ *         https://github.com/viboes/std-make
+ *
+ *         but obviously we're not there yet.
+ *
+ * $LicenseInfo:firstyear=2015&license=viewerlgpl$
+ * Copyright (c) 2015, Linden Research, Inc.
+ * $/LicenseInfo$
+ */
+
+#if ! defined(LL_LLMAKE_H)
+#define LL_LLMAKE_H
+
+/*==========================================================================*|
+// When we allow ourselves to compile with C++11 features enabled, this form
+// should generically handle an arbitrary number of arguments.
+
+template <template<typename...> class CLASS_TEMPLATE, typename... ARGS>
+CLASS_TEMPLATE<ARGS...> llmake(ARGS && ... args)
+{
+    return CLASS_TEMPLATE<ARGS...>(std::forward<ARGS>(args)...);
+}
+|*==========================================================================*/
+
+// As of 2015-12-18, this is what we'll use instead. Add explicit overloads
+// for different numbers of template parameters as use cases arise.
+
+/**
+ * Usage: llmake<SomeTemplate>(arg)
+ *
+ * Deduces the type T of 'arg' and returns an instance of SomeTemplate<T>
+ * initialized with 'arg'. Assumes a constructor accepting T (by value,
+ * reference or whatever).
+ */
+template <template<typename> class CLASS_TEMPLATE, typename ARG1>
+CLASS_TEMPLATE<ARG1> llmake(const ARG1& arg1)
+{
+    return CLASS_TEMPLATE<ARG1>(arg1);
+}
+
+template <template<typename, typename> class CLASS_TEMPLATE, typename ARG1, typename ARG2>
+CLASS_TEMPLATE<ARG1, ARG2> llmake(const ARG1& arg1, const ARG2& arg2)
+{
+    return CLASS_TEMPLATE<ARG1, ARG2>(arg1, arg2);
+}
+
+#endif /* ! defined(LL_LLMAKE_H) */
diff --git a/indra/llcommon/tests/lleventcoro_test.cpp b/indra/llcommon/tests/lleventcoro_test.cpp
index da1439418f..a459d17fb8 100755
--- a/indra/llcommon/tests/lleventcoro_test.cpp
+++ b/indra/llcommon/tests/lleventcoro_test.cpp
@@ -65,14 +65,11 @@
 // 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.
-#define BOOST_RESULT_OF_USE_TR1 1
 #include <boost/dcoroutine/coroutine.hpp>
-// Normally, lleventcoro.h obviates future.hpp. We only include this because
-// we implement a "by hand" test of future functionality.
-#include <boost/dcoroutine/future.hpp>
 #include <boost/bind.hpp>
 #include <boost/range.hpp>
 #include <boost/utility.hpp>
+#include <boost/shared_ptr.hpp>
 
 #include "linden_common.h"
 
@@ -218,7 +215,7 @@ namespace tut
     // use static data so we can intersperse coroutine functions with the
     // tests that engage them
     ImmediateAPI immediateAPI;
-    std::string replyName, errorName, threw;
+    std::string replyName, errorName, threw, stringdata;
     LLSD result, errordata;
     int which;
 
@@ -228,28 +225,40 @@ namespace tut
         replyName.clear();
         errorName.clear();
         threw.clear();
+        stringdata.clear();
         result = LLSD();
         errordata = LLSD();
         which = 0;
     }
 
-    void explicit_wait(boost::dcoroutines::coroutine<void()>::self& self)
+    void explicit_wait(boost::shared_ptr<LLCoros::Future<std::string>::callback_t>& cbp)
     {
         BEGIN
         {
-            // ... do whatever preliminary stuff must happen ...
+            // The point of this test is to verify / illustrate suspending a
+            // coroutine for something other than an LLEventPump. In other
+            // words, this shows how to adapt to any async operation that
+            // provides a callback-style notification (and prove that it
+            // works).
+
+            LLCoros::Future<std::string> future;
+            // get the callback from that future
+            LLCoros::Future<std::string>::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
+            // adapter) off to some I/O subsystem, we'll just pass it back to
+            // our caller.
+            cbp.reset(new LLCoros::Future<std::string>::callback_t(callback));
 
-            // declare the future
-            boost::dcoroutines::future<llcoro::LLSD_consumed> future(self);
-            // tell the future what to suspend for
-            LLTempBoundListener connection(
-                LLEventPumps::instance().obtain("source").listen("coro", voidlistener(boost::dcoroutines::make_callback(future))));
             ensure("Not yet", ! future);
-            // attempting to dereference ("resolve") the future causes the calling
-            // coroutine to suspend for it
+            // calling get() on the future causes us to suspend
             debug("about to suspend");
-            result = (*future).first;
-            ensure("Got it", future);
+            stringdata = future.get();
+            ensure("Got it", bool(future));
         }
         END
     }
@@ -262,19 +271,16 @@ namespace tut
         DEBUG;
 
         // Construct the coroutine instance that will run explicit_wait.
-        // Pass the ctor a callable that accepts the coroutine_type::self
-        // param passed by the library.
-        boost::dcoroutines::coroutine<void()> coro(explicit_wait);
-        // Start the coroutine
-        coro(std::nothrow);
-        // When the coroutine waits for the event pump, it returns here.
-        debug("about to send");
-        // Satisfy the suspend.
-        LLEventPumps::instance().obtain("source").post("received");
-        // Now suspend for the coroutine to complete.
-        ensure("coroutine complete", ! coro);
+        boost::shared_ptr<LLCoros::Future<std::string>::callback_t> respond;
+        LLCoros::instance().launch("test<2>",
+                                   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");
         // ensure the coroutine ran and woke up again with the intended result
-        ensure_equals(result.asString(), "received");
+        ensure_equals(stringdata, "received");
     }
 
     void waitForEventOn1()