Commit e6029638 authored by Vladislav Isenbaev's avatar Vladislav Isenbaev Committed by Praveen Kumar Ramakrishnan

Fix race condition in collect(..)

Summary: This is a temporary fix (until D2015320 is checked in) for race condition(s) in collect(..) method.

Test Plan:
Run unit tests
Run buffalo_aggregator canary

Reviewed By: jsedgwick@fb.com

Subscribers: folly-diffs@, jsedgwick, yfeldblum, chalfant

FB internal diff: D2037406

Tasks: 6894157

Signature: t1:2037406:1430435227:ed9612d016cdbd708e2deba02dc4fe0b59632f5a
parent 0b46bcd5
...@@ -620,13 +620,13 @@ struct CollectContext { ...@@ -620,13 +620,13 @@ struct CollectContext {
Optional<T> Optional<T>
>::type VecT; >::type VecT;
explicit CollectContext(int n) : count(0), threw(false) { explicit CollectContext(int n) : count(0), success_count(0), threw(false) {
results.resize(n); results.resize(n);
} }
Promise<std::vector<T>> p; Promise<std::vector<T>> p;
std::vector<VecT> results; std::vector<VecT> results;
std::atomic<size_t> count; std::atomic<size_t> count, success_count;
std::atomic_bool threw; std::atomic_bool threw;
typedef std::vector<T> result_type; typedef std::vector<T> result_type;
...@@ -647,10 +647,10 @@ struct CollectContext { ...@@ -647,10 +647,10 @@ struct CollectContext {
template <> template <>
struct CollectContext<void> { struct CollectContext<void> {
explicit CollectContext(int n) : count(0), threw(false) {} explicit CollectContext(int n) : count(0), success_count(0), threw(false) {}
Promise<void> p; Promise<void> p;
std::atomic<size_t> count; std::atomic<size_t> count, success_count;
std::atomic_bool threw; std::atomic_bool threw;
typedef void result_type; typedef void result_type;
...@@ -690,7 +690,6 @@ collect(InputIterator first, InputIterator last) { ...@@ -690,7 +690,6 @@ collect(InputIterator first, InputIterator last) {
assert(i < n); assert(i < n);
auto& f = *first; auto& f = *first;
f.setCallback_([ctx, i, n](Try<T> t) { f.setCallback_([ctx, i, n](Try<T> t) {
auto c = ++ctx->count;
if (t.hasException()) { if (t.hasException()) {
if (!ctx->threw.exchange(true)) { if (!ctx->threw.exchange(true)) {
...@@ -698,12 +697,12 @@ collect(InputIterator first, InputIterator last) { ...@@ -698,12 +697,12 @@ collect(InputIterator first, InputIterator last) {
} }
} else if (!ctx->threw) { } else if (!ctx->threw) {
ctx->addResult(i, t); ctx->addResult(i, t);
if (c == n) { if (++ctx->success_count == n) {
ctx->setValue(); ctx->setValue();
} }
} }
if (c == n) { if (++ctx->count == n) {
delete ctx; delete ctx;
} }
}); });
......
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