From 24e2938fde0b44cf1b8b8edb7c103c2810e2e3a7 Mon Sep 17 00:00:00 2001 From: Kevin Schoedel Date: Wed, 11 Aug 2021 10:27:15 -0400 Subject: [PATCH] Have Linux/MdnsImpl use System::Layer timer. #### Problem Linux/MdnsImpl currently uses ad-hoc coupling with the POSIX I/O event loop, which intrudes on and constrains the latter's implementation. #### Change overview Replace the `GetMdnsTimeout()`/`HandleMdnsTimeout()` pair with a `System::Layer` timer tracking the earliest mDNS timeout. The last remnants of ~the Old Republic~ `select()` have been swept away. #### Testing Existing unit test TestMdns --- src/controller/java/MdnsImpl.cpp | 5 -- src/platform/Darwin/MdnsImpl.cpp | 4 -- src/platform/Linux/MdnsImpl.cpp | 68 +++++++++----------- src/platform/Linux/MdnsImpl.h | 6 +- src/system/WatchableEventManagerLibevent.cpp | 11 ---- src/system/WatchableEventManagerSelect.cpp | 19 ------ 6 files changed, 35 insertions(+), 78 deletions(-) diff --git a/src/controller/java/MdnsImpl.cpp b/src/controller/java/MdnsImpl.cpp index db0d6aaa978446..078ffb59fca374 100644 --- a/src/controller/java/MdnsImpl.cpp +++ b/src/controller/java/MdnsImpl.cpp @@ -103,11 +103,6 @@ CHIP_ERROR ChipMdnsResolve(MdnsService * service, Inet::InterfaceId interface, M return CHIP_NO_ERROR; } -// Implementation of other methods required by the CHIP stack - -void GetMdnsTimeout(timeval & timeout) {} -void HandleMdnsTimeout() {} - // Implemention of Java-specific functions void InitializeWithObjects(jobject resolverObject, jobject mdnsCallbackObject) diff --git a/src/platform/Darwin/MdnsImpl.cpp b/src/platform/Darwin/MdnsImpl.cpp index fe97ad1e3de6b2..2eda54e5a2ed1e 100644 --- a/src/platform/Darwin/MdnsImpl.cpp +++ b/src/platform/Darwin/MdnsImpl.cpp @@ -549,9 +549,5 @@ CHIP_ERROR ChipMdnsResolve(MdnsService * service, chip::Inet::InterfaceId interf return Resolve(context, callback, interfaceId, service->mAddressType, regtype.c_str(), service->mName); } -void GetMdnsTimeout(timeval & timeout) {} - -void HandleMdnsTimeout() {} - } // namespace Mdns } // namespace chip diff --git a/src/platform/Linux/MdnsImpl.cpp b/src/platform/Linux/MdnsImpl.cpp index e1a303c910c975..1e74e4b2cfcb41 100644 --- a/src/platform/Linux/MdnsImpl.cpp +++ b/src/platform/Linux/MdnsImpl.cpp @@ -29,6 +29,7 @@ #include #include #include +#include using chip::Mdns::kMdnsTypeMaxSize; using chip::Mdns::MdnsServiceProtocol; @@ -136,8 +137,6 @@ namespace Mdns { MdnsAvahi MdnsAvahi::sInstance; -constexpr uint64_t kUsPerSec = 1000 * 1000; - Poller::Poller() { mAvahiPoller.userdata = this; @@ -150,6 +149,7 @@ Poller::Poller() mAvahiPoller.timeout_update = TimeoutUpdate; mAvahiPoller.timeout_free = TimeoutFree; + mEarliestTimeout = std::chrono::steady_clock::time_point(); mWatchableEvents = &DeviceLayer::SystemLayer.WatchableEventsManager(); } @@ -239,9 +239,10 @@ steady_clock::time_point GetAbsTimeout(const struct timeval * timeout) AvahiTimeout * Poller::TimeoutNew(const struct timeval * timeout, AvahiTimeoutCallback callback, void * context) { - mTimers.emplace_back(new AvahiTimeout{ GetAbsTimeout(timeout), callback, timeout != nullptr, context, this }); - return mTimers.back().get(); + AvahiTimeout * timer = mTimers.back().get(); + SystemTimerUpdate(timer); + return timer; } void Poller::TimeoutUpdate(AvahiTimeout * timer, const struct timeval * timeout) @@ -250,6 +251,7 @@ void Poller::TimeoutUpdate(AvahiTimeout * timer, const struct timeval * timeout) { timer->mAbsTimeout = GetAbsTimeout(timeout); timer->mEnabled = true; + static_cast(timer->mPoller)->SystemTimerUpdate(timer); } else { @@ -269,48 +271,48 @@ void Poller::TimeoutFree(AvahiTimeout & timer) mTimers.end()); } -void Poller::GetTimeout(timeval & timeout) +void Poller::SystemTimerCallback(System::Layer * layer, void * data) { - microseconds timeoutVal = seconds(timeout.tv_sec) + microseconds(timeout.tv_usec); + static_cast(data)->HandleTimeout(); +} +void Poller::HandleTimeout() +{ + mEarliestTimeout = std::chrono::steady_clock::time_point(); + steady_clock::time_point now = steady_clock::now(); + + AvahiTimeout * earliest = nullptr; for (auto && timer : mTimers) { - steady_clock::time_point absTimeout = timer->mAbsTimeout; - steady_clock::time_point now = steady_clock::now(); - if (!timer->mEnabled) { continue; } - if (absTimeout < now) + if (timer->mAbsTimeout <= now) { - timeoutVal = microseconds(0); - break; + timer->mCallback(timer.get(), timer->mContext); } else { - timeoutVal = std::min(timeoutVal, duration_cast(absTimeout - now)); + if ((earliest == nullptr) || (timer->mAbsTimeout < earliest->mAbsTimeout)) + { + earliest = timer.get(); + } } } - - timeout.tv_sec = static_cast(timeoutVal.count()) / kUsPerSec; - timeout.tv_usec = static_cast(timeoutVal.count()) % kUsPerSec; + if (earliest) + { + SystemTimerUpdate(earliest); + } } -void Poller::HandleTimeout() +void Poller::SystemTimerUpdate(AvahiTimeout * timer) { - steady_clock::time_point now = steady_clock::now(); - - for (auto && timer : mTimers) + if ((mEarliestTimeout == std::chrono::steady_clock::time_point()) || (timer->mAbsTimeout < mEarliestTimeout)) { - if (!timer->mEnabled) - { - continue; - } - if (timer->mAbsTimeout <= now) - { - timer->mCallback(timer.get(), timer->mContext); - } + mEarliestTimeout = timer->mAbsTimeout; + auto msDelay = std::chrono::duration_cast(steady_clock::now() - mEarliestTimeout).count(); + DeviceLayer::SystemLayer.StartTimer(msDelay, SystemTimerCallback, this); } } @@ -753,16 +755,6 @@ void MdnsAvahi::HandleResolve(AvahiServiceResolver * resolver, AvahiIfIndex inte chip::Platform::Delete(context); } -void GetMdnsTimeout(timeval & timeout) -{ - MdnsAvahi::GetInstance().GetPoller().GetTimeout(timeout); -} - -void HandleMdnsTimeout() -{ - MdnsAvahi::GetInstance().GetPoller().HandleTimeout(); -} - CHIP_ERROR ChipMdnsInit(MdnsAsyncReturnCallback initCallback, MdnsAsyncReturnCallback errorCallback, void * context) { return MdnsAvahi::GetInstance().Init(initCallback, errorCallback, context); diff --git a/src/platform/Linux/MdnsImpl.h b/src/platform/Linux/MdnsImpl.h index d6a0bc44fa908d..f14dc760974630 100644 --- a/src/platform/Linux/MdnsImpl.h +++ b/src/platform/Linux/MdnsImpl.h @@ -62,7 +62,6 @@ class Poller public: Poller(void); - void GetTimeout(timeval & timeout); void HandleTimeout(); const AvahiPoll * GetAvahiPoll(void) const { return &mAvahiPoller; } @@ -88,8 +87,13 @@ class Poller static void TimeoutFree(AvahiTimeout * timer); void TimeoutFree(AvahiTimeout & timer); + void SystemTimerUpdate(AvahiTimeout * timer); + static void SystemTimerCallback(System::Layer * layer, void * data); + std::vector> mWatches; std::vector> mTimers; + std::chrono::steady_clock::time_point mEarliestTimeout; + AvahiPoll mAvahiPoller; System::WatchableEventManager * mWatchableEvents; }; diff --git a/src/system/WatchableEventManagerLibevent.cpp b/src/system/WatchableEventManagerLibevent.cpp index f0780198de1f26..129f8e02f90b0c 100644 --- a/src/system/WatchableEventManagerLibevent.cpp +++ b/src/system/WatchableEventManagerLibevent.cpp @@ -30,17 +30,6 @@ #include #include -#if CHIP_DEVICE_CONFIG_ENABLE_MDNS && !__ZEPHYR__ - -namespace chip { -namespace Mdns { -void GetMdnsTimeout(timeval & timeout); -void HandleMdnsTimeout(); -} // namespace Mdns -} // namespace chip - -#endif // CHIP_DEVICE_CONFIG_ENABLE_MDNS && !__ZEPHYR__ - #ifndef CHIP_CONFIG_LIBEVENT_DEBUG_CHECKS #define CHIP_CONFIG_LIBEVENT_DEBUG_CHECKS 1 #endif diff --git a/src/system/WatchableEventManagerSelect.cpp b/src/system/WatchableEventManagerSelect.cpp index c932911fac6e03..d463550286372f 100644 --- a/src/system/WatchableEventManagerSelect.cpp +++ b/src/system/WatchableEventManagerSelect.cpp @@ -37,17 +37,6 @@ #define PTHREAD_NULL 0 #endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING && !defined(PTHREAD_NULL) -#if CHIP_DEVICE_CONFIG_ENABLE_MDNS && !__ZEPHYR__ - -namespace chip { -namespace Mdns { -void GetMdnsTimeout(timeval & timeout); -void HandleMdnsTimeout(); -} // namespace Mdns -} // namespace chip - -#endif // CHIP_DEVICE_CONFIG_ENABLE_MDNS && !__ZEPHYR__ - namespace chip { namespace System { @@ -340,10 +329,6 @@ void WatchableEventManager::PrepareEvents() const Clock::MonotonicMilliseconds sleepTime = (awakenTime > currentTime) ? (awakenTime - currentTime) : 0; MillisecondsToTimeval(sleepTime, mNextTimeout); -#if CHIP_DEVICE_CONFIG_ENABLE_MDNS && !__ZEPHYR__ && !__MBED__ - chip::Mdns::GetMdnsTimeout(mNextTimeout); -#endif // CHIP_DEVICE_CONFIG_ENABLE_MDNS && !__ZEPHYR__ - mMaxFd = -1; FD_ZERO(&mSelected.mReadSet); FD_ZERO(&mSelected.mWriteSet); @@ -410,10 +395,6 @@ void WatchableEventManager::HandleEvents() } } -#if CHIP_DEVICE_CONFIG_ENABLE_MDNS && !__ZEPHYR__ && !__MBED__ - chip::Mdns::HandleMdnsTimeout(); -#endif // CHIP_DEVICE_CONFIG_ENABLE_MDNS && !__ZEPHYR__ - #if CHIP_SYSTEM_CONFIG_POSIX_LOCKING mHandleSelectThread = PTHREAD_NULL; #endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING