Commit d1caa885 authored by Andrii Grynenko's avatar Andrii Grynenko Committed by Facebook Github Bot

Remove non-destructive timed wait() from the public API

Summary: This change will allow simplifying DeferredExecutor and implementing both wait() and timed wait() using custom executors.

Reviewed By: LeeHowes

Differential Revision: D9890644

fbshipit-source-id: ad8472213e7022ccc23b4a9c48f8b9c4b9c70dfa
parent 959e3cff
...@@ -2119,8 +2119,10 @@ SemiFuture<T>& SemiFuture<T>::wait(Duration dur) & { ...@@ -2119,8 +2119,10 @@ SemiFuture<T>& SemiFuture<T>::wait(Duration dur) & {
} }
template <class T> template <class T>
SemiFuture<T>&& SemiFuture<T>::wait(Duration dur) && { bool SemiFuture<T>::wait(Duration dur) && {
return std::move(wait(dur)); auto future = std::move(*this);
future.wait(dur);
return future.isReady();
} }
template <class T> template <class T>
......
...@@ -619,9 +619,7 @@ class SemiFuture : private futures::detail::FutureBase<T> { ...@@ -619,9 +619,7 @@ class SemiFuture : private futures::detail::FutureBase<T> {
/// ///
/// Postconditions: /// Postconditions:
/// ///
/// - if returns (no exception), moves-out the Try; treat `*this` as if /// - `valid() == false`
/// `!valid()`.
/// - on FutureTimeout exception, `valid()` remains true.
Try<T> getTry(Duration dur) &&; Try<T> getTry(Duration dur) &&;
/// Blocks the caller's thread until this Future `isReady()`, i.e., until the /// Blocks the caller's thread until this Future `isReady()`, i.e., until the
...@@ -654,6 +652,7 @@ class SemiFuture : private futures::detail::FutureBase<T> { ...@@ -654,6 +652,7 @@ class SemiFuture : private futures::detail::FutureBase<T> {
SemiFuture<T>&& wait() &&; SemiFuture<T>&& wait() &&;
/// Blocks until the future is fulfilled, or `dur` elapses. /// Blocks until the future is fulfilled, or `dur` elapses.
/// Returns true if the future was fulfilled.
/// ///
/// Preconditions: /// Preconditions:
/// ///
...@@ -661,24 +660,8 @@ class SemiFuture : private futures::detail::FutureBase<T> { ...@@ -661,24 +660,8 @@ class SemiFuture : private futures::detail::FutureBase<T> {
/// ///
/// Postconditions: /// Postconditions:
/// ///
/// - `valid() == true` /// - `valid() == false`
/// - `&RESULT == this` bool wait(Duration dur) &&;
/// - `isReady()` will be indeterminate - may or may not be true
SemiFuture<T>& wait(Duration dur) &;
/// Blocks until the future is fulfilled, or `dur` elapses.
///
/// Preconditions:
///
/// - `valid() == true` (else throws FutureInvalid)
///
/// Postconditions:
///
/// - `valid() == true` (but the calling code can trivially move-out `*this`
/// by assigning or constructing the result into a distinct object).
/// - `&RESULT == this`
/// - `isReady()` will be indeterminate - may or may not be true
SemiFuture<T>&& wait(Duration dur) &&;
/// Returns a Future which will call back on the other side of executor. /// Returns a Future which will call back on the other side of executor.
Future<T> via(Executor* executor, int8_t priority = Executor::MID_PRI) &&; Future<T> via(Executor* executor, int8_t priority = Executor::MID_PRI) &&;
...@@ -915,6 +898,19 @@ class SemiFuture : private futures::detail::FutureBase<T> { ...@@ -915,6 +898,19 @@ class SemiFuture : private futures::detail::FutureBase<T> {
// Throws FutureInvalid if !this->core_ // Throws FutureInvalid if !this->core_
DeferredExecutor* stealDeferredExecutor() const; DeferredExecutor* stealDeferredExecutor() const;
/// Blocks until the future is fulfilled, or `dur` elapses.
///
/// Preconditions:
///
/// - `valid() == true` (else throws FutureInvalid)
///
/// Postconditions:
///
/// - `valid() == true`
/// - `&RESULT == this`
/// - `isReady()` will be indeterminate - may or may not be true
SemiFuture<T>& wait(Duration dur) &;
static void releaseDeferredExecutor(Core* core); static void releaseDeferredExecutor(Core* core);
}; };
......
...@@ -494,26 +494,6 @@ TEST(SemiFuture, SimpleTimedGet) { ...@@ -494,26 +494,6 @@ TEST(SemiFuture, SimpleTimedGet) {
std::move(sf).get(std::chrono::milliseconds(100)), FutureTimeout); std::move(sf).get(std::chrono::milliseconds(100)), FutureTimeout);
} }
TEST(SemiFuture, SimpleTimedWait) {
Promise<folly::Unit> p;
auto sf = p.getSemiFuture();
sf.wait(std::chrono::milliseconds(100));
EXPECT_FALSE(sf.isReady());
p.setValue();
EXPECT_TRUE(sf.isReady());
}
TEST(SemiFuture, SimpleTimedMultipleWait) {
Promise<folly::Unit> p;
auto sf = p.getSemiFuture();
sf.wait(std::chrono::milliseconds(100));
sf.wait(std::chrono::milliseconds(100));
EXPECT_FALSE(sf.isReady());
p.setValue();
sf.wait(std::chrono::milliseconds(100));
EXPECT_TRUE(sf.isReady());
}
TEST(SemiFuture, SimpleTimedGetViaFromSemiFuture) { TEST(SemiFuture, SimpleTimedGetViaFromSemiFuture) {
TimedDrivableExecutor e2; TimedDrivableExecutor e2;
Promise<folly::Unit> p; Promise<folly::Unit> p;
...@@ -631,29 +611,6 @@ TEST(SemiFuture, DeferWithGetTimedGet) { ...@@ -631,29 +611,6 @@ TEST(SemiFuture, DeferWithGetTimedGet) {
ASSERT_EQ(innerResult, 0); ASSERT_EQ(innerResult, 0);
} }
TEST(SemiFuture, DeferWithGetTimedWait) {
Promise<folly::Unit> p;
auto f = p.getSemiFuture().toUnsafeFuture();
auto sf = std::move(f).semi().defer([&](auto&&) { return 17; });
ASSERT_FALSE(sf.isReady());
sf.wait(std::chrono::milliseconds(100));
ASSERT_FALSE(sf.isReady());
p.setValue();
ASSERT_EQ(std::move(sf).get(), 17);
}
TEST(SemiFuture, DeferWithGetMultipleTimedWait) {
Promise<folly::Unit> p;
auto f = p.getSemiFuture().toUnsafeFuture();
auto sf = std::move(f).semi().defer([&](auto&&) { return 17; });
sf.wait(std::chrono::milliseconds(100));
sf.wait(std::chrono::milliseconds(100));
ASSERT_FALSE(sf.isReady());
p.setValue();
sf.wait(std::chrono::milliseconds(100));
ASSERT_EQ(std::move(sf).get(), 17);
}
TEST(SemiFuture, DeferWithVia) { TEST(SemiFuture, DeferWithVia) {
std::atomic<int> innerResult{0}; std::atomic<int> innerResult{0};
EventBase e2; EventBase e2;
...@@ -1029,11 +986,12 @@ TEST(SemiFuture, collectAllSemiFutureDeferredWork) { ...@@ -1029,11 +986,12 @@ TEST(SemiFuture, collectAllSemiFutureDeferredWork) {
promise1.setValue(1); promise1.setValue(1);
promise2.setValue(2); promise2.setValue(2);
EXPECT_TRUE(future.wait(std::chrono::milliseconds{100}).isReady()); auto result = std::move(future).getTry(std::chrono::milliseconds{100});
auto value = std::move(future).get(); EXPECT_TRUE(result.hasValue());
EXPECT_EQ(2, *std::get<0>(value));
EXPECT_EQ(4, *std::get<1>(value)); EXPECT_EQ(2, *std::get<0>(*result));
EXPECT_EQ(4, *std::get<1>(*result));
} }
{ {
......
...@@ -251,7 +251,7 @@ TEST(Wait, waitWithDuration) { ...@@ -251,7 +251,7 @@ TEST(Wait, waitWithDuration) {
} }
{ {
// `SemiFuture::wait(Duration) &` when promise is fulfilled during the wait // `SemiFuture::get(Duration) &&` when promise is fulfilled during the get
Promise<int> p; Promise<int> p;
auto f = p.getSemiFuture(); auto f = p.getSemiFuture();
...@@ -265,41 +265,9 @@ TEST(Wait, waitWithDuration) { ...@@ -265,41 +265,9 @@ TEST(Wait, waitWithDuration) {
}); });
b.wait(); b.wait();
f.wait(std::chrono::seconds(10)); EXPECT_EQ(std::move(f).get(std::chrono::seconds(10)), 42);
EXPECT_TRUE(f.valid());
EXPECT_TRUE(f.isReady());
EXPECT_EQ(f.value(), 42);
t.join(); t.join();
EXPECT_TRUE(f.isReady());
EXPECT_EQ(f.value(), 42);
}
{
// `SemiFuture::wait(Duration) &&` when promise is fulfilled during the wait
Promise<int> p;
auto f1 = p.getSemiFuture();
EXPECT_FALSE(f1.isReady());
folly::Baton<> b;
auto t = std::thread([&] {
b.post();
/* sleep override */ std::this_thread::sleep_for(milliseconds(100));
p.setValue(42);
});
b.wait();
auto f2 = std::move(f1).wait(std::chrono::seconds(10));
EXPECT_FALSE(f1.valid());
EXPECT_TRUE(f2.valid());
EXPECT_TRUE(f2.isReady());
EXPECT_EQ(f2.value(), 42);
t.join();
EXPECT_TRUE(f2.valid());
EXPECT_TRUE(f2.isReady());
EXPECT_EQ(f2.value(), 42);
} }
} }
...@@ -455,63 +423,4 @@ TEST(Wait, WaitPlusThen) { ...@@ -455,63 +423,4 @@ TEST(Wait, WaitPlusThen) {
EXPECT_EQ(continuation, 42); EXPECT_EQ(continuation, 42);
t.join(); t.join();
} }
{
// Sub-case: SemiFuture fulfilled before `wait(dur)` is called.
// Expect call to `.then()` to succeed & continuation to run immediately.
Promise<int> p;
auto f = p.getSemiFuture();
p.setValue(42);
EXPECT_TRUE(f.isReady());
EXPECT_EQ(f.value(), 42);
f.wait(std::chrono::seconds(10));
auto continuation = 0;
InlineExecutor e;
auto f2 = std::move(f).via(&e);
EXPECT_NO_THROW(
std::move(f2).thenValue([&](auto&& v) { continuation = v; }));
EXPECT_EQ(continuation, 42);
}
{
// Sub-case: SemiFuture fulfilled after `wait(dur)` actually starts waiting.
// Expect call to `.then()` to succeed & continuation to when result ready.
Promise<int> p;
auto f = p.getSemiFuture();
folly::Baton<> b;
auto t = std::thread([&] {
b.post();
/* sleep override */ std::this_thread::sleep_for(milliseconds(100));
p.setValue(42);
});
b.wait();
EXPECT_FALSE(f.isReady()); // deterministically passes in practice
f.wait(std::chrono::seconds(10));
EXPECT_TRUE(f.isReady()); // deterministically passes in practice
auto continuation = 0;
InlineExecutor e;
auto f2 = std::move(f).via(&e);
EXPECT_NO_THROW(
std::move(f2).thenValue([&](auto&& v) { continuation = v; }));
EXPECT_EQ(continuation, 42);
t.join();
}
{
// Sub-case: SemiFuture fulfilled after `wait(dur)` times out.
// Expect call to `.then()` to succeed; continuation runs when fulfilled.
Promise<int> p;
auto f = p.getSemiFuture();
f.wait(milliseconds(1));
auto continuation = 0;
InlineExecutor e;
auto f2 = std::move(f).via(&e);
EXPECT_NO_THROW(
std::move(f2).thenValue([&](auto&& v) { continuation = v; }));
EXPECT_EQ(continuation, 0);
p.setValue(42);
EXPECT_EQ(continuation, 42);
}
} }
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