Revert D4164236: [EventBase] Move runAfterDelay/tryRunAfterDelay into TimeoutManager v2016.11.14.00
authorAlejandro Peláez <alejopelaez@fb.com>
Mon, 14 Nov 2016 00:19:32 +0000 (16:19 -0800)
committerFacebook Github Bot <facebook-github-bot-bot@fb.com>
Mon, 14 Nov 2016 00:23:27 +0000 (16:23 -0800)
Summary: This reverts commit 5f2057f6ebfbdc971bc0b5594e3bc4b5a337aaef

Differential Revision: D4164236

fbshipit-source-id: a397e6ba8c9d7a691cc6381b8f60bfcea3eb91b0

folly/Makefile.am
folly/io/async/EventBase.cpp
folly/io/async/EventBase.h
folly/io/async/TimeoutManager.cpp [deleted file]
folly/io/async/TimeoutManager.h

index 0ce666505b24a26a647b399b5ce70608d0c350a8..ef8500ab7490e81ce58a7aebfde2ab996c9b6f37 100644 (file)
@@ -447,7 +447,6 @@ libfolly_la_SOURCES = \
        io/async/SSLContext.cpp \
        io/async/ScopedEventBaseThread.cpp \
        io/async/HHWheelTimer.cpp \
-       io/async/TimeoutManager.cpp \
        io/async/test/ScopedBoundPort.cpp \
        io/async/test/SocketPair.cpp \
        io/async/test/TimeUtil.cpp \
index 2474a0e8cdd91840dba2c3a2aa93a9450bead423..a4cbfa2055724c6c94166ffa98f4db3f2e6734db 100644 (file)
@@ -89,6 +89,28 @@ class EventBase::FunctionRunner
   }
 };
 
+/*
+ * EventBase::CobTimeout methods
+ */
+
+void EventBase::CobTimeout::timeoutExpired() noexcept {
+  // For now, we just swallow any exceptions that the callback threw.
+  try {
+    cob_();
+  } catch (const std::exception& ex) {
+    LOG(ERROR) << "EventBase::runAfterDelay() callback threw "
+               << typeid(ex).name() << " exception: " << ex.what();
+  } catch (...) {
+    LOG(ERROR) << "EventBase::runAfterDelay() callback threw non-exception "
+               << "type";
+  }
+
+  // The CobTimeout object was allocated on the heap by runAfterDelay(),
+  // so delete it now that the it has fired.
+  delete this;
+}
+
+
 // The interface used to libevent is not thread-safe.  Calls to
 // event_init() and event_base_free() directly modify an internal
 // global 'current_base', so a mutex is required to protect this.
@@ -188,7 +210,14 @@ EventBase::~EventBase() {
     callback->runLoopCallback();
   }
 
-  clearCobTimeouts();
+  // Delete any unfired callback objects, so that we don't leak memory
+  // (Note that we don't fire them.  The caller is responsible for cleaning up
+  // its own data structures if it destroys the EventBase with unfired events
+  // remaining.)
+  while (!pendingCobTimeouts_.empty()) {
+    CobTimeout* timeout = &pendingCobTimeouts_.front();
+    delete timeout;
+  }
 
   while (!runBeforeLoopCallbacks_.empty()) {
     delete &runBeforeLoopCallbacks_.front();
@@ -591,6 +620,29 @@ bool EventBase::runImmediatelyOrRunInEventBaseThreadAndWait(Func fn) {
   }
 }
 
