From 1170540838b544c19772ca85748f017cf7c4e425 Mon Sep 17 00:00:00 2001 From: Kevin Schoedel <67607049+kpschoedel@users.noreply.github.com> Date: Tue, 7 Dec 2021 09:45:23 -0500 Subject: [PATCH] Streamline ObjectPool in System::Timer (#12628) #### Problem After the unification of the former `system/SystemObject.h` with `support/Pool.h`, there is an opportunity to simplify `System::Timer`. #### Change overview - Clean up use of `ObjectPool`, with a `TimerPool` wrapper. - Fix some #includes formerly indirectly included by `SystemTimer.h` #### Testing CI; no changes to external functionality. --- src/credentials/CertificationDeclaration.cpp | 5 +- src/inet/InetLayer.cpp | 3 + src/inet/InetLayer.h | 1 + src/inet/tests/TestInetEndPoint.cpp | 28 +- .../support/TestPersistentStorageDelegate.h | 1 + src/lib/support/Variant.h | 1 + src/messaging/ExchangeMessageDispatch.cpp | 1 + src/platform/Linux/ConnectivityManagerImpl.h | 2 + src/system/SystemConfig.h | 14 - src/system/SystemLayer.h | 3 +- src/system/SystemLayerImplLwIP.cpp | 38 +-- src/system/SystemLayerImplLwIP.h | 4 +- src/system/SystemLayerImplSelect.cpp | 39 ++- src/system/SystemLayerImplSelect.h | 6 +- src/system/SystemTimer.cpp | 206 +++--------- src/system/SystemTimer.h | 316 ++++++++---------- 16 files changed, 255 insertions(+), 413 deletions(-) diff --git a/src/credentials/CertificationDeclaration.cpp b/src/credentials/CertificationDeclaration.cpp index 5ca6224a5ff605..72d8fe7af89d74 100644 --- a/src/credentials/CertificationDeclaration.cpp +++ b/src/credentials/CertificationDeclaration.cpp @@ -22,8 +22,9 @@ * working with Certification Declaration elements. */ -#include -#include +#include +#include +#include #include #include diff --git a/src/inet/InetLayer.cpp b/src/inet/InetLayer.cpp index 06024c7f582359..57229256d50b5f 100644 --- a/src/inet/InetLayer.cpp +++ b/src/inet/InetLayer.cpp @@ -48,7 +48,10 @@ CHIP_ERROR InetLayer::Shutdown() { VerifyOrReturnError(mLayerState.SetShuttingDown(), CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mTCPEndPointManager == nullptr, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mUDPEndPointManager != nullptr, CHIP_ERROR_INCORRECT_STATE); ReturnErrorOnFailure(mUDPEndPointManager->Shutdown()); + mUDPEndPointManager = nullptr; + mSystemLayer = nullptr; mLayerState.ResetFromShuttingDown(); // Return to uninitialized state to permit re-initialization. return CHIP_NO_ERROR; } diff --git a/src/inet/InetLayer.h b/src/inet/InetLayer.h index a028a76eee979a..cace73625b104f 100644 --- a/src/inet/InetLayer.h +++ b/src/inet/InetLayer.h @@ -23,6 +23,7 @@ #pragma once #include +#include #include #include #include diff --git a/src/inet/tests/TestInetEndPoint.cpp b/src/inet/tests/TestInetEndPoint.cpp index 0fab309d824c66..f6aacc3fbce7d3 100644 --- a/src/inet/tests/TestInetEndPoint.cpp +++ b/src/inet/tests/TestInetEndPoint.cpp @@ -348,16 +348,19 @@ static void TestInetEndPointLimit(nlTestSuite * inSuite, void * inContext) CHIP_ERROR err = CHIP_NO_ERROR; - // TODO: err is not validated EXCEPT the last call - for (int i = 0; i < INET_CONFIG_NUM_UDP_ENDPOINTS + 1; i++) + for (int i = INET_CONFIG_NUM_UDP_ENDPOINTS; i >= 0; --i) + { err = gInet.GetUDPEndPointManager()->NewEndPoint(&testUDPEP[i]); - NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_ENDPOINT_POOL_FULL); + NL_TEST_ASSERT(inSuite, err == (i ? CHIP_NO_ERROR : CHIP_ERROR_ENDPOINT_POOL_FULL)); + } - // TODO: err is not validated EXCEPT the last call - for (int i = 0; i < INET_CONFIG_NUM_TCP_ENDPOINTS + 1; i++) + for (int i = INET_CONFIG_NUM_TCP_ENDPOINTS; i >= 0; --i) + { err = gInet.GetTCPEndPointManager()->NewEndPoint(&testTCPEP[i]); - NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_ENDPOINT_POOL_FULL); + NL_TEST_ASSERT(inSuite, err == (i ? CHIP_NO_ERROR : CHIP_ERROR_ENDPOINT_POOL_FULL)); + } +#if CHIP_SYSTEM_CONFIG_NUM_TIMERS // Verify same aComplete and aAppState args do not exhaust timer pool for (int i = 0; i < CHIP_SYSTEM_CONFIG_NUM_TIMERS + 1; i++) { @@ -365,18 +368,14 @@ static void TestInetEndPointLimit(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); } -#if CHIP_SYSTEM_CONFIG_USE_TIMER_POOL char numTimersTest[CHIP_SYSTEM_CONFIG_NUM_TIMERS + 1]; for (int i = 0; i < CHIP_SYSTEM_CONFIG_NUM_TIMERS + 1; i++) err = gSystemLayer.StartTimer(10_ms32, HandleTimer, &numTimersTest[i]); NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_NO_MEMORY); -#endif // CHIP_SYSTEM_CONFIG_USE_TIMER_POOL - - ShutdownNetwork(); - ShutdownSystemLayer(); +#endif // CHIP_SYSTEM_CONFIG_NUM_TIMERS // Release UDP endpoints - for (int i = 0; i < INET_CONFIG_NUM_UDP_ENDPOINTS; i++) + for (int i = 0; i <= INET_CONFIG_NUM_UDP_ENDPOINTS; i++) { if (testUDPEP[i] != nullptr) { @@ -385,13 +384,16 @@ static void TestInetEndPointLimit(nlTestSuite * inSuite, void * inContext) } // Release TCP endpoints - for (int i = 0; i < INET_CONFIG_NUM_TCP_ENDPOINTS; i++) + for (int i = 0; i <= INET_CONFIG_NUM_TCP_ENDPOINTS; i++) { if (testTCPEP[i] != nullptr) { testTCPEP[i]->Free(); } } + + ShutdownNetwork(); + ShutdownSystemLayer(); } #endif diff --git a/src/lib/support/TestPersistentStorageDelegate.h b/src/lib/support/TestPersistentStorageDelegate.h index e21159cb0c1581..70c70fd12d56f4 100644 --- a/src/lib/support/TestPersistentStorageDelegate.h +++ b/src/lib/support/TestPersistentStorageDelegate.h @@ -23,6 +23,7 @@ #include #include #include +#include #include namespace chip { diff --git a/src/lib/support/Variant.h b/src/lib/support/Variant.h index 8c6a99dae3dc41..80929f2750a36a 100644 --- a/src/lib/support/Variant.h +++ b/src/lib/support/Variant.h @@ -22,6 +22,7 @@ #include #include +#include #include #include #include diff --git a/src/messaging/ExchangeMessageDispatch.cpp b/src/messaging/ExchangeMessageDispatch.cpp index d683b10f47b9b0..8b5a26d8898245 100644 --- a/src/messaging/ExchangeMessageDispatch.cpp +++ b/src/messaging/ExchangeMessageDispatch.cpp @@ -28,6 +28,7 @@ #define __STDC_LIMIT_MACROS #endif +#include #include #include diff --git a/src/platform/Linux/ConnectivityManagerImpl.h b/src/platform/Linux/ConnectivityManagerImpl.h index cbaeb242c9a430..0adc1f52a21c49 100644 --- a/src/platform/Linux/ConnectivityManagerImpl.h +++ b/src/platform/Linux/ConnectivityManagerImpl.h @@ -41,6 +41,8 @@ #include #include #include + +#include #endif namespace chip { diff --git a/src/system/SystemConfig.h b/src/system/SystemConfig.h index 0b2ec5c382516d..7e8450af33a95f 100644 --- a/src/system/SystemConfig.h +++ b/src/system/SystemConfig.h @@ -439,20 +439,6 @@ struct LwIPEvent; #define CHIP_SYSTEM_CONFIG_NUM_TIMERS 32 #endif /* CHIP_SYSTEM_CONFIG_NUM_TIMERS */ -/** - * @def CHIP_SYSTEM_CONFIG_USE_TIMER_POOL - * - * @brief - * This defines whether (1) or not (0) the implementation uses the System::Timer pool. - */ -#ifndef CHIP_SYSTEM_CONFIG_USE_TIMER_POOL -#if CHIP_SYSTEM_CONFIG_NUM_TIMERS > 0 -#define CHIP_SYSTEM_CONFIG_USE_TIMER_POOL 1 -#else -#define CHIP_SYSTEM_CONFIG_USE_TIMER_POOL 0 -#endif -#endif /* CHIP_SYSTEM_CONFIG_USE_TIMER_POOL */ - /** * @def CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS * diff --git a/src/system/SystemLayer.h b/src/system/SystemLayer.h index 0aa97ce24ca80a..c8fb4d5e28e19e 100644 --- a/src/system/SystemLayer.h +++ b/src/system/SystemLayer.h @@ -37,7 +37,6 @@ #include #include #include -#include #if CHIP_SYSTEM_CONFIG_USE_SOCKETS #include @@ -47,6 +46,8 @@ #include #endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH +#include + namespace chip { namespace System { diff --git a/src/system/SystemLayerImplLwIP.cpp b/src/system/SystemLayerImplLwIP.cpp index 5957328cbb7a76..7c03a18600be45 100644 --- a/src/system/SystemLayerImplLwIP.cpp +++ b/src/system/SystemLayerImplLwIP.cpp @@ -38,8 +38,6 @@ CHIP_ERROR LayerImplLwIP::Init() RegisterLwIPErrorFormatter(); - ReturnErrorOnFailure(mTimerList.Init()); - VerifyOrReturnError(mLayerState.SetInitialized(), CHIP_ERROR_INCORRECT_STATE); return CHIP_NO_ERROR; } @@ -58,7 +56,7 @@ CHIP_ERROR LayerImplLwIP::StartTimer(Clock::Timeout delay, TimerCompleteCallback CancelTimer(onComplete, appState); - Timer * timer = Timer::New(*this, delay, onComplete, appState); + TimerList::Node * timer = mTimerPool.Create(*this, delay, onComplete, appState); VerifyOrReturnError(timer != nullptr, CHIP_ERROR_NO_MEMORY); if (mTimerList.Add(timer) == timer) @@ -78,34 +76,25 @@ void LayerImplLwIP::CancelTimer(TimerCompleteCallback onComplete, void * appStat { VerifyOrReturn(mLayerState.IsInitialized()); - Timer * timer = mTimerList.Remove(onComplete, appState); - VerifyOrReturn(timer != nullptr); - - timer->Clear(); - timer->Release(); + TimerList::Node * timer = mTimerList.Remove(onComplete, appState); + if (timer != nullptr) + { + mTimerPool.Release(timer); + } } CHIP_ERROR LayerImplLwIP::ScheduleWork(TimerCompleteCallback onComplete, void * appState) { VerifyOrReturnError(mLayerState.IsInitialized(), CHIP_ERROR_INCORRECT_STATE); - Timer * timer = Timer::New(*this, System::Clock::kZero, onComplete, appState); + TimerList::Node * timer = mTimerPool.Create(*this, System::Clock::kZero, onComplete, appState); VerifyOrReturnError(timer != nullptr, CHIP_ERROR_NO_MEMORY); - return ScheduleLambda([timer] { timer->HandleComplete(); }); + return ScheduleLambda([this, timer] { this->mTimerPool.Invoke(timer); }); } /** * Start the platform timer with specified millsecond duration. - * - * @brief - * Calls the Platform specific API to start a platform timer. This API is called by the chip::System::Timer class when - * one or more timers are active and require deferred execution. - * - * @param[in] aDelay The timer duration in milliseconds. - * - * @return CHIP_NO_ERROR on success, error code otherwise. - * */ CHIP_ERROR LayerImplLwIP::StartPlatformTimer(System::Clock::Timeout aDelay) { @@ -142,14 +131,13 @@ CHIP_ERROR LayerImplLwIP::HandlePlatformTimer() // limit the number of timers handled before the control is returned to the event queue. The bound is similar to // (though not exactly same) as that on the sockets-based systems. - size_t timersHandled = 0; - Timer * timer = nullptr; + size_t timersHandled = 0; + TimerList::Node * timer = nullptr; while ((timersHandled < CHIP_SYSTEM_CONFIG_NUM_TIMERS) && ((timer = mTimerList.PopIfEarlier(expirationTime)) != nullptr)) { mHandlingTimerComplete = true; - timer->HandleComplete(); + mTimerPool.Invoke(timer); mHandlingTimerComplete = false; - timersHandled++; } @@ -160,10 +148,10 @@ CHIP_ERROR LayerImplLwIP::HandlePlatformTimer() Clock::Timestamp currentTime = SystemClock().GetMonotonicTimestamp(); - if (currentTime < mTimerList.Earliest()->mAwakenTime) + if (currentTime < mTimerList.Earliest()->AwakenTime()) { // the next timer expires in the future, so set the delay to a non-zero value - delay = mTimerList.Earliest()->mAwakenTime - currentTime; + delay = mTimerList.Earliest()->AwakenTime() - currentTime; } StartPlatformTimer(delay); diff --git a/src/system/SystemLayerImplLwIP.h b/src/system/SystemLayerImplLwIP.h index f2099b3aab7ed2..e3ee1d5f885182 100644 --- a/src/system/SystemLayerImplLwIP.h +++ b/src/system/SystemLayerImplLwIP.h @@ -24,6 +24,7 @@ #include #include +#include namespace chip { namespace System { @@ -51,7 +52,8 @@ class LayerImplLwIP : public LayerLwIP CHIP_ERROR StartPlatformTimer(System::Clock::Timeout aDelay); - Timer::MutexedList mTimerList; + TimerPool mTimerPool; + TimerList mTimerList; bool mHandlingTimerComplete; // true while handling any timer completion ObjectLifeCycle mLayerState; }; diff --git a/src/system/SystemLayerImplSelect.cpp b/src/system/SystemLayerImplSelect.cpp index 63b67cbdc5b30c..88d915ca7d1ae7 100644 --- a/src/system/SystemLayerImplSelect.cpp +++ b/src/system/SystemLayerImplSelect.cpp @@ -46,8 +46,6 @@ CHIP_ERROR LayerImplSelect::Init() RegisterPOSIXErrorFormatter(); - ReturnErrorOnFailure(mTimerList.Init()); - for (auto & w : mSocketWatchPool) { w.Clear(); @@ -68,21 +66,23 @@ CHIP_ERROR LayerImplSelect::Shutdown() { VerifyOrReturnError(mLayerState.SetShuttingDown(), CHIP_ERROR_INCORRECT_STATE); - Timer * timer; +#if CHIP_SYSTEM_CONFIG_USE_DISPATCH + TimerList::Node * timer; while ((timer = mTimerList.PopEarliest()) != nullptr) { - timer->Clear(); - -#if CHIP_SYSTEM_CONFIG_USE_DISPATCH if (timer->mTimerSource != nullptr) { dispatch_source_cancel(timer->mTimerSource); dispatch_release(timer->mTimerSource); } -#endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH - timer->Release(); + mTimerPool.Release(timer); } +#else // CHIP_SYSTEM_CONFIG_USE_DISPATCH + mTimerList.Clear(); + mTimerPool.ReleaseAll(); +#endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH + mWakeEvent.Close(*this); mLayerState.ResetFromShuttingDown(); // Return to uninitialized state to permit re-initialization. @@ -111,6 +111,7 @@ void LayerImplSelect::Signal() CHIP_ERROR status = mWakeEvent.Notify(); if (status != CHIP_NO_ERROR) { + ChipLogError(chipSystemLayer, "System wake event notify failed: %" CHIP_ERROR_FORMAT, status.Format()); } } @@ -123,7 +124,7 @@ CHIP_ERROR LayerImplSelect::StartTimer(Clock::Timeout delay, TimerCompleteCallba CancelTimer(onComplete, appState); - Timer * timer = Timer::New(*this, delay, onComplete, appState); + TimerList::Node * timer = mTimerPool.Create(*this, delay, onComplete, appState); VerifyOrReturnError(timer != nullptr, CHIP_ERROR_NO_MEMORY); #if CHIP_SYSTEM_CONFIG_USE_DISPATCH @@ -164,11 +165,9 @@ void LayerImplSelect::CancelTimer(TimerCompleteCallback onComplete, void * appSt { VerifyOrReturn(mLayerState.IsInitialized()); - Timer * timer = mTimerList.Remove(onComplete, appState); + TimerList::Node * timer = mTimerList.Remove(onComplete, appState); VerifyOrReturn(timer != nullptr); - timer->Clear(); - #if CHIP_SYSTEM_CONFIG_USE_DISPATCH if (timer->mTimerSource != nullptr) { @@ -177,7 +176,7 @@ void LayerImplSelect::CancelTimer(TimerCompleteCallback onComplete, void * appSt } #endif - timer->Release(); + mTimerPool.Release(timer); Signal(); } @@ -187,7 +186,7 @@ CHIP_ERROR LayerImplSelect::ScheduleWork(TimerCompleteCallback onComplete, void CancelTimer(onComplete, appState); - Timer * timer = Timer::New(*this, Clock::kZero, onComplete, appState); + TimerList::Node * timer = mTimerPool.Create(*this, Clock::kZero, onComplete, appState); VerifyOrReturnError(timer != nullptr, CHIP_ERROR_NO_MEMORY); #if CHIP_SYSTEM_CONFIG_USE_DISPATCH @@ -332,7 +331,7 @@ void LayerImplSelect::PrepareEvents() const Clock::Timestamp currentTime = SystemClock().GetMonotonicTimestamp(); Clock::Timestamp awakenTime = currentTime + kDefaultMinSleepPeriod; - Timer * timer = mTimerList.Earliest(); + TimerList::Node * timer = mTimerList.Earliest(); if (timer && timer->AwakenTime() < awakenTime) { awakenTime = timer->AwakenTime(); @@ -386,11 +385,11 @@ void LayerImplSelect::HandleEvents() // Obtain the list of currently expired timers. Any new timers added by timer callback are NOT handled on this pass, // since that could result in infinite handling of new timers blocking any other progress. - Timer::List expiredTimers(mTimerList.ExtractEarlier(Clock::Timeout(1) + SystemClock().GetMonotonicTimestamp())); - Timer * timer = nullptr; + TimerList expiredTimers = mTimerList.ExtractEarlier(Clock::Timeout(1) + SystemClock().GetMonotonicTimestamp()); + TimerList::Node * timer = nullptr; while ((timer = expiredTimers.PopEarliest()) != nullptr) { - timer->HandleComplete(); + mTimerPool.Invoke(timer); } for (auto & w : mSocketWatchPool) @@ -411,10 +410,10 @@ void LayerImplSelect::HandleEvents() } #if CHIP_SYSTEM_CONFIG_USE_DISPATCH -void LayerImplSelect::HandleTimerComplete(Timer * timer) +void LayerImplSelect::HandleTimerComplete(TimerList::Node * timer) { mTimerList.Remove(timer); - timer->HandleComplete(); + mTimerPool.Invoke(timer); } #endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH diff --git a/src/system/SystemLayerImplSelect.h b/src/system/SystemLayerImplSelect.h index d079d8c56b9f43..65d852cd7e4629 100644 --- a/src/system/SystemLayerImplSelect.h +++ b/src/system/SystemLayerImplSelect.h @@ -31,6 +31,7 @@ #include #include +#include #include namespace chip { @@ -71,7 +72,7 @@ class LayerImplSelect : public LayerSocketsLoop #if CHIP_SYSTEM_CONFIG_USE_DISPATCH void SetDispatchQueue(dispatch_queue_t dispatchQueue) override { mDispatchQueue = dispatchQueue; }; dispatch_queue_t GetDispatchQueue() override { return mDispatchQueue; }; - void HandleTimerComplete(Timer * timer); + void HandleTimerComplete(TimerList::Node * timer); #endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH protected: @@ -90,7 +91,8 @@ class LayerImplSelect : public LayerSocketsLoop }; SocketWatch mSocketWatchPool[kSocketWatchMax]; - Timer::MutexedList mTimerList; + TimerPool mTimerPool; + TimerList mTimerList; timeval mNextTimeout; // Members for select loop diff --git a/src/system/SystemTimer.cpp b/src/system/SystemTimer.cpp index 7e4a606099e5ea..5764228e844322 100644 --- a/src/system/SystemTimer.cpp +++ b/src/system/SystemTimer.cpp @@ -38,133 +38,21 @@ namespace chip { namespace System { -#if CHIP_SYSTEM_CONFIG_NUM_TIMERS - -/******************************************************************************* - * Timer state - * - * There are two fundamental state-change variables: Object::mSystemLayer and - * Timer::mOnComplete. These must be checked and changed atomically. The state - * of the timer is governed by the following state machine: - * - * INITIAL STATE: mSystemLayer == NULL, mOnComplete == NULL - * | - * V - * UNALLOCATED<-----------------------------+ - * | | - * (set mSystemLayer != NULL) | - * | | - * V | - * ALLOCATED-------(set mSystemLayer NULL)--+ - * | \-----------------------------+ - * | | - * (set mOnComplete != NULL) | - * | | - * V | - * ARMED ---------( clear mOnComplete )--+ - * - * When in the ARMED state: - * - * * None of the member variables may mutate. - * * mOnComplete must only be cleared by Cancel() or HandleComplete() - * * Cancel() and HandleComplete() will test that they are the one to - * successfully set mOnComplete NULL. And if so, that will be the - * thread that must call Object::Release(). - * - ******************************************************************************* - */ - -chip::ObjectPool Timer::sPool; -Stats::count_t Timer::mNumInUse = 0; -Stats::count_t Timer::mHighWatermark = 0; - -Timer * Timer::New(System::Layer & systemLayer, System::Clock::Timeout delay, TimerCompleteCallback onComplete, void * appState) +TimerList::Node * TimerList::Add(TimerList::Node * add) { - Timer * timer = Timer::sPool.CreateObject(); - if (timer == nullptr) + VerifyOrDie(add != mEarliestTimer); + if (mEarliestTimer == NULL || (add->AwakenTime() < mEarliestTimer->AwakenTime())) { - ChipLogError(chipSystemLayer, "Timer pool EMPTY"); + add->mNextTimer = mEarliestTimer; + mEarliestTimer = add; } else { - timer->mAppState = appState; - timer->mSystemLayer = &systemLayer; - timer->mAwakenTime = SystemClock().GetMonotonicTimestamp() + delay; - if (!__sync_bool_compare_and_swap(&timer->mOnComplete, nullptr, onComplete)) - { - chipDie(); - } -#if CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS - static_assert(CHIP_SYSTEM_CONFIG_NUM_TIMERS < CHIP_SYS_STATS_COUNT_MAX, "Stats count is too small"); - if (++mNumInUse > mHighWatermark) - { - mHighWatermark = mNumInUse; - } -#endif // CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS - } - return timer; -} - -void Timer::Release() -{ - Timer::sPool.ReleaseObject(this); -#if CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS - --mNumInUse; -#endif // CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS -} - -void Timer::Clear() -{ - TimerCompleteCallback lOnComplete = this->mOnComplete; - - // Check if the timer is armed - VerifyOrReturn(lOnComplete != nullptr); - - // Atomically disarm if the value has not changed - VerifyOrReturn(__sync_bool_compare_and_swap(&mOnComplete, lOnComplete, nullptr)); - - // Since this thread changed the state of mOnComplete, release the timer. - mAppState = nullptr; - mSystemLayer = nullptr; -} - -void Timer::HandleComplete() -{ - // Save information needed to perform the callback. - Layer * lLayer = this->mSystemLayer; - const TimerCompleteCallback lOnComplete = this->mOnComplete; - void * lAppState = this->mAppState; - - // Check if timer is armed - VerifyOrReturn(lOnComplete != nullptr, ); - // Atomically disarm if the value has not changed. - VerifyOrReturn(__sync_bool_compare_and_swap(&this->mOnComplete, lOnComplete, nullptr), ); - - // Since this thread changed the state of mOnComplete, release the timer. - mAppState = nullptr; - mSystemLayer = nullptr; - this->Release(); - - // Invoke the app's callback, if it's still valid. - if (lOnComplete != nullptr) - lOnComplete(lLayer, lAppState); -} - -Timer * Timer::List::Add(Timer * add) -{ - VerifyOrDie(add != mHead); - if (mHead == NULL || (add->mAwakenTime < mHead->mAwakenTime)) - { - add->mNextTimer = mHead; - mHead = add; - } - else - { - Timer * lTimer = mHead; + TimerList::Node * lTimer = mEarliestTimer; while (lTimer->mNextTimer) { VerifyOrDie(lTimer->mNextTimer != add); - if (add->mAwakenTime < lTimer->mNextTimer->mAwakenTime) + if (add->AwakenTime() < lTimer->mNextTimer->AwakenTime()) { // found the insert location. break; @@ -174,20 +62,20 @@ Timer * Timer::List::Add(Timer * add) add->mNextTimer = lTimer->mNextTimer; lTimer->mNextTimer = add; } - return mHead; + return mEarliestTimer; } -Timer * Timer::List::Remove(Timer * remove) +TimerList::Node * TimerList::Remove(TimerList::Node * remove) { - VerifyOrDie(mHead != nullptr); + VerifyOrDie(mEarliestTimer != nullptr); - if (remove == mHead) + if (remove == mEarliestTimer) { - mHead = remove->mNextTimer; + mEarliestTimer = remove->mNextTimer; } else { - Timer * lTimer = mHead; + TimerList::Node * lTimer = mEarliestTimer; while (lTimer->mNextTimer) { @@ -202,19 +90,19 @@ Timer * Timer::List::Remove(Timer * remove) } remove->mNextTimer = nullptr; - return mHead; + return mEarliestTimer; } -Timer * Timer::List::Remove(TimerCompleteCallback aOnComplete, void * aAppState) +TimerList::Node * TimerList::Remove(TimerCompleteCallback aOnComplete, void * aAppState) { - Timer * previous = nullptr; - for (Timer * timer = mHead; timer != nullptr; timer = timer->mNextTimer) + TimerList::Node * previous = nullptr; + for (TimerList::Node * timer = mEarliestTimer; timer != nullptr; timer = timer->mNextTimer) { - if (timer->mOnComplete == aOnComplete && timer->mAppState == aAppState) + if (timer->GetCallback().GetOnComplete() == aOnComplete && timer->GetCallback().GetAppState() == aAppState) { if (previous == nullptr) { - mHead = timer->mNextTimer; + mEarliestTimer = timer->mNextTimer; } else { @@ -228,58 +116,48 @@ Timer * Timer::List::Remove(TimerCompleteCallback aOnComplete, void * aAppState) return nullptr; } -Timer * Timer::List::PopEarliest() +TimerList::Node * TimerList::PopEarliest() { - if (mHead == nullptr) + if (mEarliestTimer == nullptr) { return nullptr; } - Timer * earliest = mHead; - mHead = mHead->mNextTimer; - earliest->mNextTimer = nullptr; + TimerList::Node * earliest = mEarliestTimer; + mEarliestTimer = mEarliestTimer->mNextTimer; + earliest->mNextTimer = nullptr; return earliest; } -Timer * Timer::List::PopIfEarlier(Clock::Timestamp t) +TimerList::Node * TimerList::PopIfEarlier(Clock::Timestamp t) { - if ((mHead == nullptr) || !(mHead->mAwakenTime < t)) + if ((mEarliestTimer == nullptr) || !(mEarliestTimer->AwakenTime() < t)) { return nullptr; } - Timer * earliest = mHead; - mHead = mHead->mNextTimer; - earliest->mNextTimer = nullptr; + TimerList::Node * earliest = mEarliestTimer; + mEarliestTimer = mEarliestTimer->mNextTimer; + earliest->mNextTimer = nullptr; return earliest; } -Timer * Timer::List::ExtractEarlier(Clock::Timestamp t) +TimerList TimerList::ExtractEarlier(Clock::Timestamp t) { - if ((mHead == nullptr) || !(mHead->mAwakenTime < t)) - { - return nullptr; - } - Timer * begin = mHead; - Timer * end = mHead; - while ((end->mNextTimer != nullptr) && (end->mNextTimer->mAwakenTime < t)) + TimerList out; + + if ((mEarliestTimer != nullptr) && (mEarliestTimer->AwakenTime() < t)) { - end = end->mNextTimer; + out.mEarliestTimer = mEarliestTimer; + TimerList::Node * end = mEarliestTimer; + while ((end->mNextTimer != nullptr) && (end->mNextTimer->AwakenTime() < t)) + { + end = end->mNextTimer; + } + mEarliestTimer = end->mNextTimer; + end->mNextTimer = nullptr; } - mHead = end->mNextTimer; - end->mNextTimer = nullptr; - return begin; -} -CHIP_ERROR Timer::MutexedList::Init() -{ - mHead = nullptr; -#if CHIP_SYSTEM_CONFIG_NO_LOCKING - return CHIP_NO_ERROR; -#else // CHIP_SYSTEM_CONFIG_NO_LOCKING - return Mutex::Init(mMutex); -#endif // CHIP_SYSTEM_CONFIG_NO_LOCKING + return out; } -#endif // CHIP_SYSTEM_CONFIG_NUM_TIMERS - } // namespace System } // namespace chip diff --git a/src/system/SystemTimer.h b/src/system/SystemTimer.h index 1fd3138873f7d1..59457d2c855c00 100644 --- a/src/system/SystemTimer.h +++ b/src/system/SystemTimer.h @@ -17,12 +17,9 @@ */ /** - * @file - * This file defines the chip::System::Timer class and its - * related types used for representing an in-progress one-shot - * timer. - * - * Some platforms use this to implement System::Layer timer events. + * This file defines the chip::System::Timer class and related types that can be used for representing + * an in-progress one-shot timer. Implementations of System::Layer may (but are not required to) use + * these for their versions of timer events. */ #pragma once @@ -36,222 +33,199 @@ #include #include -#include +#include #include #if CHIP_SYSTEM_CONFIG_USE_DISPATCH #include #endif -#if CHIP_SYSTEM_CONFIG_USE_TIMER_POOL -#include -#endif // CHIP_SYSTEM_CONFIG_USE_TIMER_POOL - namespace chip { namespace System { class Layer; -using TimerCompleteCallback = void (*)(Layer * aLayer, void * appState); - -#if CHIP_SYSTEM_CONFIG_USE_TIMER_POOL - /** - * This is an Object-pool based class that System::Layer implementations can use to assist in providing timer functions. + * Basic Timer information: time and callback. */ -class DLL_EXPORT Timer +class DLL_EXPORT TimerData { public: - /** - * List of timers ordered by completion time. - * - * @note - * This is an intrusive linked list, using the Timer field `mNextTimer`. - */ - class List + class Callback { public: - List() : mHead(nullptr) {} - List(Timer * head) : mHead(head) {} - - /** - * Add a timer to the list - * - * @return The new earliest timer in the list. If this is the newly added timer, that implies it is earlier - * than any existing timer. - */ - Timer * Add(Timer * add); - - /** - * Remove the given timer from the list, if present. It is not an error for the timer not to be present. - * - * @return The new earliest timer in the list, or nullptr if the list is empty. - */ - Timer * Remove(Timer * remove); - - /** - * Remove the first timer with the given properties, if present. It is not an error for no such timer to be present. - * - * @return The removed timer, or nullptr if the list contains no matching timer. - */ - Timer * Remove(TimerCompleteCallback onComplete, void * appState); - - /** - * Remove and return the earliest timer in the list. - * - * @return The earliest timer, or nullptr if the list is empty. - */ - Timer * PopEarliest(); - - /** - * Remove and return the earliest timer in the list, provided it expires earlier than the given time @a t. - * - * @return The earliest timer expiring before @a t, or nullptr if there is no such timer. - */ - Timer * PopIfEarlier(Clock::Timestamp t); - - /** - * Remove and return all timers that expire before the given time @a t. - * - * @return An ordered linked list (by `mNextTimer`) of all timers that expire before @a t, or nullptr if there are none. - */ - Timer * ExtractEarlier(Clock::Timestamp t); - - /** - * Get the earliest timer in the list. - */ - Timer * Earliest() const { return mHead; } - - protected: - Timer * mHead; - List(const List &) = delete; - List & operator=(const List &) = delete; + Callback(Layer & systemLayer, TimerCompleteCallback onComplete, void * appState) : + mSystemLayer(&systemLayer), mOnComplete(onComplete), mAppState(appState) + {} + void Invoke() { mOnComplete(mSystemLayer, mAppState); } + const TimerCompleteCallback & GetOnComplete() const { return mOnComplete; } + void * GetAppState() const { return mAppState; } + Layer * GetSystemLayer() const { return mSystemLayer; } + + private: + Layer * mSystemLayer; + TimerCompleteCallback mOnComplete; + void * mAppState; }; + + TimerData(Layer & systemLayer, System::Clock::Timeout delay, TimerCompleteCallback onComplete, void * appState) : + mAwakenTime(SystemClock().GetMonotonicTimestamp() + delay), mCallback(systemLayer, onComplete, appState) + {} + ~TimerData() = default; + /** - * List of timers ordered by completion time. - * - * This extends Timer::List to lock all access to the list. + * Return the expiration time. + */ + Clock::Timestamp AwakenTime() const { return mAwakenTime; } + + /** + * Return callback information. */ - class MutexedList : private List + const Callback & GetCallback() const { return mCallback; } + +private: + Clock::Timestamp mAwakenTime; + Callback mCallback; + +#if CHIP_SYSTEM_CONFIG_USE_DISPATCH + friend class LayerImplSelect; + dispatch_source_t mTimerSource = nullptr; +#endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH + + // Not defined + TimerData(const TimerData &) = delete; + TimerData & operator=(const TimerData &) = delete; +}; + +/** + * List of `Timer`s ordered by expiration time. + */ +class TimerList +{ +public: + class Node : public TimerData { public: - MutexedList() = default; - CHIP_ERROR Init(); - bool Empty() const - { - std::lock_guard lock(mMutex); - return mHead == nullptr; - } - Timer * Add(Timer * add) - { - std::lock_guard lock(mMutex); - return List::Add(add); - } - Timer * Remove(Timer * remove) - { - std::lock_guard lock(mMutex); - return List::Remove(remove); - } - Timer * Remove(TimerCompleteCallback onComplete, void * appState) - { - std::lock_guard lock(mMutex); - return List::Remove(onComplete, appState); - } - Timer * PopEarliest() - { - std::lock_guard lock(mMutex); - return List::PopEarliest(); - } - Timer * PopIfEarlier(Clock::Timestamp t) - { - std::lock_guard lock(mMutex); - return List::PopIfEarlier(t); - } - Timer * ExtractEarlier(Clock::Timestamp t) - { - std::lock_guard lock(mMutex); - return List::ExtractEarlier(t); - } - Timer * Earliest() const - { - std::lock_guard lock(mMutex); - return mHead; - } - - private: - mutable Mutex mMutex; - MutexedList(const MutexedList &) = delete; - MutexedList & operator=(const MutexedList &) = delete; + Node(Layer & systemLayer, System::Clock::Timeout delay, TimerCompleteCallback onComplete, void * appState) : + TimerData(systemLayer, delay, onComplete, appState), mNextTimer(nullptr) + {} + Node * mNextTimer; }; - Timer() = default; + TimerList() : mEarliestTimer(nullptr) {} /** - * Obtain a new timer from the object pool. + * Add a timer to the list + * + * @return The new earliest timer in the list. If this is the newly added timer, that implies it is earlier + * than any existing timer. */ - static Timer * New(System::Layer & systemLayer, System::Clock::Timeout delay, TimerCompleteCallback onComplete, - void * appState); + Node * Add(Node * timer); /** - * Return a timer to the object pool. + * Remove the given timer from the list, if present. It is not an error for the timer not to be present. + * + * @return The new earliest timer in the list, or nullptr if the list is empty. */ - void Release(); + Node * Remove(Node * remove); /** - * Return the expiration time. + * Remove the first timer with the given properties, if present. It is not an error for no such timer to be present. + * + * @return The removed timer, or nullptr if the list contains no matching timer. */ - Clock::Timestamp AwakenTime() const { return mAwakenTime; } + Node * Remove(TimerCompleteCallback onComplete, void * appState); /** - * Fire the timer. + * Remove and return the earliest timer in the list. * - * This method is called by the underlying timer mechanism provided by the platform when the timer fires. - * It invalidates this timer object, calls Object::Release() on it, and invokes the callback. + * @return The earliest timer, or nullptr if the list is empty. */ - void HandleComplete(); + Node * PopEarliest(); /** - * Invalidate the timer fields. This is intended for timer cancellation, and typically this will be followed by - * an object Release(). + * Remove and return the earliest timer in the list, provided it expires earlier than the given time @a t. * - * @note - * The Timer owner is responsible for ensuring this timer is not in use, e.g. in a List or by a platform timer implementation. + * @return The earliest timer expiring before @a t, or nullptr if there is no such timer. */ - void Clear(); + Node * PopIfEarlier(Clock::Timestamp t); /** - * Read timer pool statistics. + * Get the earliest timer in the list. + * + * @return The earliest timer, or nullptr if there are no timers. */ - static void GetStatistics(Stats::count_t & aNumInUse, Stats::count_t & aHighWatermark) - { - aNumInUse = mNumInUse; - aHighWatermark = mHighWatermark; - } + Node * Earliest() const { return mEarliestTimer; } + + /** + * Test whether there are any timers. + */ + bool Empty() const { return mEarliestTimer != nullptr; } + + /** + * Remove and return all timers that expire before the given time @a t. + */ + TimerList ExtractEarlier(Clock::Timestamp t); + + /** + * Remove all timers. + */ + void Clear() { mEarliestTimer = nullptr; } private: - friend class LayerImplLwIP; - static chip::ObjectPool sPool; - static Stats::count_t mNumInUse; - static Stats::count_t mHighWatermark; + Node * mEarliestTimer; +}; - TimerCompleteCallback mOnComplete; - Clock::Timestamp mAwakenTime; - Timer * mNextTimer; +/** + * ObjectPool wrapper that keeps System Timer statistics. + */ +template +class TimerPool +{ +public: + using Timer = T; - Layer * mSystemLayer; - void * mAppState; + /** + * Create a new timer from the pool. + */ + Timer * Create(Layer & systemLayer, System::Clock::Timeout delay, TimerCompleteCallback onComplete, void * appState) + { + Timer * timer = mTimerPool.CreateObject(systemLayer, delay, onComplete, appState); + SYSTEM_STATS_INCREMENT(Stats::kSystemLayer_NumTimers); + return timer; + } -#if CHIP_SYSTEM_CONFIG_USE_DISPATCH - friend class LayerImplSelect; - dispatch_source_t mTimerSource = nullptr; -#endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH + /** + * Release a timer to the pool. + */ + void Release(Timer * timer) + { + SYSTEM_STATS_DECREMENT(Stats::kSystemLayer_NumTimers); + mTimerPool.ReleaseObject(timer); + } - // Not defined - Timer(const Timer &) = delete; - Timer & operator=(const Timer &) = delete; -}; + /** + * Release all timers. + */ + void ReleaseAll() + { + SYSTEM_STATS_RESET(Stats::kSystemLayer_NumTimers); + mTimerPool.ReleaseAll(); + } + + /** + * Release a timer to the pool and invoke its callback. + */ + void Invoke(Timer * timer) + { + typename Timer::Callback callback = timer->GetCallback(); + Release(timer); + callback.Invoke(); + } -#endif // CHIP_SYSTEM_CONFIG_USE_TIMER_POOL +private: + ObjectPool mTimerPool; +}; } // namespace System } // namespace chip