DRTVWR-476: Fix overflow case in llcoro::postAndSuspend().

Actually the fix is in postAndSuspendSetup(), which affects postAndSuspend(),
postAndSuspendWithTimeout(), suspendUntilEventOnWithTimeout() and
suspendUntilEventOn().

By "overflow case" we mean the special circumstance in which:

* the LLEventPump in question is an LLEventMailDrop, meaning its listeners
  eventually expect to see every post()ed value
* one of the listeners is supposed to consume those values (has called
  LLCoros::set_consuming(true))
* post() is called more than once before that listener is resumed.

The magic of postAndSuspend() (et al.) is a temporary LLCoros::Promise. The
waiting coroutine calls get() on the corresponding Future, causing it to
suspend (as promised) until the Promise is fulfilled.

With the Boost.Fiber implementation of coroutines, fulfilling the Promise
doesn't immediately resume the suspended coroutine -- it merely marks it ready
to resume, next time the scheduler gets control.

A second post() call before the suspended coroutine is resumed results in a
second call to Promise::set_value(). But Promise is a one-shot entity. This
results in a promise_already_satisfied exception. Because a second post() call
during that time window is perfectly reasonable, we catch that exception and
carry on.

The tricky part is: when that exception is thrown, what should the listener
return? Previously we were returning the listener's current consuming setting,
just as when the set_value() call succeeds.

But when the LLEventPump is an LLEventMailDrop, and the listener's consuming
flag is true, that told LLEventMailDrop::post() that the value got through,
and that it needn't bother to save it in its history queue. The net effect was
to discard the value.

Instead, return the listener's consuming flag only when Promise::set_value()
succeeds. When it throws promise_already_satisfied, unconditionally return
false. That directs LLEventMailDrop::post() to enqueue the undelivered value
so that the *next* suspendUntilEventOn() call can pick it up.

1 files changed, 21 insertions, 14 deletions

diff --git a/indra/llcommon/lleventcoro.cpp b/indra/llcommon/lleventcoro.cpp
index b1fb8ffd04..18a3595c24 100644
--- a/indra/llcommon/lleventcoro.cpp
+++ b/indra/llcommon/lleventcoro.cpp
@@ -137,20 +137,27 @@ LLBoundListener postAndSuspendSetup(const std::string& callerName,
     // make a callback that will assign a value to the future, and listen on
     // the specified LLEventPump with that callback
     LLBoundListener connection(
-        replyPump.getPump().listen(listenerName,
-                                   [&promise, consuming, listenerName](const LLSD& result)
-                                   {
-                                       try
-                                       {
-                                           promise.set_value(result);
-                                       }
-                                       catch(boost::fibers::promise_already_satisfied & ex)
-                                       {
-                                           LL_WARNS("lleventcoro") << "promise already satisfied in '"
-                                               << listenerName << "' "  << ex.what() << LL_ENDL;
-                                       }
-                                       return consuming;
-                                   }));
+        replyPump.getPump().listen(
+            listenerName,
+            [&promise, consuming, listenerName](const LLSD& result)
+            {
+                try
+                {
+                    promise.set_value(result);
+                    // We did manage to propagate the result value to the
+                    // (real) listener. If we're supposed to indicate that
+                    // we've consumed it, do so.
+                    return consuming;
+                }
+                catch(boost::fibers::promise_already_satisfied & ex)
+                {
+                    LL_WARNS("lleventcoro") << "promise already satisfied in '"
+                        << listenerName << "' "  << ex.what() << LL_ENDL;
+                    // We could not propagate the result value to the
+                    // listener.
+                    return false;
+                }
+            }));
     // skip the "post" part if requestPump is default-constructed
     if (requestPump)
     {