EventBase::runAfterDelay to throw an exception

Summary:
EventBase::runAfterDelay to throw an exception in case it fails to
schedule a proper execution of the given callback
(instead of silently returning false)

it appears to be the right thing to do, since:
@davejwatson: Digging through the layers of code, it appears this can only fail if
epoll_ctl() with EPOLL_CTL_ADD fails. Ignoring libevent bugs, it looks
like the only relevant errors could be ENOMEM or ENOSPC. So
nonrecoverable

Test Plan: Unit tests

Reviewed By: anca@fb.com

Subscribers: yzhan, haijunz, simpkins, net-systems@, varunk, zeus-diffs@, nli, dfechete, fugalh, atlas2-eng@, alandau, bmatheny, everstore-dev@, zhuohuang, wormhole-diffs@, anca, mwa, jgehring, oujin, alikhtarov, fuegen, mshneer, wch, bil, sanketh, zippydb, maxwellsayles, jsedgwick, trunkagent, fbcode-common-diffs@, chaoyc, search-fbcode-diffs@, andrewcox, unicorn-diffs@, tw-eng@, xie, kennyyu, yfeldblum, folly-diffs@, davejwatson

FB internal diff: D1805125

Signature: t1:1805125:1424927912:8bebb4c3b9f1fa189c0ce97b12cdb8f95dba97ae

diff --git a/folly/io/async/EventBase.cpp b/folly/io/async/EventBase.cpp
index 0a7b0bb7d0a18c581131badbfd893c358670fe3f..33b100235ad0afb78877ef548045da507554c8d1 100644 (file)
--- a/folly/io/async/EventBase.cpp
+++ b/folly/io/async/EventBase.cpp
@@ -610,15 +610,23 @@ bool EventBase::runInEventBaseThreadAndWait(const Cob& fn) {
   return true;
 }
 
