Commit 9af220fa authored by Lee Howes's avatar Lee Howes Committed by Facebook Github Bot

Ensure that SemiFuture executor is correctly chained to deal with deferred work.

Summary: There was previously a bug in that defer used within a continuation would not correctly chain the deferred executor on the executor in the returned (and hence in the original) folly::Future like it would when .via was called on a SemiFuture. This fixes that situation by returning a semifuture from a continuation by correctly chaining it onto the outer future's executor.

Reviewed By: yfeldblum

Differential Revision: D7156241

fbshipit-source-id: ed6db3ad0de7a921b72459fcd2b1228114832853
parent e0d04ed7
......@@ -342,6 +342,21 @@ FutureBase<T>::thenImplementation(
return f;
}
// Pass through a simple future as it needs no deferral adaptation
template <class T>
Future<T> chainExecutor(Executor*, Future<T>&& f) {
return std::move(f);
}
// Correctly chain a SemiFuture for deferral
template <class T>
Future<T> chainExecutor(Executor* e, SemiFuture<T>&& f) {
if (!e) {
e = &folly::InlineExecutor::instance();
}
return std::move(f).via(e);
}
// Variant: returns a Future
// e.g. f.then([](T&& t){ return makeFuture<T>(t); });
template <class T>
......@@ -359,22 +374,28 @@ FutureBase<T>::thenImplementation(
// grab the Future now before we lose our handle on the Promise
auto sf = p.getSemiFuture();
sf.core_->setExecutor(this->getExecutor());
auto* e = this->getExecutor();
sf.core_->setExecutor(e);
auto f = Future<B>(sf.core_);
sf.core_ = nullptr;
this->setCallback_(
[state = futures::detail::makeCoreCallbackState(
std::move(p), std::forward<F>(func))](Try<T>&& t) mutable {
this->setCallback_([state = futures::detail::makeCoreCallbackState(
std::move(p), std::forward<F>(func))](
Try<T>&& t) mutable {
if (!isTry && t.hasException()) {
state.setException(std::move(t.exception()));
} else {
// Ensure that if function returned a SemiFuture we correctly chain
// potential deferral.
auto tf2 = state.tryInvoke(t.template get<isTry, Args>()...);
if (tf2.hasException()) {
state.setException(std::move(tf2.exception()));
} else {
tf2->setCallback_([p = state.stealPromise()](Try<B> && b) mutable {
p.setTry(std::move(b));
auto statePromise = state.stealPromise();
auto tf3 =
chainExecutor(statePromise.core_->getExecutor(), *std::move(tf2));
tf3.setCallback_([p2 = std::move(statePromise)](Try<B>&& b) mutable {
p2.setTry(std::move(b));
});
}
}
......
......@@ -517,3 +517,26 @@ TEST(SemiFuture, MakeSemiFutureFromFutureWithTry) {
auto tryResult = std::move(sf).get();
ASSERT_EQ(tryResult.value(), "Try");
}
TEST(SemiFuture, DeferWithinContinuation) {
std::atomic<int> innerResult{0};
std::atomic<int> result{0};
EventBase e2;
Promise<int> p;
Promise<int> p2;
auto f = p.getSemiFuture().via(&e2);
auto resultF = std::move(f).then([&, p3 = std::move(p2)](int outer) mutable {
result = outer;
return makeSemiFuture<int>(std::move(outer))
.defer([&, p4 = std::move(p3)](int inner) mutable {
innerResult = inner;
p4.setValue(inner);
return inner;
});
});
p.setValue(7);
auto r = resultF.getVia(&e2);
ASSERT_EQ(r, 7);
ASSERT_EQ(innerResult, 7);
ASSERT_EQ(result, 7);
}
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