Commit 12f18243 authored by Erik Hortsch's avatar Erik Hortsch Committed by Facebook Github Bot

Standardize fiber baton behavior across threads and fibers

Summary:
The behavior of fiber Baton is currently inconsistent around posting of a timed out baton, and subsequent calls to try_wait().  As stated in the post, only when the waiter is a thread will a post overwrite a timeout internally, and this is observable externally because try_wait returns true only if a baton is in the POSTED state, but not the timed out state.  Example code:

```
folly::fibers::Baton b;

b.try_wait_for(std::chrono::milliseconds(1));
b.try_wait(); // returns false

b.post();
b.try_wait(); // Returns true on threads, false on fibers
```

Other options we could consider for the fix:
 * Update postThread to leave baton in a timeout state, and have waitThread move the baton into the timeout state on a failed wait.
 * Update try_wait() to return true if the state is either POSTED or TIMEOUT.

Reviewed By: andriigrynenko

Differential Revision: D14324532

fbshipit-source-id: 51b993a6153cba0fc3cf5701f765fff8ad1c8e3c
parent 49a54a71
...@@ -160,12 +160,12 @@ void Baton::postHelper(intptr_t new_value) { ...@@ -160,12 +160,12 @@ void Baton::postHelper(intptr_t new_value) {
return postThread(); return postThread();
} }
if (waiter == POSTED || waiter == TIMEOUT) { if (waiter == POSTED) {
return; return;
} }
} while (!waiter_.compare_exchange_weak(waiter, new_value)); } while (!waiter_.compare_exchange_weak(waiter, new_value));
if (waiter != NO_WAITER) { if (waiter != NO_WAITER && waiter != TIMEOUT) {
reinterpret_cast<Waiter*>(waiter)->post(); reinterpret_cast<Waiter*>(waiter)->post();
} }
} }
......
...@@ -217,6 +217,35 @@ TEST(FiberManager, batonTryWait) { ...@@ -217,6 +217,35 @@ TEST(FiberManager, batonTryWait) {
manager.loopUntilNoReady(); manager.loopUntilNoReady();
} }
TEST(FiberManager, batonTryWaitConsistent) {
folly::EventBase evb;
struct ExpectedException {};
auto& fm = getFiberManager(evb);
// Check if try_wait and post have a consistent behavior on a timed out
// baton
Baton b;
b.try_wait_for(std::chrono::milliseconds(1));
b.try_wait(); // returns false
b.post();
EXPECT_TRUE(b.try_wait());
b.reset();
fm.addTask([&]() {
b.try_wait_for(std::chrono::milliseconds(1));
b.try_wait(); // returns false
b.post();
EXPECT_TRUE(b.try_wait());
});
while (fm.hasTasks()) {
evb.loopOnce();
}
}
TEST(FiberManager, genericBatonFiberWait) { TEST(FiberManager, genericBatonFiberWait) {
FiberManager manager(std::make_unique<SimpleLoopController>()); FiberManager manager(std::make_unique<SimpleLoopController>());
......
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