-bool EventBase::runAfterDelay(const Cob& cob,
-                               int milliseconds,
-                               TimeoutManager::InternalEnum in) {
+void EventBase::runAfterDelay(const Cob& cob,
+                              int milliseconds,
+                              TimeoutManager::InternalEnum in) {
+  if (!tryRunAfterDelay(cob, milliseconds, in)) {
+    folly::throwSystemError(
+      "error in EventBase::runAfterDelay(), failed to schedule timeout");
+  }
+}
+
+bool EventBase::tryRunAfterDelay(const Cob& cob,
+                                 int milliseconds,
+                                 TimeoutManager::InternalEnum in) {
   CobTimeout* timeout = new CobTimeout(this, cob, in);
   if (!timeout->scheduleTimeout(milliseconds)) {
     delete timeout;
     return false;
   }
-
   pendingCobTimeouts_.push_back(*timeout);
   return true;
 }

diff --git a/folly/io/async/EventBase.h b/folly/io/async/EventBase.h
index 769d72fb058512ef4d48d148b42942993a384402..8f70dd592446b7da6e1ca7c59b65a8cfbf17b69a 100644 (file)
--- a/folly/io/async/EventBase.h
+++ b/folly/io/async/EventBase.h
@@ -386,12 +386,23 @@ class EventBase : private boost::noncopyable,
    * Runs the given Cob at some time after the specified number of
    * milliseconds.  (No guarantees exactly when.)
    *
-   * @return  true iff the cob was successfully registered.
+   * Throws a std::system_error if an error occurs.
    */
-  bool runAfterDelay(
+  void runAfterDelay(
       const Cob& c,
       int milliseconds,
-      TimeoutManager::InternalEnum = TimeoutManager::InternalEnum::NORMAL);
+      TimeoutManager::InternalEnum in = TimeoutManager::InternalEnum::NORMAL);
+
+  /**
+   * @see tryRunAfterDelay for more details
+   *
+   * @return  true iff the cob was successfully registered.
+   *
+   * */
+  bool tryRunAfterDelay(
+      const Cob& cob,
+      int milliseconds,
+      TimeoutManager::InternalEnum in = TimeoutManager::InternalEnum::NORMAL);
 
   /**
    * Set the maximum desired latency in us and provide a callback which will be

diff --git a/folly/io/async/test/EventBaseTest.cpp b/folly/io/async/test/EventBaseTest.cpp
index f80cdcc4631dd953887e6434c4d30615270f10ae..baad13437ada623f964c92dbc230198f85abbe03 100644 (file)
--- a/folly/io/async/test/EventBaseTest.cpp
+++ b/folly/io/async/test/EventBaseTest.cpp
@@ -131,7 +131,7 @@ struct ScheduledEvent {
 
 void scheduleEvents(EventBase* eventBase, int fd, ScheduledEvent* events) {
   for (ScheduledEvent* ev = events; ev->milliseconds > 0; ++ev) {
-    eventBase->runAfterDelay(std::bind(&ScheduledEvent::perform, ev, fd),
+    eventBase->tryRunAfterDelay(std::bind(&ScheduledEvent::perform, ev, fd),
                              ev->milliseconds);
   }
 }
@@ -240,7 +240,7 @@ TEST(EventBaseTest, ReadPersist) {
   scheduleEvents(&eb, sp[1], events);
 
   // Schedule a timeout to unregister the handler after the third write
-  eb.runAfterDelay(std::bind(&TestHandler::unregisterHandler, &handler), 85);
+  eb.tryRunAfterDelay(std::bind(&TestHandler::unregisterHandler, &handler), 85);
 
   // Loop
   TimePoint start;
@@ -288,7 +288,7 @@ TEST(EventBaseTest, ReadImmediate) {
   scheduleEvents(&eb, sp[1], events);
 
   // Schedule a timeout to unregister the handler
-  eb.runAfterDelay(std::bind(&TestHandler::unregisterHandler, &handler), 20);
+  eb.tryRunAfterDelay(std::bind(&TestHandler::unregisterHandler, &handler), 20);
 
   // Loop
   TimePoint start;
@@ -379,7 +379,7 @@ TEST(EventBaseTest, WritePersist) {
   scheduleEvents(&eb, sp[1], events);
 
   // Schedule a timeout to unregister the handler after the third read
-  eb.runAfterDelay(std::bind(&TestHandler::unregisterHandler, &handler), 85);
+  eb.tryRunAfterDelay(std::bind(&TestHandler::unregisterHandler, &handler), 85);
 
   // Loop
   TimePoint start;
@@ -421,7 +421,7 @@ TEST(EventBaseTest, WriteImmediate) {
 
   // Schedule a timeout to unregister the handler
   int64_t unregisterTimeout = 40;
-  eb.runAfterDelay(std::bind(&TestHandler::unregisterHandler, &handler),
+  eb.tryRunAfterDelay(std::bind(&TestHandler::unregisterHandler, &handler),
                    unregisterTimeout);
 
   // Loop
@@ -601,7 +601,7 @@ TEST(EventBaseTest, ReadWritePersist) {
   scheduleEvents(&eb, sp[1], events);
 
   // Schedule a timeout to unregister the handler
-  eb.runAfterDelay(std::bind(&TestHandler::unregisterHandler, &handler), 80);
+  eb.tryRunAfterDelay(std::bind(&TestHandler::unregisterHandler, &handler), 80);
 
   // Loop
   TimePoint start;
@@ -679,7 +679,7 @@ TEST(EventBaseTest, ReadPartial) {
   scheduleEvents(&eb, sp[1], events);
 
   // Schedule a timeout to unregister the handler
-  eb.runAfterDelay(std::bind(&TestHandler::unregisterHandler, &handler), 30);
+  eb.tryRunAfterDelay(std::bind(&TestHandler::unregisterHandler, &handler), 30);
 
   // Loop
   TimePoint start;
@@ -746,7 +746,7 @@ TEST(EventBaseTest, WritePartial) {
   scheduleEvents(&eb, sp[1], events);
 
   // Schedule a timeout to unregister the handler
-  eb.runAfterDelay(std::bind(&TestHandler::unregisterHandler, &handler), 30);
+  eb.tryRunAfterDelay(std::bind(&TestHandler::unregisterHandler, &handler), 30);
 
   // Loop
   TimePoint start;
@@ -800,7 +800,7 @@ TEST(EventBaseTest, DestroyHandler) {
 
   // After 10ms, read some data, so that the handler
   // will be notified that it can write.
-  eb.runAfterDelay(std::bind(checkReadUntilEmpty, sp[1], initialBytesWritten),
+  eb.tryRunAfterDelay(std::bind(checkReadUntilEmpty, sp[1], initialBytesWritten),
                    10);
 
   // Start a timer to destroy the handler after 25ms
@@ -833,9 +833,9 @@ TEST(EventBaseTest, RunAfterDelay) {
   TimePoint timestamp1(false);
   TimePoint timestamp2(false);
   TimePoint timestamp3(false);
-  eb.runAfterDelay(std::bind(&TimePoint::reset, &timestamp1), 10);
-  eb.runAfterDelay(std::bind(&TimePoint::reset, &timestamp2), 20);
-  eb.runAfterDelay(std::bind(&TimePoint::reset, &timestamp3), 40);
+  eb.tryRunAfterDelay(std::bind(&TimePoint::reset, &timestamp1), 10);
+  eb.tryRunAfterDelay(std::bind(&TimePoint::reset, &timestamp2), 20);
+  eb.tryRunAfterDelay(std::bind(&TimePoint::reset, &timestamp3), 40);
 
   TimePoint start;
   eb.loop();
@@ -848,7 +848,7 @@ TEST(EventBaseTest, RunAfterDelay) {
 }
 
 /**
- * Test the behavior of runAfterDelay() when some timeouts are
+ * Test the behavior of tryRunAfterDelay() when some timeouts are
  * still scheduled when the EventBase is destroyed.
  */
 TEST(EventBaseTest, RunAfterDelayDestruction) {
@@ -863,15 +863,15 @@ TEST(EventBaseTest, RunAfterDelayDestruction) {
     EventBase eb;
 
     // Run two normal timeouts
-    eb.runAfterDelay(std::bind(&TimePoint::reset, &timestamp1), 10);
-    eb.runAfterDelay(std::bind(&TimePoint::reset, &timestamp2), 20);
+    eb.tryRunAfterDelay(std::bind(&TimePoint::reset, &timestamp1), 10);
+    eb.tryRunAfterDelay(std::bind(&TimePoint::reset, &timestamp2), 20);
 
     // Schedule a timeout to stop the event loop after 40ms
-    eb.runAfterDelay(std::bind(&EventBase::terminateLoopSoon, &eb), 40);
+    eb.tryRunAfterDelay(std::bind(&EventBase::terminateLoopSoon, &eb), 40);
 
     // Schedule 2 timeouts that would fire after the event loop stops
-    eb.runAfterDelay(std::bind(&TimePoint::reset, &timestamp3), 80);
-    eb.runAfterDelay(std::bind(&TimePoint::reset, &timestamp4), 160);
+    eb.tryRunAfterDelay(std::bind(&TimePoint::reset, &timestamp3), 80);
+    eb.tryRunAfterDelay(std::bind(&TimePoint::reset, &timestamp4), 160);
 
     start.reset();
     eb.loop();
@@ -1003,9 +1003,9 @@ TEST(EventBaseTest, RescheduleTimeout) {
       &AsyncTimeout::scheduleTimeout);
 
   // after 10ms, reschedule t2 to run sooner than originally scheduled
-  eb.runAfterDelay(std::bind(f, &t2, 10), 10);
+  eb.tryRunAfterDelay(std::bind(f, &t2, 10), 10);
   // after 10ms, reschedule t3 to run later than originally scheduled
-  eb.runAfterDelay(std::bind(f, &t3, 40), 10);
+  eb.tryRunAfterDelay(std::bind(f, &t3, 40), 10);
 
   TimePoint start;
   eb.loop();
@@ -1030,7 +1030,7 @@ TEST(EventBaseTest, CancelTimeout) {
 
   ReschedulingTimeout t(&eb, timeouts);
   t.start();
-  eb.runAfterDelay(std::bind(&AsyncTimeout::cancelTimeout, &t), 50);
+  eb.tryRunAfterDelay(std::bind(&AsyncTimeout::cancelTimeout, &t), 50);
 
   TimePoint start;
   eb.loop();
@@ -1137,7 +1137,7 @@ TEST(EventBaseTest, RunInThread) {
   // Once the last thread exits, it will stop the loop().  However, this
   // timeout also stops the loop in case there is a bug performing the normal
   // stop.
-  data.evb.runAfterDelay(std::bind(&EventBase::terminateLoopSoon, &data.evb),
+  data.evb.tryRunAfterDelay(std::bind(&EventBase::terminateLoopSoon, &data.evb),
                          3000);
 
   TimePoint start;
@@ -1628,7 +1628,7 @@ TEST(EventBaseTest, RunBeforeLoop) {
 TEST(EventBaseTest, RunBeforeLoopWait) {
   EventBase base;
   CountedLoopCallback cb(&base, 1);
-  base.runAfterDelay([&](){
+  base.tryRunAfterDelay([&](){
       base.terminateLoopSoon();
     }, 500);
   base.runBeforeLoop(&cb);

diff --git a/folly/wangle/bootstrap/BootstrapTest.cpp b/folly/wangle/bootstrap/BootstrapTest.cpp
index 4bbd80c22f22a5442be0e84377e1ebe1f76598b8..b0e359a3306a1247988e7e3c0568a54914feb509 100644 (file)
--- a/folly/wangle/bootstrap/BootstrapTest.cpp
+++ b/folly/wangle/bootstrap/BootstrapTest.cpp
@@ -36,7 +36,7 @@ class TestClientPipelineFactory : public PipelineFactory<Pipeline> {
     CHECK(sock->good());
 
     // We probably aren't connected immedately, check after a small delay
-    EventBaseManager::get()->getEventBase()->runAfterDelay([sock](){
+    EventBaseManager::get()->getEventBase()->tryRunAfterDelay([sock](){
       CHECK(sock->readable());
     }, 100);
     return nullptr;