From ad0f7b2703ffc886a13ec5b420a5282cda380ae1 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Alejandro=20Pel=C3=A1ez?= Date: Sun, 13 Nov 2016 16:19:32 -0800 Subject: [PATCH] Revert D4164236: [EventBase] Move runAfterDelay/tryRunAfterDelay into TimeoutManager Summary: This reverts commit 5f2057f6ebfbdc971bc0b5594e3bc4b5a337aaef Differential Revision: D4164236 fbshipit-source-id: a397e6ba8c9d7a691cc6381b8f60bfcea3eb91b0 --- folly/Makefile.am | 1 - folly/io/async/EventBase.cpp | 54 +++++++++++++- folly/io/async/EventBase.h | 62 ++++++++++++++-- folly/io/async/TimeoutManager.cpp | 119 ------------------------------ folly/io/async/TimeoutManager.h | 35 +-------- 5 files changed, 109 insertions(+), 162 deletions(-) delete mode 100644 folly/io/async/TimeoutManager.cpp diff --git a/folly/Makefile.am b/folly/Makefile.am index 0ce66650..ef8500ab 100644 --- a/folly/Makefile.am +++ b/folly/Makefile.am @@ -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 \ diff --git a/folly/io/async/EventBase.cpp b/folly/io/async/EventBase.cpp index 2474a0e8..a4cbfa20 100644 --- a/folly/io/async/EventBase.cpp +++ b/folly/io/async/EventBase.cpp @@ -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(); diff --git a/folly/io/async/EventBase.h b/folly/io/async/EventBase.h index a8d4926f..16d406ea 100644 --- a/folly/io/async/EventBase.h +++ b/folly/io/async/EventBase.h @@ -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 > ListHook; + + ListHook hook; + + typedef boost::intrusive::list< + CobTimeout, + boost::intrusive::member_hook, + boost::intrusive::constant_time_size > 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 index 55832c00..00000000 --- a/folly/io/async/TimeoutManager.cpp +++ /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 - -#include - -#include -#include - -#include - -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>; - ListHook hook; - using List = boost::intrusive::list< - CobTimeout, - boost::intrusive::member_hook, - boost::intrusive::constant_time_size>; - }; - - CobTimeout::List list; -}; - -TimeoutManager::TimeoutManager() - : cobTimeouts_(std::make_unique()) {} - -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(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(); -} -} diff --git a/folly/io/async/TimeoutManager.h b/folly/io/async/TimeoutManager.h index 53093d02..e7df72e9 100644 --- a/folly/io/async/TimeoutManager.h +++ b/folly/io/async/TimeoutManager.h @@ -23,8 +23,6 @@ #include #include -#include - namespace folly { class AsyncTimeout; @@ -37,16 +35,13 @@ class AsyncTimeout; class TimeoutManager { public: typedef std::chrono::milliseconds timeout_type; - using Func = folly::Function; 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_; }; } // folly -- 2.34.1