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

@@ -22,8 +22,9 @@
  *      working with Certification Declaration elements.
  */
 
-#include <inttypes.h>
-#include <stddef.h>
+#include <algorithm>
+#include <cinttypes>
+#include <cstddef>
 
 #include <credentials/CertificationDeclaration.h>
 #include <crypto/CHIPCryptoPAL.h>

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;
 }

src/inet/InetLayer.h

@@ -23,6 +23,7 @@
 #pragma once
 
 #include <lib/support/ObjectLifeCycle.h>
+#include <lib/support/Pool.h>
 #include <platform/LockTracker.h>
 #include <system/SystemLayer.h>
 #include <system/SystemStats.h>

src/inet/tests/TestInetEndPoint.cpp

@@ -348,35 +348,34 @@ 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++)
     {
         err = gSystemLayer.StartTimer(10_ms32, HandleTimer, nullptr);
         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
 

src/lib/support/TestPersistentStorageDelegate.h

@@ -23,6 +23,7 @@
 #include <lib/core/CHIPPersistentStorageDelegate.h>
 #include <lib/support/DLLUtil.h>
 #include <map>
+#include <string>
 #include <vector>
 
 namespace chip {

src/lib/support/Variant.h

@@ -22,6 +22,7 @@
 
 #include <algorithm>
 #include <new>
+#include <tuple>
 #include <type_traits>
 #include <typeinfo>
 #include <utility>

src/messaging/ExchangeMessageDispatch.cpp

@@ -28,6 +28,7 @@
 #define __STDC_LIMIT_MACROS
 #endif
 
+#include <errno.h>
 #include <inttypes.h>
 #include <memory>
 

src/platform/Linux/ConnectivityManagerImpl.h

@@ -41,6 +41,8 @@
 #include <platform/Linux/dbus/wpa/DBusWpaBss.h>
 #include <platform/Linux/dbus/wpa/DBusWpaInterface.h>
 #include <platform/Linux/dbus/wpa/DBusWpaNetwork.h>
+
+#include <mutex>
 #endif
 
 namespace chip {

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
  *

src/system/SystemLayer.h

@@ -37,7 +37,6 @@
 #include <system/SystemClock.h>
 #include <system/SystemError.h>
 #include <system/SystemEvent.h>
-#include <system/SystemTimer.h>
 
 #if CHIP_SYSTEM_CONFIG_USE_SOCKETS
 #include <system/SocketEvents.h>
@@ -47,6 +46,8 @@
 #include <dispatch/dispatch.h>
 #endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH
 
+#include <utility>
+
 namespace chip {
 namespace System {
 

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);

src/system/SystemLayerImplLwIP.h

@@ -24,6 +24,7 @@
 
 #include <lib/support/ObjectLifeCycle.h>
 #include <system/SystemLayer.h>
+#include <system/SystemTimer.h>
 
 namespace chip {
 namespace System {
@@ -51,7 +52,8 @@ class LayerImplLwIP : public LayerLwIP
 
     CHIP_ERROR StartPlatformTimer(System::Clock::Timeout aDelay);
 
-    Timer::MutexedList mTimerList;
+    TimerPool<TimerList::Node> mTimerPool;
+    TimerList mTimerList;
     bool mHandlingTimerComplete; // true while handling any timer completion
     ObjectLifeCycle mLayerState;
 };

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