Revert: (Wangle) within should raise TimedOut()

Summary: This reverts commit 956351018a495af89575526536af7e7c0bb285aa.

Reviewed By: @​labrams

Differential Revision: D2258219

diff --git a/folly/futures/Future-inl.h b/folly/futures/Future-inl.h
index d79fea347a191c900e5584f4f15c9e4f29d164a4..380b006925fcd97647cde9e0cac4565be729d2a6 100644 (file)
--- a/folly/futures/Future-inl.h
+++ b/folly/futures/Future-inl.h
@@ -855,41 +855,34 @@ template <class E>
 Future<T> Future<T>::within(Duration dur, E e, Timekeeper* tk) {
 
   struct Context {
-    Context(E ex, Future<Unit>&& f)
-        : exception(std::move(ex)), afterFuture(std::move(f)), promise() {}
+    Context(E ex) : exception(std::move(ex)), promise() {}
     E exception;
-    Future<Unit> afterFuture;
-    Future<Unit> thisFuture;
     Promise<T> promise;
     std::atomic<bool> token {false};
   };
+  auto ctx = std::make_shared<Context>(std::move(e));
 
   if (!tk) {
     tk = folly::detail::getTimekeeperSingleton();
   }
 
-  auto ctx = std::make_shared<Context>(std::move(e), tk->after(dur));
+  tk->after(dur)
+    .then([ctx](Try<Unit> const& t) {
+      if (ctx->token.exchange(true) == false) {
+        if (t.hasException()) {
+          ctx->promise.setException(std::move(t.exception()));
+        } else {
+          ctx->promise.setException(std::move(ctx->exception));
+        }
+      }
+    });
 
-  ctx->thisFuture = this->then([ctx](Try<T>&& t) mutable {
-    // "this" completed first, cancel "after"
-    ctx->afterFuture.raise(CancelTimer());
+  this->then([ctx](Try<T>&& t) {
     if (ctx->token.exchange(true) == false) {
       ctx->promise.setTry(std::move(t));
     }
   });
 
-  ctx->afterFuture.then([ctx](Try<Unit> const& t) mutable {
-    // "after" completed first, cancel "this"
-    ctx->thisFuture.raise(TimedOut());
-    if (ctx->token.exchange(true) == false) {
-      if (t.hasException()) {
-        ctx->promise.setException(std::move(t.exception()));
-      } else {
-        ctx->promise.setException(std::move(ctx->exception));
-      }
-    }
-  });
-
   return ctx->promise.getFuture().via(getExecutor());
 }
 

diff --git a/folly/futures/FutureException.h b/folly/futures/FutureException.h
index c5dbf1584820a7e8fb4db8095a3b7b12ff49c049..1e004dcf167a88b37a9a97762aa3b3370669244e 100644 (file)
--- a/folly/futures/FutureException.h
+++ b/folly/futures/FutureException.h
@@ -91,11 +91,6 @@ class TimedOut : public FutureException {
   TimedOut() : FutureException("Timed out") {}
 };
 
-class CancelTimer : public FutureException {
- public:
-  CancelTimer() : FutureException("Timer should be cancelled") {}
-};
-
 class PredicateDoesNotObtain : public FutureException {
  public:
   PredicateDoesNotObtain() : FutureException("Predicate does not obtain") {}

diff --git a/folly/futures/test/InterruptTest.cpp b/folly/futures/test/InterruptTest.cpp
index 7d34106fe8954e3246b97291fca7bf6bd8dd4605..99f8b696bb91d6c863583465bd72a12277ecd46d 100644 (file)
--- a/folly/futures/test/InterruptTest.cpp
+++ b/folly/futures/test/InterruptTest.cpp
@@ -18,7 +18,6 @@
 
 #include <folly/futures/Future.h>
 #include <folly/futures/Promise.h>
-#include <folly/Baton.h>
 
 using namespace folly;
 
@@ -73,44 +72,3 @@ TEST(Interrupt, secondInterruptNoop) {
   f.cancel();
   EXPECT_EQ(1, count);
 }
-
-TEST(Interrupt, withinTimedOut) {
-  Promise<int> p;
-  Baton<> done;
-  p.setInterruptHandler([&](const exception_wrapper& e) { done.post(); });
-  p.getFuture().within(std::chrono::milliseconds(1));
-  // Give it 100ms to time out and call the interrupt handler
-  auto t = std::chrono::steady_clock::now() + std::chrono::milliseconds(100);
-  EXPECT_TRUE(done.timed_wait(t));
-}
-
-class DummyTimeKeeper : public Timekeeper {
- public:
-  explicit DummyTimeKeeper() : interrupted() {}
-
-  Future<Unit> after(Duration) override {
-    promise.setInterruptHandler(
-      [this](const exception_wrapper& e) {
-        EXPECT_THROW(e.throwException(), CancelTimer);
-        interrupted.post();
-      }
-    );
-    return promise.getFuture();
-  }
-
-  Baton<> interrupted;
-
- private:
-  Promise<Unit> promise;
-};
-
-TEST(Interrupt, withinCancelTimer) {
-  DummyTimeKeeper tk;
-  Promise<int> p;
-  Baton<> done;
-  p.getFuture().within(std::chrono::milliseconds(10), TimedOut(), &tk);
-  p.setValue(1); // this should cancel the timer
-  // Give it 100ms to interrupt the timer Future
-  auto t = std::chrono::steady_clock::now() + std::chrono::milliseconds(100);
-  EXPECT_TRUE(tk.interrupted.timed_wait(t));
-}