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

Fix convertFuture

Summary: It's unsafe to use Future after setCallback was called (its executor may be moved out), so we have to copy its executor before that.

Reviewed By: yfeldblum

Differential Revision: D15113490

fbshipit-source-id: 2fa9c323ddfe1919b84bbfe05b6c5643d0c65703
parent 2d41ab6d
...@@ -2114,27 +2114,27 @@ void waitImpl(FutureType& f) { ...@@ -2114,27 +2114,27 @@ void waitImpl(FutureType& f) {
} }
Promise<T> promise; Promise<T> promise;
auto ret = promise.getSemiFuture(); auto ret = convertFuture(promise.getSemiFuture(), f);
auto baton = std::make_shared<FutureBatonType>(); auto baton = std::make_shared<FutureBatonType>();
f.setCallback_([baton, promise = std::move(promise)](Try<T>&& t) mutable { f.setCallback_([baton, promise = std::move(promise)](Try<T>&& t) mutable {
promise.setTry(std::move(t)); promise.setTry(std::move(t));
baton->post(); baton->post();
}); });
convertFuture(std::move(ret), f); f = std::move(ret);
baton->wait(); baton->wait();
assert(f.isReady()); assert(f.isReady());
} }
template <class T> template <class T>
void convertFuture(SemiFuture<T>&& sf, Future<T>& f) { Future<T> convertFuture(SemiFuture<T>&& sf, const Future<T>& f) {
// Carry executor from f, inserting an inline executor if it did not have one // Carry executor from f, inserting an inline executor if it did not have one
auto* exe = f.getExecutor(); auto* exe = f.getExecutor();
f = std::move(sf).via(exe ? exe : &InlineExecutor::instance()); return std::move(sf).via(exe ? exe : &InlineExecutor::instance());
} }
template <class T> template <class T>
void convertFuture(SemiFuture<T>&& sf, SemiFuture<T>& f) { SemiFuture<T> convertFuture(SemiFuture<T>&& sf, const SemiFuture<T>&) {
f = std::move(sf); return std::move(sf);
} }
template <class FutureType, typename T = typename FutureType::value_type> template <class FutureType, typename T = typename FutureType::value_type>
...@@ -2148,13 +2148,13 @@ void waitImpl(FutureType& f, Duration dur) { ...@@ -2148,13 +2148,13 @@ void waitImpl(FutureType& f, Duration dur) {
} }
Promise<T> promise; Promise<T> promise;
auto ret = promise.getSemiFuture(); auto ret = convertFuture(promise.getSemiFuture(), f);
auto baton = std::make_shared<FutureBatonType>(); auto baton = std::make_shared<FutureBatonType>();
f.setCallback_([baton, promise = std::move(promise)](Try<T>&& t) mutable { f.setCallback_([baton, promise = std::move(promise)](Try<T>&& t) mutable {
promise.setTry(std::move(t)); promise.setTry(std::move(t));
baton->post(); baton->post();
}); });
convertFuture(std::move(ret), f); f = std::move(ret);
if (baton->try_wait_for(dur)) { if (baton->try_wait_for(dur)) {
assert(f.isReady()); assert(f.isReady());
} }
......
...@@ -430,7 +430,7 @@ class FutureBase { ...@@ -430,7 +430,7 @@ class FutureBase {
SemiFuture<T> withinImplementation(Duration dur, E e, Timekeeper* tk) &&; SemiFuture<T> withinImplementation(Duration dur, E e, Timekeeper* tk) &&;
}; };
template <class T> template <class T>
void convertFuture(SemiFuture<T>&& sf, Future<T>& f); Future<T> convertFuture(SemiFuture<T>&& sf, const Future<T>& f);
class DeferredExecutor; class DeferredExecutor;
...@@ -1914,9 +1914,9 @@ class Future : private futures::detail::FutureBase<T> { ...@@ -1914,9 +1914,9 @@ class Future : private futures::detail::FutureBase<T> {
friend Future<Unit> whileDo(P&& predicate, F&& thunk); friend Future<Unit> whileDo(P&& predicate, F&& thunk);
template <class FT> template <class FT>
friend void futures::detail::convertFuture( friend Future<FT> futures::detail::convertFuture(
SemiFuture<FT>&& sf, SemiFuture<FT>&& sf,
Future<FT>& f); const Future<FT>& f);
}; };
/// A Timekeeper handles the details of keeping time and fulfilling delay /// A Timekeeper handles the details of keeping time and fulfilling delay
......
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