DRTVWR-476: Add Sync class to help with stepwise coroutine tests.

Sync is specifically intended for test programs. It is based on an
LLScalarCond<int>. The idea is that each of two coroutines can watch for the
other to get a chance to run, indicated by incrementing the wrapped int and
notifying the wrapped condition_variable. This is less hand-wavy than calling
llcoro::suspend() and hoping that the other routine will have had a chance to
run.

Use Sync in lleventcoro_test.cpp.

Also refactor lleventcoro_test.cpp so that instead of a collection of static
data requiring a clear() call at start of each individual test function, the
relevant data is all part of the test_data struct common to all test
functions. Make the helper coroutine functions members of test_data too.

Introduce llcoro::logname(), a convenience function to log the name of the
currently executing coroutine or "main" if in the thread's main coroutine.

4 files changed, 148 insertions, 53 deletions

diff --git a/indra/llcommon/llcoros.h b/indra/llcommon/llcoros.h
index 678633497d..dedb6c8eca 100644
--- a/indra/llcommon/llcoros.h
+++ b/indra/llcommon/llcoros.h
@@ -239,4 +239,17 @@ private:
     static void delete_CoroData(CoroData* cdptr);
 };
 
+namespace llcoro
+{
+
+inline
+std::string logname()
+{
+    static std::string main("main");
+    std::string name(LLCoros::instance().getName());
+    return name.empty()? main : name;
+}
+
+} // llcoro
+
 #endif /* ! defined(LL_LLCOROS_H) */
diff --git a/indra/llcommon/tests/lleventcoro_test.cpp b/indra/llcommon/tests/lleventcoro_test.cpp
index 2e4b6ba823..4e774b27d9 100644
--- a/indra/llcommon/tests/lleventcoro_test.cpp
+++ b/indra/llcommon/tests/lleventcoro_test.cpp
@@ -45,6 +45,7 @@
 #include "llcoros.h"
 #include "lleventcoro.h"
 #include "../test/debug.h"
+#include "../test/sync.h"
 
 using namespace llcoro;
 
