Commit 20dfb55a authored by Hans Fugal's avatar Hans Fugal Committed by Pavlo Kushnir

(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
parent 773fa87a
...@@ -457,16 +457,16 @@ whenAll(InputIterator first, InputIterator last) ...@@ -457,16 +457,16 @@ whenAll(InputIterator first, InputIterator last)
auto ctx = new detail::WhenAllContext<T>(); auto ctx = new detail::WhenAllContext<T>();
ctx->total = n; ctx->results.resize(n);
ctx->results.resize(ctx->total);
auto f_saved = ctx->p.getFuture(); auto f_saved = ctx->p.getFuture();
for (size_t i = 0; first != last; ++first, ++i) { for (size_t i = 0; first != last; ++first, ++i) {
assert(i < n);
auto& f = *first; auto& f = *first;
f.setCallback_([ctx, i](Try<T>&& t) { f.setCallback_([ctx, i, n](Try<T>&& t) {
ctx->results[i] = std::move(t); ctx->results[i] = std::move(t);
if (++ctx->count == ctx->total) { if (++ctx->count == n) {
ctx->p.setValue(std::move(ctx->results)); ctx->p.setValue(std::move(ctx->results));
delete ctx; delete ctx;
} }
......
...@@ -269,11 +269,10 @@ whenAllVariadicHelper(VariadicContext<Ts...> *ctx, THead&& head, Fs&&... tail) { ...@@ -269,11 +269,10 @@ whenAllVariadicHelper(VariadicContext<Ts...> *ctx, THead&& head, Fs&&... tail) {
template <typename T> template <typename T>
struct WhenAllContext { struct WhenAllContext {
explicit WhenAllContext() : count(0), total(0) {} WhenAllContext() : count(0) {}
Promise<std::vector<Try<T> > > p; Promise<std::vector<Try<T> > > p;
std::vector<Try<T> > results; std::vector<Try<T> > results;
std::atomic<size_t> count; std::atomic<size_t> count;
size_t total;
}; };
template <typename T> template <typename T>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment