Remove deactivation from Future

Summary: [Folly] Remove deactivation, a rarely-used facility, from `Future`.

Reviewed By: marshallcline

Differential Revision: D7788059

fbshipit-source-id: 38db16fc3c2694b3cd772844fdafe71f6033a97c

folly/futures/Future.h

@@ -132,10 +132,6 @@ class FutureBase {
   template <class F>
   void setCallback_(F&& func);
 
-  bool isActive() const {
-    return getCore().isActive();
-  }
-
   template <class E>
   void raise(E&& exception) {
     raise(make_exception_wrapper<typename std::remove_reference<E>::type>(
@@ -270,7 +266,6 @@ class SemiFuture : private futures::detail::FutureBase<T> {
   using Base::cancel;
   using Base::hasException;
   using Base::hasValue;
-  using Base::isActive;
   using Base::isReady;
   using Base::poll;
   using Base::raise;
@@ -312,26 +307,11 @@ class SemiFuture : private futures::detail::FutureBase<T> {
   /// Overload of wait(Duration) for rvalue Futures
   SemiFuture<T>&& wait(Duration) &&;
 
-  /// Returns an inactive Future which will call back on the other side of
-  /// executor (when it is activated).
-  ///
-  /// NB remember that Futures activate when they destruct. This is good,
-  /// it means that this will work:
-  ///
-  ///   f.via(e).then(a).then(b);
+  /// Returns a Future which will call back on the other side of executor.
   ///
-  /// a and b will execute in the same context (the far side of e), because
-  /// the Future (temporary variable) created by via(e) does not call back
-  /// until it destructs, which is after then(a) and then(b) have been wired
-  /// up.
-  ///
-  /// But this is still racy:
-  ///
-  ///   f = f.via(e).then(a);
-  ///   f.then(b);
-  // The ref-qualifier allows for `this` to be moved out so we
-  // don't get access-after-free situations in chaining.
-  // https://akrzemi1.wordpress.com/2014/06/02/ref-qualifiers/
+  /// The ref-qualifier allows for `this` to be moved out so we
+  /// don't get access-after-free situations in chaining.
+  /// https://akrzemi1.wordpress.com/2014/06/02/ref-qualifiers/
   Future<T> via(Executor* executor, int8_t priority = Executor::MID_PRI) &&;
 
   /**
@@ -537,7 +517,6 @@ class Future : private futures::detail::FutureBase<T> {
   using Base::cancel;
   using Base::hasException;
   using Base::hasValue;
-  using Base::isActive;
   using Base::isReady;
   using Base::poll;
   using Base::raise;
@@ -578,26 +557,11 @@ class Future : private futures::detail::FutureBase<T> {
       enable_if<isFuture<F>::value, Future<typename isFuture<T>::Inner>>::type
       unwrap();
 
-  /// Returns an inactive Future which will call back on the other side of
-  /// executor (when it is activated).
-  ///
-  /// NB remember that Futures activate when they destruct. This is good,
-  /// it means that this will work:
-  ///
-  ///   f.via(e).then(a).then(b);
+  /// Returns a Future which will call back on the other side of executor.
   ///
-  /// a and b will execute in the same context (the far side of e), because
-  /// the Future (temporary variable) created by via(e) does not call back
-  /// until it destructs, which is after then(a) and then(b) have been wired
-  /// up.
-  ///
-  /// But this is still racy:
-  ///
-  ///   f = f.via(e).then(a);
-  ///   f.then(b);
-  // The ref-qualifier allows for `this` to be moved out so we
-  // don't get access-after-free situations in chaining.
-  // https://akrzemi1.wordpress.com/2014/06/02/ref-qualifiers/
+  /// The ref-qualifier allows for `this` to be moved out so we
+  /// don't get access-after-free situations in chaining.
+  /// https://akrzemi1.wordpress.com/2014/06/02/ref-qualifiers/
   Future<T> via(Executor* executor, int8_t priority = Executor::MID_PRI) &&;
 
   /// This variant creates a new future, where the ref-qualifier && version
@@ -748,28 +712,6 @@ class Future : private futures::detail::FutureBase<T> {
   template <class F>
   Future<T> onTimeout(Duration, F&& func, Timekeeper* = nullptr);
 
-  /// A Future's callback is executed when all three of these conditions have
-  /// become true: it has a value (set by the Promise), it has a callback (set
-  /// by then), and it is active (active by default).
-  ///
-  /// Inactive Futures will activate upon destruction.
-  [[deprecated("do not use")]] Future<T>& activate() & {
-    this->getCore().activate();
-    return *this;
-  }
-  [[deprecated("do not use")]] Future<T>& deactivate() & {
-    this->getCore().deactivate();
-    return *this;
-  }
-  [[deprecated("do not use")]] Future<T> activate() && {
-    this->getCore().activate();
-    return std::move(*this);
-  }
-  [[deprecated("do not use")]] Future<T> deactivate() && {
-    this->getCore().deactivate();
-    return std::move(*this);
-  }
-
   /// Throw TimedOut if this Future does not complete within the given
   /// duration from now. The optional Timeekeeper is as with futures::sleep().
   Future<T> within(Duration, Timekeeper* = nullptr);

folly/futures/detail/Core.h

@@ -49,8 +49,8 @@ namespace detail {
 
 This state machine is fairly self-explanatory. The most important bit is
 that the callback is only executed on the transition from Armed to Done,
-and that transition can happen immediately after transitioning from Only*
-to Armed, if it is active (the usual case).
+and that transition happens immediately after transitioning from Only*
+to Armed.
 */
 enum class State : uint8_t {
   Start,
@@ -215,7 +215,6 @@ class Core final {
 
   /// Called by a destructing Future (in the Future thread, by definition)
   void detachFuture() {
-    activate();
     detachOne();
   }
 
@@ -229,20 +228,6 @@ class Core final {
     detachOne();
   }
 
-  /// May call from any thread
-  void deactivate() {
-    active_.store(false, std::memory_order_release);
-  }
-
-  /// May call from any thread
-  void activate() {
-    active_.store(true, std::memory_order_release);
-    maybeCallback();
-  }
-
-  /// May call from any thread
-  bool isActive() const { return active_.load(std::memory_order_acquire); }
-
   /// Call only from Future thread, either before attaching a callback or after
   /// the callback has already been invoked, but not concurrently with anything
   /// which might trigger invocation of the callback
@@ -333,11 +318,8 @@ class Core final {
     fsm_.transition([&](State state) {
       switch (state) {
         case State::Armed:
-          if (active_.load(std::memory_order_acquire)) {
-            return fsm_.tryUpdateState(
-                state, State::Done, [] {}, [&] { doCallback(); });
-          }
-          return true;
+          return fsm_.tryUpdateState(
+              state, State::Done, [] {}, [&] { doCallback(); });
 
         default:
           return true;
@@ -433,7 +415,6 @@ class Core final {
   FSM<State, SpinLock> fsm_;
   std::atomic<unsigned char> attached_;
   std::atomic<unsigned char> callbackReferences_{0};
-  std::atomic<bool> active_ {true};
   std::atomic<bool> interruptHandlerSet_ {false};
   SpinLock interruptLock_;
   int8_t priority_ {-1};

folly/futures/test/CoreTest.cpp

@@ -29,7 +29,6 @@ TEST(Core, size) {
     futures::detail::FSM<futures::detail::State, futures::detail::SpinLock>
         fsm_;
     std::atomic<unsigned char> attached_;
-    std::atomic<bool> active_;
     std::atomic<bool> interruptHandlerSet_;
     folly::MicroSpinLock interruptLock_;
     int8_t priority_;