From 9c999a7a90a4efb6d25250d4de120e9f4e5803aa Mon Sep 17 00:00:00 2001 From: Kevin Schoedel Date: Thu, 4 Nov 2021 10:35:57 -0400 Subject: [PATCH] Convert TimeSource to safer Clock types #### Problem `System::TimeSource` still uses raw integer times rather than the safer `Syste::Clock` types based on `std::chrono::duration`. Followup to #10062 _Some operations on System::Clock types are not safe_ #### Change overview Convert `TimeSource` and its uses. #### Testing CI; no changes to functionality. Converted `TestTimeSource`. --- src/app/server/Dnssd.cpp | 21 ++++++++------- src/app/server/Dnssd.h | 15 ++++++----- src/lib/dnssd/Discovery_ImplPlatform.cpp | 3 ++- src/lib/dnssd/DnssdCache.h | 25 ++++++++--------- src/lib/dnssd/tests/TestDnssdCache.cpp | 10 +++---- .../UDCClientState.h | 12 ++++----- .../user_directed_commissioning/UDCClients.h | 16 +++++------ .../tests/TestUdcMessages.cpp | 8 +++--- src/system/TimeSource.h | 14 +++++----- src/system/tests/TestTimeSource.cpp | 11 ++++---- src/transport/SecureSession.h | 16 ++++++----- src/transport/SecureSessionTable.h | 10 +++---- src/transport/UnauthenticatedSessionTable.h | 18 ++++++------- src/transport/tests/TestPeerConnections.cpp | 27 ++++++++++--------- 14 files changed, 107 insertions(+), 99 deletions(-) diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp index 044bb4860ddd46..7c20d03e5fc4d9 100644 --- a/src/app/server/Dnssd.cpp +++ b/src/app/server/Dnssd.cpp @@ -76,6 +76,8 @@ void OnPlatformEventWrapper(const DeviceLayer::ChipDeviceEvent * event, intptr_t } // namespace +constexpr System::Clock::Timestamp DnssdServer::kTimeoutCleared; + #if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY constexpr const char kExtendedDiscoveryTimeoutKeypairStorage[] = "ExtDiscKey"; @@ -110,7 +112,7 @@ void HandleExtendedDiscoveryExpiration(System::Layer * aSystemLayer, void * aApp void DnssdServer::OnExtendedDiscoveryExpiration(System::Layer * aSystemLayer, void * aAppState) { - if (!DnssdServer::OnExpiration(mExtendedDiscoveryExpirationMs)) + if (!DnssdServer::OnExpiration(mExtendedDiscoveryExpiration)) { ChipLogDetail(Discovery, "OnExtendedDiscoveryExpiration callback for cleared session"); return; @@ -118,7 +120,7 @@ void DnssdServer::OnExtendedDiscoveryExpiration(System::Layer * aSystemLayer, vo ChipLogDetail(Discovery, "OnExtendedDiscoveryExpiration callback for valid session"); - mExtendedDiscoveryExpirationMs = TIMEOUT_CLEARED; + mExtendedDiscoveryExpiration = kTimeoutCleared; } #endif // CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY @@ -128,14 +130,14 @@ void HandleDiscoveryExpiration(System::Layer * aSystemLayer, void * aAppState) DnssdServer::Instance().OnDiscoveryExpiration(aSystemLayer, aAppState); } -bool DnssdServer::OnExpiration(uint64_t expirationMs) +bool DnssdServer::OnExpiration(System::Clock::Timestamp expirationMs) { - if (expirationMs == TIMEOUT_CLEARED) + if (expirationMs == kTimeoutCleared) { ChipLogDetail(Discovery, "OnExpiration callback for cleared session"); return false; } - uint64_t now = mTimeSource.GetCurrentMonotonicTimeMs(); + System::Clock::Timestamp now = mTimeSource.GetMonotonicTimestamp(); if (expirationMs > now) { ChipLogDetail(Discovery, "OnExpiration callback for reset session"); @@ -183,7 +185,7 @@ bool DnssdServer::OnExpiration(uint64_t expirationMs) void DnssdServer::OnDiscoveryExpiration(System::Layer * aSystemLayer, void * aAppState) { - if (!DnssdServer::OnExpiration(mDiscoveryExpirationMs)) + if (!DnssdServer::OnExpiration(mDiscoveryExpiration)) { ChipLogDetail(Discovery, "OnDiscoveryExpiration callback for cleared session"); return; @@ -205,7 +207,7 @@ void DnssdServer::OnDiscoveryExpiration(System::Layer * aSystemLayer, void * aAp } #endif // CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY - mDiscoveryExpirationMs = TIMEOUT_CLEARED; + mDiscoveryExpiration = kTimeoutCleared; } CHIP_ERROR DnssdServer::ScheduleDiscoveryExpiration() @@ -216,7 +218,7 @@ CHIP_ERROR DnssdServer::ScheduleDiscoveryExpiration() } ChipLogDetail(Discovery, "Scheduling Discovery timeout in secs=%d", mDiscoveryTimeoutSecs); - mDiscoveryExpirationMs = mTimeSource.GetCurrentMonotonicTimeMs() + static_cast(mDiscoveryTimeoutSecs) * 1000; + mDiscoveryExpiration = mTimeSource.GetMonotonicTimestamp() + System::Clock::Seconds16(mDiscoveryTimeoutSecs); return DeviceLayer::SystemLayer().StartTimer(System::Clock::Seconds16(mDiscoveryTimeoutSecs), HandleDiscoveryExpiration, nullptr); @@ -232,8 +234,7 @@ CHIP_ERROR DnssdServer::ScheduleExtendedDiscoveryExpiration() } ChipLogDetail(Discovery, "Scheduling Extended Discovery timeout in secs=%d", extendedDiscoveryTimeoutSecs); - mExtendedDiscoveryExpirationMs = - mTimeSource.GetCurrentMonotonicTimeMs() + static_cast(extendedDiscoveryTimeoutSecs) * 1000; + mExtendedDiscoveryExpiration = mTimeSource.GetMonotonicTimestamp() + System::Clock::Seconds16(extendedDiscoveryTimeoutSecs); return DeviceLayer::SystemLayer().StartTimer(System::Clock::Seconds16(extendedDiscoveryTimeoutSecs), HandleExtendedDiscoveryExpiration, nullptr); diff --git a/src/app/server/Dnssd.h b/src/app/server/Dnssd.h index 7c6d95696bc77b..7fce0ecb8255e3 100644 --- a/src/app/server/Dnssd.h +++ b/src/app/server/Dnssd.h @@ -26,10 +26,11 @@ namespace chip { namespace app { -#define TIMEOUT_CLEARED 0 class DLL_EXPORT DnssdServer { public: + static constexpr System::Clock::Timestamp kTimeoutCleared = System::Clock::kZero; + /// Provides the system-wide implementation of the service advertiser static DnssdServer & Instance() { @@ -104,9 +105,9 @@ class DLL_EXPORT DnssdServer void ClearTimeouts() { - mDiscoveryExpirationMs = TIMEOUT_CLEARED; + mDiscoveryExpiration = kTimeoutCleared; #if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY - mExtendedDiscoveryExpirationMs = TIMEOUT_CLEARED; + mExtendedDiscoveryExpiration = kTimeoutCleared; #endif // CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY } @@ -115,11 +116,11 @@ class DLL_EXPORT DnssdServer /// schedule next discovery expiration CHIP_ERROR ScheduleDiscoveryExpiration(); - int16_t mDiscoveryTimeoutSecs = CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS; - uint64_t mDiscoveryExpirationMs = TIMEOUT_CLEARED; + int16_t mDiscoveryTimeoutSecs = CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS; + System::Clock::Timestamp mDiscoveryExpiration = kTimeoutCleared; /// return true if expirationMs is valid (not cleared and not in the future) - bool OnExpiration(uint64_t expirationMs); + bool OnExpiration(System::Clock::Timestamp expiration); #if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY /// get the current extended discovery timeout (from persistent storage) @@ -128,7 +129,7 @@ class DLL_EXPORT DnssdServer /// schedule next extended discovery expiration CHIP_ERROR ScheduleExtendedDiscoveryExpiration(); - uint64_t mExtendedDiscoveryExpirationMs = TIMEOUT_CLEARED; + System::Clock::Timestamp mExtendedDiscoveryExpiration = kTimeoutCleared; #endif // CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY }; diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index 8bcc96e066ad47..012412d587fb93 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -589,7 +589,8 @@ void DiscoveryImplPlatform::HandleNodeIdResolve(void * context, DnssdService * r #if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 // TODO -- define appropriate TTL, for now use 2000 msec (rfc default) // figure out way to use TTL value from mDNS packet in future update - error = mgr->sDnssdCache.Insert(nodeData.mPeerId, result->mAddress.Value(), result->mPort, result->mInterface, 2 * 1000); + error = mgr->sDnssdCache.Insert(nodeData.mPeerId, result->mAddress.Value(), result->mPort, result->mInterface, + System::Clock::Seconds16(2)); if (CHIP_NO_ERROR != error) { diff --git a/src/lib/dnssd/DnssdCache.h b/src/lib/dnssd/DnssdCache.h index 279108b82c11c2..99990d6cc76b26 100644 --- a/src/lib/dnssd/DnssdCache.h +++ b/src/lib/dnssd/DnssdCache.h @@ -55,9 +55,10 @@ class DnssdCache // return error if cache is full // TODO: have an eviction policy so if the cache is full, an entry may be deleted. // One policy may be Least-time-to-live - CHIP_ERROR Insert(PeerId peerId, const Inet::IPAddress & addr, uint16_t port, Inet::InterfaceId iface, uint32_t TTLms) + CHIP_ERROR Insert(PeerId peerId, const Inet::IPAddress & addr, uint16_t port, Inet::InterfaceId iface, + System::Clock::Timestamp TTL) { - const uint64_t currentTime = mTimeSource.GetCurrentMonotonicTimeMs(); + const System::Clock::Timestamp currentTime = mTimeSource.GetMonotonicTimestamp(); DnssdCacheEntry * entry; @@ -65,8 +66,8 @@ class DnssdCache if (entry) { // update timeout if found entry - entry->expiryTime = currentTime + TTLms; - entry->TTL = TTLms; // in case it changes */ + entry->expiryTime = currentTime + TTL; + entry->TTL = TTL; // in case it changes */ return CHIP_NO_ERROR; } @@ -77,8 +78,8 @@ class DnssdCache entry->ipAddr = addr; entry->port = port; entry->ifaceId = iface; - entry->TTL = TTLms; - entry->expiryTime = currentTime + TTLms; + entry->TTL = TTL; + entry->expiryTime = currentTime + TTL; elementsUsed++; return CHIP_NO_ERROR; @@ -87,7 +88,7 @@ class DnssdCache CHIP_ERROR Delete(PeerId peerId) { DnssdCacheEntry * pentry; - const uint64_t currentTime = mTimeSource.GetCurrentMonotonicTimeMs(); + const System::Clock::Timestamp currentTime = mTimeSource.GetMonotonicTimestamp(); VerifyOrReturnError(pentry = FindPeerId(peerId, currentTime), CHIP_ERROR_KEY_NOT_FOUND); @@ -99,7 +100,7 @@ class DnssdCache CHIP_ERROR Lookup(PeerId peerId, Inet::IPAddress & addr, uint16_t & port, Inet::InterfaceId & iface) { DnssdCacheEntry * pentry; - const uint64_t currentTime = mTimeSource.GetCurrentMonotonicTimeMs(); + const System::Clock::Timestamp currentTime = mTimeSource.GetMonotonicTimestamp(); VerifyOrReturnError(pentry = FindPeerId(peerId, currentTime), CHIP_ERROR_KEY_NOT_FOUND); @@ -141,8 +142,8 @@ class DnssdCache Inet::IPAddress ipAddr; uint16_t port; Inet::InterfaceId ifaceId; - uint64_t TTL; // from mdns record -- units? - uint64_t expiryTime; // units? + System::Clock::Timestamp TTL; + System::Clock::Timestamp expiryTime; }; PeerId nullPeerId; // indicates a cache entry is unused int elementsUsed; // running count of how many entries are used -- for a sanity check @@ -150,7 +151,7 @@ class DnssdCache DnssdCacheEntry mLookupTable[CACHE_SIZE]; Time::TimeSource mTimeSource; - DnssdCacheEntry * findSlot(uint64_t currentTime) + DnssdCacheEntry * findSlot(System::Clock::Timestamp currentTime) { for (DnssdCacheEntry & entry : mLookupTable) { @@ -166,7 +167,7 @@ class DnssdCache return nullptr; } - DnssdCacheEntry * FindPeerId(PeerId peerId, uint64_t current_time) + DnssdCacheEntry * FindPeerId(PeerId peerId, System::Clock::Timestamp current_time) { for (DnssdCacheEntry & entry : mLookupTable) { diff --git a/src/lib/dnssd/tests/TestDnssdCache.cpp b/src/lib/dnssd/tests/TestDnssdCache.cpp index aaddc91d767c12..38c50795a407ac 100644 --- a/src/lib/dnssd/tests/TestDnssdCache.cpp +++ b/src/lib/dnssd/tests/TestDnssdCache.cpp @@ -55,7 +55,7 @@ void TestInsert(nlTestSuite * inSuite, void * inContext) Inet::IPAddress addr; Inet::IPAddress addrV6; - const int ttl = 2; /* seconds */ + const System::Clock::Timestamp ttl = System::Clock::Seconds16(2); Inet::InterfaceId iface_out; Inet::IPAddress addr_out; uint16_t port_out; @@ -71,7 +71,7 @@ void TestInsert(nlTestSuite * inSuite, void * inContext) // ml -- why doesn't adding 2 uint16_t give a uint16_t? peerId.SetNodeId((NodeId) id + i); - result = tDnssdCache.Insert(peerId, addr, (uint16_t)(port + i), iface, 1000 * ttl); + result = tDnssdCache.Insert(peerId, addr, (uint16_t)(port + i), iface, ttl); if (i < sizeOfCache) { NL_TEST_ASSERT(inSuite, result == CHIP_NO_ERROR); @@ -83,7 +83,7 @@ void TestInsert(nlTestSuite * inSuite, void * inContext) } tDnssdCache.DumpCache(); - usleep(static_cast((ttl + 1) * 1000 * 1000)); + usleep(static_cast(std::chrono::microseconds(ttl + System::Clock::Seconds16(1)).count())); id = 0x200; port = 3000; for (uint16_t i = 0; i < sizeOfCache; i++) @@ -91,7 +91,7 @@ void TestInsert(nlTestSuite * inSuite, void * inContext) CHIP_ERROR result; peerId.SetNodeId((NodeId) id + i); - result = tDnssdCache.Insert(peerId, addr, (uint16_t)(port + i), iface, 1000 * ttl); + result = tDnssdCache.Insert(peerId, addr, (uint16_t)(port + i), iface, ttl); NL_TEST_ASSERT(inSuite, result == CHIP_NO_ERROR); } tDnssdCache.DumpCache(); @@ -115,7 +115,7 @@ void TestInsert(nlTestSuite * inSuite, void * inContext) CHIP_ERROR result; peerId.SetNodeId((NodeId) id + i); - result = tDnssdCache.Insert(peerId, addrV6, (uint16_t)(port + i), iface, 1000 * ttl); + result = tDnssdCache.Insert(peerId, addrV6, (uint16_t)(port + i), iface, ttl); NL_TEST_ASSERT(inSuite, result == CHIP_NO_ERROR); } diff --git a/src/protocols/user_directed_commissioning/UDCClientState.h b/src/protocols/user_directed_commissioning/UDCClientState.h index e3565022c23f27..088121a67afdd1 100644 --- a/src/protocols/user_directed_commissioning/UDCClientState.h +++ b/src/protocols/user_directed_commissioning/UDCClientState.h @@ -78,13 +78,13 @@ class UDCClientState UDCClientProcessingState GetUDCClientProcessingState() const { return mUDCClientProcessingState; } void SetUDCClientProcessingState(UDCClientProcessingState state) { mUDCClientProcessingState = state; } - uint64_t GetExpirationTimeMs() const { return mExpirationTimeMs; } - void SetExpirationTimeMs(uint64_t value) { mExpirationTimeMs = value; } + System::Clock::Timestamp GetExpirationTime() const { return mExpirationTime; } + void SetExpirationTime(System::Clock::Timestamp value) { mExpirationTime = value; } - bool IsInitialized(uint64_t currentTime) + bool IsInitialized(System::Clock::Timestamp currentTime) { // if state is not the "not-initialized" and it has not expired - return (mUDCClientProcessingState != UDCClientProcessingState::kNotInitialized && mExpirationTimeMs > currentTime); + return (mUDCClientProcessingState != UDCClientProcessingState::kNotInitialized && mExpirationTime > currentTime); } /** @@ -93,7 +93,7 @@ class UDCClientState void Reset() { mPeerAddress = PeerAddress::Uninitialized(); - mExpirationTimeMs = 0; + mExpirationTime = System::Clock::kZero; mUDCClientProcessingState = UDCClientProcessingState::kNotInitialized; } @@ -103,7 +103,7 @@ class UDCClientState char mDeviceName[Dnssd::kMaxDeviceNameLen + 1]; uint16_t mLongDiscriminator = 0; UDCClientProcessingState mUDCClientProcessingState; - uint64_t mExpirationTimeMs = 0; + System::Clock::Timestamp mExpirationTime = System::Clock::kZero; }; } // namespace UserDirectedCommissioning diff --git a/src/protocols/user_directed_commissioning/UDCClients.h b/src/protocols/user_directed_commissioning/UDCClients.h index a91f5afcf161a4..bf119fe0588ebd 100644 --- a/src/protocols/user_directed_commissioning/UDCClients.h +++ b/src/protocols/user_directed_commissioning/UDCClients.h @@ -26,7 +26,7 @@ namespace Protocols { namespace UserDirectedCommissioning { // UDC client state times out after 1 hour. This may need to be tweaked. -constexpr const uint64_t kUDCClientTimeoutMs = 60 * 60 * 1000; +constexpr const System::Clock::Timestamp kUDCClientTimeout = System::Clock::Milliseconds64(60 * 60 * 1000); /** * Handles a set of UDC Client Processing States. @@ -54,7 +54,7 @@ class UDCClients CHECK_RETURN_VALUE CHIP_ERROR CreateNewUDCClientState(const char * instanceName, UDCClientState ** state) { - const uint64_t currentTime = mTimeSource.GetCurrentMonotonicTimeMs(); + const System::Clock::Timestamp currentTime = mTimeSource.GetMonotonicTimestamp(); CHIP_ERROR err = CHIP_ERROR_NO_MEMORY; @@ -68,7 +68,7 @@ class UDCClients if (!stateiter.IsInitialized(currentTime)) { stateiter.SetInstanceName(instanceName); - stateiter.SetExpirationTimeMs(currentTime + kUDCClientTimeoutMs); + stateiter.SetExpirationTime(currentTime + kUDCClientTimeout); stateiter.SetUDCClientProcessingState(UDCClientProcessingState::kDiscoveringNode); if (state) @@ -99,8 +99,8 @@ class UDCClients return nullptr; } - const uint64_t currentTime = mTimeSource.GetCurrentMonotonicTimeMs(); - UDCClientState state = mStates[index]; + const System::Clock::Timestamp currentTime = mTimeSource.GetMonotonicTimestamp(); + UDCClientState state = mStates[index]; if (!state.IsInitialized(currentTime)) { return nullptr; @@ -118,7 +118,7 @@ class UDCClients CHECK_RETURN_VALUE UDCClientState * FindUDCClientState(const PeerAddress & address) { - const uint64_t currentTime = mTimeSource.GetCurrentMonotonicTimeMs(); + const System::Clock::Timestamp currentTime = mTimeSource.GetMonotonicTimestamp(); UDCClientState * state = nullptr; @@ -147,7 +147,7 @@ class UDCClients CHECK_RETURN_VALUE UDCClientState * FindUDCClientState(const char * instanceName) { - const uint64_t currentTime = mTimeSource.GetCurrentMonotonicTimeMs(); + const System::Clock::Timestamp currentTime = mTimeSource.GetMonotonicTimestamp(); UDCClientState * state = nullptr; @@ -180,7 +180,7 @@ class UDCClients /// Convenience method to mark a UDC Client state as active (non-expired) void MarkUDCClientActive(UDCClientState * state) { - state->SetExpirationTimeMs(mTimeSource.GetCurrentMonotonicTimeMs() + kUDCClientTimeoutMs); + state->SetExpirationTime(mTimeSource.GetMonotonicTimestamp() + kUDCClientTimeout); } private: diff --git a/src/protocols/user_directed_commissioning/tests/TestUdcMessages.cpp b/src/protocols/user_directed_commissioning/tests/TestUdcMessages.cpp index 87b7d01106f221..d9a270cefa7c80 100644 --- a/src/protocols/user_directed_commissioning/tests/TestUdcMessages.cpp +++ b/src/protocols/user_directed_commissioning/tests/TestUdcMessages.cpp @@ -256,11 +256,11 @@ void TestUDCClients(nlTestSuite * inSuite, void * inContext) // test re-activation NL_TEST_ASSERT(inSuite, CHIP_NO_ERROR == mUdcClients.CreateNewUDCClientState(instanceName4, &state)); - uint64_t expirationTime = state->GetExpirationTimeMs(); - state->SetExpirationTimeMs(expirationTime - 1); - NL_TEST_ASSERT(inSuite, (expirationTime - 1) == state->GetExpirationTimeMs()); + System::Clock::Timestamp expirationTime = state->GetExpirationTime(); + state->SetExpirationTime(expirationTime - System::Clock::Milliseconds64(1)); + NL_TEST_ASSERT(inSuite, (expirationTime - System::Clock::Milliseconds64(1)) == state->GetExpirationTime()); mUdcClients.MarkUDCClientActive(state); - NL_TEST_ASSERT(inSuite, (expirationTime - 1) < state->GetExpirationTimeMs()); + NL_TEST_ASSERT(inSuite, (expirationTime - System::Clock::Milliseconds64(1)) < state->GetExpirationTime()); } // Test Suite diff --git a/src/system/TimeSource.h b/src/system/TimeSource.h index 672e1af4af1c1a..9fe5c383f76524 100644 --- a/src/system/TimeSource.h +++ b/src/system/TimeSource.h @@ -53,7 +53,7 @@ class TimeSource * such that the values do not entail a restart upon wake. * - This function is expected to be thread-safe on any platform that employs threading. */ - uint64_t GetCurrentMonotonicTimeMs(); + System::Clock::Timestamp GetMonotonicTimestamp(); }; /** @@ -63,7 +63,7 @@ template <> class TimeSource { public: - uint64_t GetCurrentMonotonicTimeMs() { return System::SystemClock().GetMonotonicMilliseconds64().count(); } + System::Clock::Timestamp GetMonotonicTimestamp() { return System::SystemClock().GetMonotonicTimestamp(); } }; /** @@ -73,19 +73,19 @@ template <> class TimeSource { public: - uint64_t GetCurrentMonotonicTimeMs() { return mCurrentTimeMs; } + System::Clock::Timestamp GetMonotonicTimestamp() { return mCurrentTime; } - void SetCurrentMonotonicTimeMs(uint64_t value) + void SetMonotonicTimestamp(System::Clock::Timestamp value) { - if (value < mCurrentTimeMs) + if (value < mCurrentTime) { abort(); } - mCurrentTimeMs = value; + mCurrentTime = value; } private: - uint64_t mCurrentTimeMs = 0; + System::Clock::Timestamp mCurrentTime = System::Clock::kZero; }; } // namespace Time diff --git a/src/system/tests/TestTimeSource.cpp b/src/system/tests/TestTimeSource.cpp index 8abf8edbccd394..89ce5b6bad57e4 100644 --- a/src/system/tests/TestTimeSource.cpp +++ b/src/system/tests/TestTimeSource.cpp @@ -40,10 +40,11 @@ void TestTimeSourceSetAndGet(nlTestSuite * inSuite, void * inContext) chip::Time::TimeSource source; - NL_TEST_ASSERT(inSuite, source.GetCurrentMonotonicTimeMs() == 0); + NL_TEST_ASSERT(inSuite, source.GetMonotonicTimestamp() == chip::System::Clock::kZero); - source.SetCurrentMonotonicTimeMs(1234); - NL_TEST_ASSERT(inSuite, source.GetCurrentMonotonicTimeMs() == 1234); + constexpr chip::System::Clock::Milliseconds64 k1234 = chip::System::Clock::Milliseconds64(1234); + source.SetMonotonicTimestamp(k1234); + NL_TEST_ASSERT(inSuite, source.GetMonotonicTimestamp() == k1234); } void SystemTimeSourceGet(nlTestSuite * inSuite, void * inContext) @@ -51,13 +52,13 @@ void SystemTimeSourceGet(nlTestSuite * inSuite, void * inContext) chip::Time::TimeSource source; - uint64_t oldValue = source.GetCurrentMonotonicTimeMs(); + chip::System::Clock::Timestamp oldValue = source.GetMonotonicTimestamp(); // a basic monotonic test. This is likely to take less than 1ms, so the // actual test value lies mostly in ensuring things compile. for (int i = 0; i < 100; i++) { - uint64_t newValue = source.GetCurrentMonotonicTimeMs(); + chip::System::Clock::Timestamp newValue = source.GetMonotonicTimestamp(); NL_TEST_ASSERT(inSuite, newValue >= oldValue); oldValue = newValue; } diff --git a/src/transport/SecureSession.h b/src/transport/SecureSession.h index 323c010f3fe57c..a3f098ac31d8d8 100644 --- a/src/transport/SecureSession.h +++ b/src/transport/SecureSession.h @@ -40,7 +40,7 @@ static constexpr uint32_t kUndefinedMessageIndex = UINT32_MAX; * - PeerAddress represents how to talk to the peer * - PeerNodeId is the unique ID of the peer * - SendMessageIndex is an ever increasing index for sending messages - * - LastActivityTimeMs is a monotonic timestamp of when this connection was + * - LastActivityTime is a monotonic timestamp of when this connection was * last used. Inactive connections can expire. * - CryptoContext contains the encryption context of a connection * @@ -49,10 +49,12 @@ static constexpr uint32_t kUndefinedMessageIndex = UINT32_MAX; class SecureSession { public: - SecureSession(uint16_t localSessionId, NodeId peerNodeId, uint16_t peerSessionId, FabricIndex fabric, uint64_t currentTime) : - mPeerNodeId(peerNodeId), mLocalSessionId(localSessionId), mPeerSessionId(peerSessionId), mFabric(fabric) + SecureSession(uint16_t localSessionId, NodeId peerNodeId, uint16_t peerSessionId, FabricIndex fabric, + System::Clock::Timestamp currentTime) : + mPeerNodeId(peerNodeId), + mLocalSessionId(localSessionId), mPeerSessionId(peerSessionId), mFabric(fabric) { - SetLastActivityTimeMs(currentTime); + SetLastActivityTime(currentTime); } SecureSession(SecureSession &&) = delete; @@ -69,8 +71,8 @@ class SecureSession uint16_t GetPeerSessionId() const { return mPeerSessionId; } FabricIndex GetFabricIndex() const { return mFabric; } - uint64_t GetLastActivityTimeMs() const { return mLastActivityTimeMs; } - void SetLastActivityTimeMs(uint64_t value) { mLastActivityTimeMs = value; } + System::Clock::Timestamp GetLastActivityTime() const { return mLastActivityTime; } + void SetLastActivityTime(System::Clock::Timestamp value) { mLastActivityTime = value; } CryptoContext & GetCryptoContext() { return mCryptoContext; } @@ -95,7 +97,7 @@ class SecureSession const FabricIndex mFabric; PeerAddress mPeerAddress; - uint64_t mLastActivityTimeMs = 0; + System::Clock::Timestamp mLastActivityTime = System::Clock::kZero; CryptoContext mCryptoContext; SessionMessageCounter mSessionMessageCounter; }; diff --git a/src/transport/SecureSessionTable.h b/src/transport/SecureSessionTable.h index 01fa082055892f..d2745aa013aa2e 100644 --- a/src/transport/SecureSessionTable.h +++ b/src/transport/SecureSessionTable.h @@ -57,7 +57,7 @@ class SecureSessionTable CHECK_RETURN_VALUE SecureSession * CreateNewSecureSession(uint16_t localSessionId, NodeId peerNodeId, uint16_t peerSessionId, FabricIndex fabric) { - return mEntries.CreateObject(localSessionId, peerNodeId, peerSessionId, fabric, mTimeSource.GetCurrentMonotonicTimeMs()); + return mEntries.CreateObject(localSessionId, peerNodeId, peerSessionId, fabric, mTimeSource.GetMonotonicTimestamp()); } void ReleaseSession(SecureSession * session) { mEntries.ReleaseObject(session); } @@ -91,7 +91,7 @@ class SecureSessionTable } /// Convenience method to mark a session as active - void MarkSessionActive(SecureSession * state) { state->SetLastActivityTimeMs(mTimeSource.GetCurrentMonotonicTimeMs()); } + void MarkSessionActive(SecureSession * state) { state->SetLastActivityTime(mTimeSource.GetMonotonicTimestamp()); } /** * Iterates through all active sessions and expires any sessions with an idle time @@ -100,11 +100,11 @@ class SecureSessionTable * Expiring a session involves callback execution and then clearing the internal state. */ template - void ExpireInactiveSessions(uint64_t maxIdleTimeMs, Callback callback) + void ExpireInactiveSessions(System::Clock::Timestamp maxIdleTime, Callback callback) { - const uint64_t currentTime = mTimeSource.GetCurrentMonotonicTimeMs(); + const System::Clock::Timestamp currentTime = mTimeSource.GetMonotonicTimestamp(); mEntries.ForEachActiveObject([&](auto session) { - if (session->GetLastActivityTimeMs() + maxIdleTimeMs < currentTime) + if (session->GetLastActivityTime() + maxIdleTime < currentTime) { callback(*session); ReleaseSession(session); diff --git a/src/transport/UnauthenticatedSessionTable.h b/src/transport/UnauthenticatedSessionTable.h index 381a7a4a0d64bc..a4a3cdfa50098a 100644 --- a/src/transport/UnauthenticatedSessionTable.h +++ b/src/transport/UnauthenticatedSessionTable.h @@ -54,15 +54,15 @@ class UnauthenticatedSession : public ReferenceCountedSetLastActivityTimeMs(mTimeSource.GetCurrentMonotonicTimeMs()); + session->SetLastActivityTime(mTimeSource.GetMonotonicTimestamp()); } /// Allows access to the underlying time source used for keeping track of session active time @@ -158,14 +158,14 @@ class UnauthenticatedSessionTable UnauthenticatedSession * FindLeastRecentUsedEntry() { - UnauthenticatedSession * result = nullptr; - uint64_t oldestTimeMs = std::numeric_limits::max(); + UnauthenticatedSession * result = nullptr; + System::Clock::Timestamp oldestTime = System::Clock::Timestamp(std::numeric_limits::max()); mEntries.ForEachActiveObject([&](UnauthenticatedSession * entry) { - if (entry->GetReferenceCount() == 0 && entry->GetLastActivityTimeMs() < oldestTimeMs) + if (entry->GetReferenceCount() == 0 && entry->GetLastActivityTime() < oldestTime) { - result = entry; - oldestTimeMs = entry->GetLastActivityTimeMs(); + result = entry; + oldestTime = entry->GetLastActivityTime(); } return true; }); diff --git a/src/transport/tests/TestPeerConnections.cpp b/src/transport/tests/TestPeerConnections.cpp index 128f7fc4e9d7b7..f9a4f6ec2f0195 100644 --- a/src/transport/tests/TestPeerConnections.cpp +++ b/src/transport/tests/TestPeerConnections.cpp @@ -33,6 +33,7 @@ namespace { using namespace chip; using namespace chip::Transport; +using namespace chip::System::Clock::Literals; PeerAddress AddressFromString(const char * str) { @@ -55,7 +56,7 @@ void TestBasicFunctionality(nlTestSuite * inSuite, void * inContext) { SecureSession * statePtr; SecureSessionTable<2, Time::Source::kTest> connections; - connections.GetTimeSource().SetCurrentMonotonicTimeMs(100); + connections.GetTimeSource().SetMonotonicTimestamp(100_ms64); // Node ID 1, peer key 1, local key 2 statePtr = connections.CreateNewSecureSession(2, kPeer1NodeId, 1, 0 /* fabricIndex */); @@ -65,7 +66,7 @@ void TestBasicFunctionality(nlTestSuite * inSuite, void * inContext) statePtr = connections.CreateNewSecureSession(4, kPeer2NodeId, 3, 0 /* fabricIndex */); NL_TEST_ASSERT(inSuite, statePtr != nullptr); NL_TEST_ASSERT(inSuite, statePtr->GetPeerNodeId() == kPeer2NodeId); - NL_TEST_ASSERT(inSuite, statePtr->GetLastActivityTimeMs() == 100); + NL_TEST_ASSERT(inSuite, statePtr->GetLastActivityTime() == 100_ms64); // Insufficient space for new connections. Object is max size 2 statePtr = connections.CreateNewSecureSession(6, kPeer3NodeId, 5, 0 /* fabricIndex */); @@ -105,26 +106,26 @@ void TestExpireConnections(nlTestSuite * inSuite, void * inContext) SecureSession * statePtr; SecureSessionTable<2, Time::Source::kTest> connections; - connections.GetTimeSource().SetCurrentMonotonicTimeMs(100); + connections.GetTimeSource().SetMonotonicTimestamp(100_ms64); // Node ID 1, peer key 1, local key 2 statePtr = connections.CreateNewSecureSession(2, kPeer1NodeId, 1, 0 /* fabricIndex */); NL_TEST_ASSERT(inSuite, statePtr != nullptr); statePtr->SetPeerAddress(kPeer1Addr); - connections.GetTimeSource().SetCurrentMonotonicTimeMs(200); + connections.GetTimeSource().SetMonotonicTimestamp(200_ms64); // Node ID 2, peer key 3, local key 4 statePtr = connections.CreateNewSecureSession(4, kPeer2NodeId, 3, 0 /* fabricIndex */); NL_TEST_ASSERT(inSuite, statePtr != nullptr); statePtr->SetPeerAddress(kPeer2Addr); // cannot add before expiry - connections.GetTimeSource().SetCurrentMonotonicTimeMs(300); + connections.GetTimeSource().SetMonotonicTimestamp(300_ms64); statePtr = connections.CreateNewSecureSession(6, kPeer3NodeId, 5, 0 /* fabricIndex */); NL_TEST_ASSERT(inSuite, statePtr == nullptr); // at time 300, this expires ip addr 1 - connections.ExpireInactiveSessions(150, [&callInfo](const SecureSession & state) { + connections.ExpireInactiveSessions(150_ms64, [&callInfo](const SecureSession & state) { callInfo.callCount++; callInfo.lastCallNodeId = state.GetPeerNodeId(); callInfo.lastCallPeerAddress = state.GetPeerAddress(); @@ -135,25 +136,25 @@ void TestExpireConnections(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, !connections.FindSecureSessionByLocalKey(2)); // now that the connections were expired, we can add peer3 - connections.GetTimeSource().SetCurrentMonotonicTimeMs(300); + connections.GetTimeSource().SetMonotonicTimestamp(300_ms64); // Node ID 3, peer key 5, local key 6 statePtr = connections.CreateNewSecureSession(6, kPeer3NodeId, 5, 0 /* fabricIndex */); NL_TEST_ASSERT(inSuite, statePtr != nullptr); statePtr->SetPeerAddress(kPeer3Addr); - connections.GetTimeSource().SetCurrentMonotonicTimeMs(400); + connections.GetTimeSource().SetMonotonicTimestamp(400_ms64); NL_TEST_ASSERT(inSuite, statePtr = connections.FindSecureSessionByLocalKey(4)); connections.MarkSessionActive(statePtr); - NL_TEST_ASSERT(inSuite, statePtr->GetLastActivityTimeMs() == connections.GetTimeSource().GetCurrentMonotonicTimeMs()); + NL_TEST_ASSERT(inSuite, statePtr->GetLastActivityTime() == connections.GetTimeSource().GetMonotonicTimestamp()); // At this time: // Peer 3 active at time 300 // Peer 2 active at time 400 - connections.GetTimeSource().SetCurrentMonotonicTimeMs(500); + connections.GetTimeSource().SetMonotonicTimestamp(500_ms64); callInfo.callCount = 0; - connections.ExpireInactiveSessions(150, [&callInfo](const SecureSession & state) { + connections.ExpireInactiveSessions(150_ms64, [&callInfo](const SecureSession & state) { callInfo.callCount++; callInfo.lastCallNodeId = state.GetPeerNodeId(); callInfo.lastCallPeerAddress = state.GetPeerAddress(); @@ -175,9 +176,9 @@ void TestExpireConnections(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, !connections.FindSecureSessionByLocalKey(6)); // peer 1 and 2 are active - connections.GetTimeSource().SetCurrentMonotonicTimeMs(1000); + connections.GetTimeSource().SetMonotonicTimestamp(1000_ms64); callInfo.callCount = 0; - connections.ExpireInactiveSessions(100, [&callInfo](const SecureSession & state) { + connections.ExpireInactiveSessions(100_ms64, [&callInfo](const SecureSession & state) { callInfo.callCount++; callInfo.lastCallNodeId = state.GetPeerNodeId(); callInfo.lastCallPeerAddress = state.GetPeerAddress();