From 1a61493fec7ca4d8d64e8325ba7bbe82fa78116a Mon Sep 17 00:00:00 2001 From: Haim Grosman Date: Wed, 4 Feb 2015 09:15:10 -0800 Subject: [PATCH] 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 --- folly/io/async/EventBase.cpp | 16 ++++++--- folly/io/async/EventBase.h | 17 +++++++-- folly/io/async/test/EventBaseTest.cpp | 46 ++++++++++++------------ folly/wangle/bootstrap/BootstrapTest.cpp | 2 +- 4 files changed, 50 insertions(+), 31 deletions(-) diff --git a/folly/io/async/EventBase.cpp b/folly/io/async/EventBase.cpp index 0a7b0bb7..33b10023 100644 --- 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 769d72fb..8f70dd59 100644 --- 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 f80cdcc4..baad1343 100644 --- 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, ×tamp1), 10); - eb.runAfterDelay(std::bind(&TimePoint::reset, ×tamp2), 20); - eb.runAfterDelay(std::bind(&TimePoint::reset, ×tamp3), 40); + eb.tryRunAfterDelay(std::bind(&TimePoint::reset, ×tamp1), 10); + eb.tryRunAfterDelay(std::bind(&TimePoint::reset, ×tamp2), 20); + eb.tryRunAfterDelay(std::bind(&TimePoint::reset, ×tamp3), 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, ×tamp1), 10); - eb.runAfterDelay(std::bind(&TimePoint::reset, ×tamp2), 20); + eb.tryRunAfterDelay(std::bind(&TimePoint::reset, ×tamp1), 10); + eb.tryRunAfterDelay(std::bind(&TimePoint::reset, ×tamp2), 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, ×tamp3), 80); - eb.runAfterDelay(std::bind(&TimePoint::reset, ×tamp4), 160); + eb.tryRunAfterDelay(std::bind(&TimePoint::reset, ×tamp3), 80); + eb.tryRunAfterDelay(std::bind(&TimePoint::reset, ×tamp4), 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 4bbd80c2..b0e359a3 100644 --- a/folly/wangle/bootstrap/BootstrapTest.cpp +++ b/folly/wangle/bootstrap/BootstrapTest.cpp @@ -36,7 +36,7 @@ class TestClientPipelineFactory : public PipelineFactory { 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; -- 2.34.1