+void EventBase::runAfterDelay(
+    Func cob,
+    uint32_t milliseconds,
+    TimeoutManager::InternalEnum in) {
+  if (!tryRunAfterDelay(std::move(cob), milliseconds, in)) {
+    folly::throwSystemError(
+      "error in EventBase::runAfterDelay(), failed to schedule timeout");
+  }
+}
+
+bool EventBase::tryRunAfterDelay(
+    Func cob,
+    uint32_t milliseconds,
+    TimeoutManager::InternalEnum in) {
+  CobTimeout* timeout = new CobTimeout(this, std::move(cob), in);
+  if (!timeout->scheduleTimeout(milliseconds)) {
+    delete timeout;
+    return false;
+  }
+  pendingCobTimeouts_.push_back(*timeout);
+  return true;
+}
+
 bool EventBase::runLoopCallbacks() {
   if (!loopCallbacks_.empty()) {
     bumpHandlingTime();
index a8d4926f26ac0c05e16c8d6f7b78fe0c05741532..16d406eaa898c205bbcb5f05a2487fdd1d21b713 100644 (file)
@@ -397,6 +397,28 @@ class EventBase : private boost::noncopyable,
    */
   bool runImmediatelyOrRunInEventBaseThreadAndWait(Func fn);
 
+  /**
+   * Runs the given Cob at some time after the specified number of
+   * milliseconds.  (No guarantees exactly when.)
+   *
+   * Throws a std::system_error if an error occurs.
+   */
+  void runAfterDelay(
+      Func c,
+      uint32_t milliseconds,
+      TimeoutManager::InternalEnum in = TimeoutManager::InternalEnum::NORMAL);
+
+  /**
+   * @see tryRunAfterDelay for more details
+   *
+   * @return  true iff the cob was successfully registered.
+   *
+   * */
+  bool tryRunAfterDelay(
+      Func cob,
+      uint32_t milliseconds,
+      TimeoutManager::InternalEnum in = TimeoutManager::InternalEnum::NORMAL);
+
   /**
    * Set the maximum desired latency in us and provide a callback which will be
    * called when that latency is exceeded.
@@ -408,6 +430,7 @@ class EventBase : private boost::noncopyable,
     maxLatencyCob_ = std::move(maxLatencyCob);
   }
 
+
   /**
    * Set smoothing coefficient for loop load average; # of milliseconds
    * for exp(-1) (1/2.71828...) decay.
@@ -579,23 +602,22 @@ class EventBase : private boost::noncopyable,
     return LoopKeepAlive(this);
   }
 
+ private:
   // TimeoutManager
-  void attachTimeoutManager(
-      AsyncTimeout* obj,
-      TimeoutManager::InternalEnum internal) override final;
+  void attachTimeoutManager(AsyncTimeout* obj,
+                            TimeoutManager::InternalEnum internal) override;
 
-  void detachTimeoutManager(AsyncTimeout* obj) override final;
+  void detachTimeoutManager(AsyncTimeout* obj) override;
 
   bool scheduleTimeout(AsyncTimeout* obj, TimeoutManager::timeout_type timeout)
-      override final;
+    override;
 
-  void cancelTimeout(AsyncTimeout* obj) override final;
+  void cancelTimeout(AsyncTimeout* obj) override;
 
   bool isInTimeoutManagerThread() override final {
     return isInEventBaseThread();
   }
 
- private:
   void applyLoopKeepAlive();
 
   /*
@@ -604,6 +626,30 @@ class EventBase : private boost::noncopyable,
    */
   bool nothingHandledYet() const noexcept;
 
+  // small object used as a callback arg with enough info to execute the
+  // appropriate client-provided Cob
+  class CobTimeout : public AsyncTimeout {
+   public:
+    CobTimeout(EventBase* b, Func c, TimeoutManager::InternalEnum in)
+        : AsyncTimeout(b, in), cob_(std::move(c)) {}
+
+    virtual void timeoutExpired() noexcept;
+
+   private:
+    Func cob_;
+
+   public:
+    typedef boost::intrusive::list_member_hook<
+      boost::intrusive::link_mode<boost::intrusive::auto_unlink> > ListHook;
+
+    ListHook hook;
+
+    typedef boost::intrusive::list<
+      CobTimeout,
+      boost::intrusive::member_hook<CobTimeout, ListHook, &CobTimeout::hook>,
+      boost::intrusive::constant_time_size<false> > List;
+  };
+
   typedef LoopCallback::List LoopCallbackList;
   class FunctionRunner;
 
@@ -617,6 +663,8 @@ class EventBase : private boost::noncopyable,
   // should only be accessed through public getter
   HHWheelTimer::UniquePtr wheelTimer_;
 
+  CobTimeout::List pendingCobTimeouts_;
+
   LoopCallbackList loopCallbacks_;
   LoopCallbackList runBeforeLoopCallbacks_;
   LoopCallbackList onDestructionCallbacks_;
diff --git a/folly/io/async/TimeoutManager.cpp b/folly/io/async/TimeoutManager.cpp
deleted file mode 100644 (file)
index 55832c0..0000000
+++ /dev/null
@@ -1,119 +0,0 @@
-/*
- * Copyright 2016 Facebook, Inc.
- *
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-#include <folly/io/async/TimeoutManager.h>
-
-#include <boost/intrusive/list.hpp>
-
-#include <folly/Exception.h>
-#include <folly/io/async/AsyncTimeout.h>
-
-#include <glog/logging.h>
-
-namespace folly {
-
-struct TimeoutManager::CobTimeouts {
-  // small object used as a callback arg with enough info to execute the
-  // appropriate client-provided Cob
-  class CobTimeout : public AsyncTimeout {
-   public:
-    CobTimeout(TimeoutManager* timeoutManager, Func cob, InternalEnum internal)
-        : AsyncTimeout(timeoutManager, internal), cob_(std::move(cob)) {}
-
-    void timeoutExpired() noexcept override {
-      // For now, we just swallow any exceptions that the callback threw.
-      try {
-        cob_();
-      } catch (const std::exception& ex) {
-        LOG(ERROR) << "TimeoutManager::runAfterDelay() callback threw "
-                   << typeid(ex).name() << " exception: " << ex.what();
-      } catch (...) {
-        LOG(ERROR) << "TimeoutManager::runAfterDelay() callback threw "
-                   << "non-exception type";
-      }
-
-      // The CobTimeout object was allocated on the heap by runAfterDelay(),
-      // so delete it now that the it has fired.
-      delete this;
-    }
-
-   private:
-    Func cob_;
-
-   public:
-    using ListHook = boost::intrusive::list_member_hook<
-        boost::intrusive::link_mode<boost::intrusive::auto_unlink>>;
-    ListHook hook;
-    using List = boost::intrusive::list<
-        CobTimeout,
-        boost::intrusive::member_hook<CobTimeout, ListHook, &CobTimeout::hook>,
-        boost::intrusive::constant_time_size<false>>;
-  };
-
-  CobTimeout::List list;
-};
-
-TimeoutManager::TimeoutManager()
-    : cobTimeouts_(std::make_unique<CobTimeouts>()) {}
-
-void TimeoutManager::runAfterDelay(
-    Func cob,
-    uint32_t milliseconds,
-    InternalEnum internal) {
-  if (!tryRunAfterDelay(std::move(cob), milliseconds, internal)) {
-    folly::throwSystemError(
-        "error in TimeoutManager::runAfterDelay(), failed to schedule timeout");
-  }
-}
-
-bool TimeoutManager::tryRunAfterDelay(
-    Func cob,
-    uint32_t milliseconds,
-    InternalEnum internal) {
-  if (!cobTimeouts_) {
-    return false;
-  }
-
-  auto timeout =
-      std::make_unique<CobTimeouts::CobTimeout>(this, std::move(cob), internal);
-  if (!timeout->scheduleTimeout(milliseconds)) {
-    return false;
-  }
-  cobTimeouts_->list.push_back(*timeout.release());
-  return true;
-}
-
-void TimeoutManager::clearCobTimeouts() {
-  if (!cobTimeouts_) {
-    return;
-  }
-
-  // Delete any unfired callback objects, so that we don't leak memory
-  // Note that we don't fire them.
-  while (!cobTimeouts_->list.empty()) {
-    auto* timeout = &cobTimeouts_->list.front();
-    delete timeout;
-  }
-}
-
-TimeoutManager::~TimeoutManager() {
-  clearCobTimeouts();
-}
-}
index 53093d02a74ccd197d69040266b93119e75204ce..e7df72e9893edfa4d221f3506c87683011548efe 100644 (file)
@@ -23,8 +23,6 @@
 #include <chrono>
 #include <stdint.h>
 
-#include <folly/Function.h>
-
 namespace folly {
 
 class AsyncTimeout;
@@ -37,16 +35,13 @@ class AsyncTimeout;
 class TimeoutManager {
  public:
   typedef std::chrono::milliseconds timeout_type;
-  using Func = folly::Function<void()>;
 
   enum class InternalEnum {
     INTERNAL,
     NORMAL
   };
 
-  TimeoutManager();
-
-  virtual ~TimeoutManager();
+  virtual ~TimeoutManager() = default;
 
   /**
    * Attaches/detaches TimeoutManager to AsyncTimeout
@@ -77,34 +72,6 @@ class TimeoutManager {
    * thread
    */
   virtual bool isInTimeoutManagerThread() = 0;
-
-  /**
-   * Runs the given Cob at some time after the specified number of
-   * milliseconds.  (No guarantees exactly when.)
-   *
-   * Throws a std::system_error if an error occurs.
-   */
-  void runAfterDelay(
-      Func cob,
-      uint32_t milliseconds,
-      InternalEnum internal = InternalEnum::NORMAL);
-
-  /**
-   * @see tryRunAfterDelay for more details
-   *
-   * @return  true iff the cob was successfully registered.
-   */
-  bool tryRunAfterDelay(
-      Func cob,
-      uint32_t milliseconds,
-      InternalEnum internal = InternalEnum::NORMAL);
-
- protected:
-  void clearCobTimeouts();
-
- private:
-  struct CobTimeouts;
-  std::unique_ptr<CobTimeouts> cobTimeouts_;
 };
 
 } // folly