Commit 67fcea20 authored by Alex Yarmula's avatar Alex Yarmula Committed by Facebook Github Bot

use previous Executor after delayed

Summary:
When `delayed` is called on the Future, the underlying `futures::sleep` call runs on a timer thread, and the resulting callback is called on the same thread. Therefore, in the following sequence:

  f.via(&someExecutor).within(one_ms).then([&]() { /* [1] */ })

The code in [1] is not running in someExecutor. This can cause confusion by users of the library who expect the initial `via` to be sticky.

This change returns to the prior `Executor` after `delayed` is finished.

Reviewed By: yfeldblum

Differential Revision: D3979179

fbshipit-source-id: 936ff5626e8ac377ffb15babf573349466984e3a
parent 008075ca
...@@ -972,10 +972,11 @@ Future<T> Future<T>::within(Duration dur, E e, Timekeeper* tk) { ...@@ -972,10 +972,11 @@ Future<T> Future<T>::within(Duration dur, E e, Timekeeper* tk) {
template <class T> template <class T>
Future<T> Future<T>::delayed(Duration dur, Timekeeper* tk) { Future<T> Future<T>::delayed(Duration dur, Timekeeper* tk) {
return collectAll(*this, futures::sleep(dur, tk)) return collectAll(*this, futures::sleep(dur, tk))
.then([](std::tuple<Try<T>, Try<Unit>> tup) { .then([](std::tuple<Try<T>, Try<Unit>> tup) {
Try<T>& t = std::get<0>(tup); Try<T>& t = std::get<0>(tup);
return makeFuture<T>(std::move(t)); return makeFuture<T>(std::move(t));
}); })
.via(getExecutor());
} }
namespace detail { namespace detail {
......
...@@ -168,20 +168,36 @@ TEST(Timekeeper, chainedInterruptTest) { ...@@ -168,20 +168,36 @@ TEST(Timekeeper, chainedInterruptTest) {
EXPECT_FALSE(test); EXPECT_FALSE(test);
} }
TEST(Timekeeper, executor) { namespace {
class ExecutorTester : public Executor { class ExecutorTester : public Executor {
public: public:
void add(Func f) override { void add(Func f) override {
count++; count++;
f(); f();
} }
std::atomic<int> count{0}; std::atomic<int> count{0};
}; };
}
auto f = makeFuture();
ExecutorTester tester; TEST(Timekeeper, executor) {
f.via(&tester).within(one_ms).then([&](){}).wait(); auto f = makeFuture();
EXPECT_EQ(2, tester.count); ExecutorTester tester;
f.via(&tester).within(one_ms).then([&]() {}).wait();
EXPECT_EQ(2, tester.count);
}
TEST(Timekeeper, executorSameAfterDelayed) {
ExecutorTester tester;
// make sure we're using the same Executor (tester) after delayed returns
// by comparing the executor count between invocations
makeFuture()
.via(&tester)
.delayed(one_ms)
.then([&]() { return tester.count.load(); })
.then([&](int countBefore) {
EXPECT_EQ(1, tester.count.load() - countBefore);
})
.wait();
} }
// TODO(5921764) // TODO(5921764)
......
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