From: Hans Fugal <fugalh@fb.com>
Date: Wed, 5 Nov 2014 19:21:49 +0000 (-0800)
Subject: (wangle) fix a race in whenAll
X-Git-Tag: v0.22.0~196
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=20dfb55adcc0c87f596047a6b3614cc36583808b;p=folly.git

(wangle) fix a race in whenAll

Summary:
Race in `++ctx->count == ctx->total`. This ordering, while not very obvious or likely, is possible:

T1            T2
--            --
++ctx->count
++ctx->count
ctx->total
==
setValue
delete ctx
ctx->total
==
setValue
delete ctx

Test Plan:
That's the idea, anyway. I need some sleep, and it takes 20 minutes to build and test. I had a more convoluted fix (using `shared_ptr`) and it did seem to fix the error we were seeing, but I was seeing another error.

Reviewed By: darshan@fb.com

Subscribers: trunkagent, net-systems@, fugalh, exa, njormrod, folly-diffs@

FB internal diff: D1660663

Tasks: 5506504

Signature: t1:1660663:1415207632:49dc224363cec27736fc71fb211fa846be9e170f

Blame Revision: D1636487
---

diff --git a/folly/wangle/Future-inl.h b/folly/wangle/Future-inl.h
index 6dd2125c..a1b8d0c5 100644
--- a/folly/wangle/Future-inl.h
+++ b/folly/wangle/Future-inl.h
@@ -457,16 +457,16 @@ whenAll(InputIterator first, InputIterator last)
 
   auto ctx = new detail::WhenAllContext<T>();
 
-  ctx->total = n;
-  ctx->results.resize(ctx->total);
+  ctx->results.resize(n);
 
   auto f_saved = ctx->p.getFuture();
 
   for (size_t i = 0; first != last; ++first, ++i) {
+     assert(i < n);
      auto& f = *first;
-     f.setCallback_([ctx, i](Try<T>&& t) {
+     f.setCallback_([ctx, i, n](Try<T>&& t) {
          ctx->results[i] = std::move(t);
-         if (++ctx->count == ctx->total) {
+         if (++ctx->count == n) {
            ctx->p.setValue(std::move(ctx->results));
            delete ctx;
          }
diff --git a/folly/wangle/detail/Core.h b/folly/wangle/detail/Core.h
index f83e4f71..bae0c870 100644
--- a/folly/wangle/detail/Core.h
+++ b/folly/wangle/detail/Core.h
@@ -269,11 +269,10 @@ whenAllVariadicHelper(VariadicContext<Ts...> *ctx, THead&& head, Fs&&... tail) {
 
 template <typename T>
 struct WhenAllContext {
-  explicit WhenAllContext() : count(0), total(0) {}
+  WhenAllContext() : count(0) {}
   Promise<std::vector<Try<T> > > p;
   std::vector<Try<T> > results;
   std::atomic<size_t> count;
-  size_t total;
 };
 
 template <typename T>