Skip to content

Commit

Permalink
fix race in Future::waitWithSemaphore
Browse files Browse the repository at this point in the history
Summary:
There's a race condition in waitWithSemaphore, specifically because we're
returning a new future that's completed by the input future. As a result,
that future may not be completed by the time waitWithSemaphore returns,
although completion should be imminent.

Test Plan:
`var=0; while true ; do echo $((var++)); _bin/folly/wangle/wangle-test --gtest_filter='Future*' || break ; done`

Before change two runs yielded 303 and 371.

After change I killed the test at 11000.

Reviewed By: njormrod@fb.com

Subscribers: fugalh, njormrod

FB internal diff: D1561281

Tasks: 5180879
  • Loading branch information
mdordal authored and Dave Watson committed Sep 18, 2014
1 parent 546bd3f commit 079e4ff
Showing 1 changed file with 13 additions and 0 deletions.
13 changes: 13 additions & 0 deletions folly/wangle/Future-inl.h
Expand Up @@ -17,6 +17,7 @@
#pragma once

#include <chrono>
#include <thread>

#include <folly/wangle/detail/State.h>
#include <folly/Baton.h>
Expand Down Expand Up @@ -416,6 +417,12 @@ waitWithSemaphore(Future<T>&& f) {
return std::move(t.value());
});
baton.wait();
while (!done.isReady()) {
// There's a race here between the return here and the actual finishing of
// the future. f is completed, but the setup may not have finished on done
// after the baton has posted.
std::this_thread::yield();
}
return done;
}

Expand All @@ -427,6 +434,12 @@ inline Future<void> waitWithSemaphore<void>(Future<void>&& f) {
t.value();
});
baton.wait();
while (!done.isReady()) {
// There's a race here between the return here and the actual finishing of
// the future. f is completed, but the setup may not have finished on done
// after the baton has posted.
std::this_thread::yield();
}
return done;
}

Expand Down

0 comments on commit 079e4ff

Please sign in to comment.