@@ -58,8 +59,9 @@ using namespace llcoro;
 class ImmediateAPI
 {
 public:
-    ImmediateAPI():
-        mPump("immediate", true)
+    ImmediateAPI(Sync& sync):
+        mPump("immediate", true),
+        mSync(sync)
     {
         mPump.listen("API", boost::bind(&ImmediateAPI::operator(), this, _1));
     }
@@ -68,22 +70,18 @@ public:
 
     // Invoke this with an LLSD map containing:
     // ["value"]: Integer value. We will reply with ["value"] + 1.
-    // ["reply"]: Name of LLEventPump on which to send success response.
-    // ["error"]: Name of LLEventPump on which to send error response.
-    // ["fail"]: Presence of this key selects ["error"], else ["success"] as
-    // the name of the pump on which to send the response.
+    // ["reply"]: Name of LLEventPump on which to send response.
     bool operator()(const LLSD& event) const
     {
+        mSync.bump();
         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();
+        LLEventPumps::instance().obtain(event["reply"]).post(value + 1);
         return false;
     }
 
 private:
     LLEventStream mPump;
+    Sync& mSync;
 };
 
 /*****************************************************************************
@@ -91,34 +89,29 @@ private:
 *****************************************************************************/
 namespace tut
 {
-    struct coroutine_data {};
-    typedef test_group<coroutine_data> coroutine_group;
+    struct test_data
+    {
+        Sync mSync;
+        ImmediateAPI immediateAPI{mSync};
+        std::string replyName, errorName, threw, stringdata;
+        LLSD result, errordata;
+        int which;
+
+        void explicit_wait(boost::shared_ptr<LLCoros::Promise<std::string>>& cbp);
+        void waitForEventOn1();
+        void coroPump();
+        void postAndWait1();
+        void coroPumpPost();
+    };
+    typedef test_group<test_data> coroutine_group;
     typedef coroutine_group::object object;
     coroutine_group coroutinegrp("coroutine");
 
-    // use static data so we can intersperse coroutine functions with the
-    // tests that engage them
-    ImmediateAPI immediateAPI;
-    std::string replyName, errorName, threw, stringdata;
-    LLSD result, errordata;
-    int which;
-
-    // reinit vars at the start of each test
-    void clear()
-    {
-        replyName.clear();
-        errorName.clear();
-        threw.clear();
-        stringdata.clear();
-        result = LLSD();
-        errordata = LLSD();
-        which = 0;
-    }
-
-    void explicit_wait(boost::shared_ptr<LLCoros::Promise<std::string>>& cbp)
+    void test_data::explicit_wait(boost::shared_ptr<LLCoros::Promise<std::string>>& cbp)
     {
         BEGIN
         {
+            mSync.bump();
             // 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
@@ -136,6 +129,7 @@ namespace tut
             // calling get() on the future causes us to suspend
             debug("about to suspend");
             stringdata = future.get();
+            mSync.bump();
             ensure_equals("Got it", stringdata, "received");
         }
         END
@@ -144,30 +138,32 @@ namespace tut
     template<> template<>
     void object::test<1>()
     {
-        clear();
         set_test_name("explicit_wait");
         DEBUG;
 
         // Construct the coroutine instance that will run explicit_wait.
         boost::shared_ptr<LLCoros::Promise<std::string>> respond;
         LLCoros::instance().launch("test<1>",
-                                   boost::bind(explicit_wait, boost::ref(respond)));
+                                   [this, &respond](){ explicit_wait(respond); });
+        mSync.bump();
         // 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 should make
         // the coroutine ready.
         respond->set_value("received");
         // but give it a chance to wake up
-        llcoro::suspend();
+        mSync.yield();
         // ensure the coroutine ran and woke up again with the intended result
         ensure_equals(stringdata, "received");
     }
 
-    void waitForEventOn1()
+    void test_data::waitForEventOn1()
     {
         BEGIN
         {
+            mSync.bump();
             result = suspendUntilEventOn("source");
+            mSync.bump();
         }
         END
     }
@@ -175,25 +171,27 @@ namespace tut
     template<> template<>
     void object::test<2>()
     {
-        clear();
         set_test_name("waitForEventOn1");
         DEBUG;
-        LLCoros::instance().launch("test<2>", waitForEventOn1);
+        LLCoros::instance().launch("test<2>", [this](){ waitForEventOn1(); });
+        mSync.bump();
         debug("about to send");
         LLEventPumps::instance().obtain("source").post("received");
         // give waitForEventOn1() a chance to run
-        llcoro::suspend();
+        mSync.yield();
         debug("back from send");
         ensure_equals(result.asString(), "received");
     }
 
-    void coroPump()
+    void test_data::coroPump()
     {
         BEGIN
         {
+            mSync.bump();
             LLCoroEventPump waiter;
             replyName = waiter.getName();
             result = waiter.suspend();
+            mSync.bump();
         }
         END
     }
@@ -201,26 +199,28 @@ namespace tut
     template<> template<>
     void object::test<3>()
     {
-        clear();
         set_test_name("coroPump");
         DEBUG;
-        LLCoros::instance().launch("test<3>", coroPump);
+        LLCoros::instance().launch("test<3>", [this](){ coroPump(); });
+        mSync.bump();
         debug("about to send");
         LLEventPumps::instance().obtain(replyName).post("received");
         // give coroPump() a chance to run
-        llcoro::suspend();
+        mSync.yield();
         debug("back from send");
         ensure_equals(result.asString(), "received");
     }
 
-    void postAndWait1()
+    void test_data::postAndWait1()
     {
         BEGIN
         {
+            mSync.bump();
             result = postAndSuspend(LLSDMap("value", 17),       // request event
                                  immediateAPI.getPump(),     // requestPump
                                  "reply1",                   // replyPump
                                  "reply");                   // request["reply"] = name
+            mSync.bump();
         }
         END
     }
@@ -228,22 +228,21 @@ namespace tut
     template<> template<>
     void object::test<4>()
     {
-        clear();
         set_test_name("postAndWait1");
         DEBUG;
-        LLCoros::instance().launch("test<4>", postAndWait1);
-        // give postAndWait1() a chance to run
-        llcoro::suspend();
+        LLCoros::instance().launch("test<4>", [this](){ postAndWait1(); });
         ensure_equals(result.asInteger(), 18);
     }
 
-    void coroPumpPost()
+    void test_data::coroPumpPost()
     {
         BEGIN
         {
+            mSync.bump();
             LLCoroEventPump waiter;
             result = waiter.postAndSuspend(LLSDMap("value", 17),
                                         immediateAPI.getPump(), "reply");
+            mSync.bump();
         }
         END
     }
@@ -251,12 +250,9 @@ namespace tut
     template<> template<>
     void object::test<5>()
     {
-        clear();
         set_test_name("coroPumpPost");
         DEBUG;
-        LLCoros::instance().launch("test<5>", coroPumpPost);
-        // give coroPumpPost() a chance to run
-        llcoro::suspend();
+        LLCoros::instance().launch("test<5>", [this](){ coroPumpPost(); });
         ensure_equals(result.asInteger(), 18);
     }
 }
