Commit daf3e6fc authored by Yedidya Feldblum's avatar Yedidya Feldblum Committed by Facebook Github Bot

Fix race condition in collectN

Summary: [Folly] Fix race condition in collectN where the future may be completed with too few values or by reading the values while they are in-process of being written.

Reviewed By: djwatson

Differential Revision: D7900639

fbshipit-source-id: 3eafa821abda4f93232db3b8114de54c6faf798a
parent ddea466e
......@@ -1382,7 +1382,8 @@ collectN(InputIterator first, InputIterator last, size_t n) {
explicit CollectNContext(size_t numFutures) : v(numFutures) {}
V v;
std::atomic<size_t> completed = {0};
std::atomic<size_t> completed = {0}; // # input futures completed
std::atomic<size_t> stored = {0}; // # output values stored
Promise<Result> p;
void setPartialResult(size_t index, Try<T>&& t) {
......@@ -1412,13 +1413,19 @@ collectN(InputIterator first, InputIterator last, size_t n) {
// have n completed futures at which point we fulfil our Promise with the
// vector
mapSetCallback<T>(first, last, [ctx, n](size_t i, Try<T>&& t) {
auto c = ++ctx->completed;
if (c <= n) {
ctx->setPartialResult(i, std::move(t));
if (c == n) {
ctx->complete();
}
// relaxed because this guards control but does not guard data
auto const c = 1 + ctx->completed.fetch_add(1, std::memory_order_relaxed);
if (c > n) {
return;
}
ctx->setPartialResult(i, std::move(t));
// release because the stored values in all threads must be visible below
// acquire because no stored value is permitted to be fetched early
auto const s = 1 + ctx->stored.fetch_add(1, std::memory_order_acq_rel);
if (s < n) {
return;
}
ctx->complete();
});
}
......
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