SERVER-42370 Fix SharedStateImpl::addChild()

There's an unfortunate race between addChild and transitionToFinished,
where the caller of addChild can observe a kInit shared state, as can
transitionToFinished, and then the caller of addChild can add a child
that is never fulfilled by the caller of transitionToFinished.

It looks like:

addChild()              | transitionToFinished()
==============================================
loadState, see init     |
acquire mutex           |
loadState, see init     |
                        | transition to finished, observed init
attempt to cas, fail    |
                        | return, doing nothing
add to children         |
strand the child        |

The fix is to check to see if our compare_exchange failed, in which case
we need to fulfill ourselves

(cherry picked from commit e82c7b495f4c4cebf12f4f1a19c2922b1d1d2683)

1 files changed, 15 insertions, 9 deletions

diff --git a/src/mongo/util/future_impl.h b/src/mongo/util/future_impl.h
index 1deacc38733..1a37254a52b 100644
--- a/src/mongo/util/future_impl.h
+++ b/src/mongo/util/future_impl.h
@@ -479,23 +479,29 @@ struct SharedStateImpl final : SharedStateBase {
         }
 
         auto lk = stdx::unique_lock(mx);
+
         auto oldState = state.load(std::memory_order_acquire);
+        if (oldState == SSBState::kInit) {
+            // On the success path, our reads and writes to children are protected by the mutex
+            //
+            // On the failure path, we raced with transitionToFinished() and lost, so we need to
+            // synchronize with it via acquire before accessing the results since it wouldn't have
+            // taken the mutex.
+            state.compare_exchange_strong(oldState,
+                                          SSBState::kWaitingOrHaveChildren,
+                                          std::memory_order_relaxed,
+                                          std::memory_order_acquire);
+        }
         if (oldState == SSBState::kFinished) {
             lk.unlock();
             out->fillFromConst(*this);
             return out;
         }
-
-        if (oldState == SSBState::kInit) {
-            // memory ordering can be relaxed because that will be handled by the mutex.
-            state.compare_exchange_strong(
-                oldState, SSBState::kWaitingOrHaveChildren, std::memory_order_relaxed);
-        }
         dassert(oldState != SSBState::kHaveCallback);
 
-        // If oldState became kFinished after we checked (and therefore after we acquired the lock),
-        // the returned continuation will be completed by the promise side once it acquires the lock
-        // since we are adding ourself to the chain here.
+        // If oldState became kFinished after we checked (or successfully stored
+        // kWaitingOrHaveChildren), the returned continuation will be completed by the promise side
+        // once it acquires the lock since we are adding ourself to the chain here.
 
         children.emplace_front(out.get(), /*add ref*/ false);
         out->threadUnsafeIncRefCountTo(2);