diff --git a/indra/test/CMakeLists.txt b/indra/test/CMakeLists.txt
index 0f14862cba..87536e146b 100644
--- a/indra/test/CMakeLists.txt
+++ b/indra/test/CMakeLists.txt
@@ -67,6 +67,7 @@ set(test_HEADER_FILES
     llpipeutil.h
     llsdtraits.h
     lltut.h
+    sync.h
     )
 
 if (NOT WINDOWS)
diff --git a/indra/test/sync.h b/indra/test/sync.h
new file mode 100644
index 0000000000..cafbc034b4
--- /dev/null
+++ b/indra/test/sync.h
@@ -0,0 +1,85 @@
+/**
+ * @file   sync.h
+ * @author Nat Goodspeed
+ * @date   2019-03-13
+ * @brief  Synchronize coroutines within a test program so we can observe side
+ *         effects. Certain test programs test coroutine synchronization
+ *         mechanisms. Such tests usually want to interleave coroutine
+ *         executions in strictly stepwise fashion. This class supports that
+ *         paradigm.
+ * 
+ * $LicenseInfo:firstyear=2019&license=viewerlgpl$
+ * Copyright (c) 2019, Linden Research, Inc.
+ * $/LicenseInfo$
+ */
+
+#if ! defined(LL_SYNC_H)
+#define LL_SYNC_H
+
+#include "llcond.h"
+#include "lltut.h"
+#include "stringize.h"
+#include "llerror.h"
+#include "llcoros.h"
+
+/**
+ * Instantiate Sync in any test in which we need to suspend one coroutine
+ * until we're sure that another has had a chance to run. Simply calling
+ * llcoro::suspend() isn't necessarily enough; that provides a chance for the
+ * other to run, but doesn't guarantee that it has. If each coroutine is
+ * consistent about calling Sync::bump() every time it wakes from any
+ * suspension, Sync::yield() and yield_until() should at least ensure that
+ * somebody else has had a chance to run.
+ */
+class Sync
+{
+    LLScalarCond<int> mCond{0};
+    F32Milliseconds mTimeout;
+
+public:
+    Sync(F32Milliseconds timeout=F32Milliseconds(10.0f)):
+        mTimeout(timeout)
+    {}
+
+    /// Bump mCond by n steps -- ideally, do this every time a participating
+    /// coroutine wakes up from any suspension. The choice to bump() after
+    /// resumption rather than just before suspending is worth calling out:
+    /// this practice relies on the fact that condition_variable::notify_all()
+    /// merely marks a suspended coroutine ready to run, rather than
+    /// immediately resuming it. This way, though, even if a coroutine exits
+    /// before reaching its next suspend point, the other coroutine isn't
+    /// left waiting forever.
+    void bump(int n=1)
+    {
+        LL_DEBUGS() << llcoro::logname() << " bump(" << n << ") -> " << (mCond.get() + n) << LL_ENDL;
+        mCond.set_all(mCond.get() + n);
+    }
+
+    /// suspend until "somebody else" has bumped mCond by n steps
+    void yield(int n=1)
+    {
+        return yield_until(STRINGIZE("Sync::yield_for(" << n << ") timed out after "
+                                     << int(mTimeout.value()) << "ms"),
+                           mCond.get() + n);
+    }
+
+    /// suspend until "somebody else" has bumped mCond to a specific value
+    void yield_until(int until)
+    {
+        return yield_until(STRINGIZE("Sync::yield_until(" << until << ") timed out after "
+                                     << int(mTimeout.value()) << "ms"),
+                           until);
+    }
+
+private:
+    void yield_until(const std::string& desc, int until)
+    {
+        std::string name(llcoro::logname());
+        LL_DEBUGS() << name << " yield_until(" << until << ") suspending" << LL_ENDL;
+        tut::ensure(name + ' ' + desc, mCond.wait_for_equal(mTimeout, until));
+        // each time we wake up, bump mCond
+        bump();
+    }
+};
+
+#endif /* ! defined(LL_SYNC_H) */