From c99b4fb9218c8f3fafc4a04b782a4b5234ac2a38 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Wed, 21 Jun 2017 00:14:33 -0700 Subject: [PATCH] Timer: Make Timer implementation robust against late firing platform alarm (#1917) This commit changes the timer code to make the implementation robust against late firing platform alarm case. In particular, it addresses the (rare corner-case) scenario where alarm fire is late and the head timer in the linked-list is already expired and then a new timer is started with maximum interval. This can possibly violate the requirement for `TimerScheduler::IsStringlyBefore()` method that the two times being compared should not differ more than the maximum interval `Timer::kMaxDt`. To address this, a new method `Timer::DoesFireBefore()` is added to compare fire time of two timers which checks for expired timers. This commit also updates the timer unit test to add test cases related to the late firing alarm. --- src/core/common/timer.cpp | 27 +++++- src/core/common/timer.hpp | 11 +++ tests/unit/test_timer.cpp | 167 ++++++++++++++++++++++++++++++++++++ tests/unit/test_windows.cpp | 2 + 4 files changed, 206 insertions(+), 1 deletion(-) diff --git a/src/core/common/timer.cpp b/src/core/common/timer.cpp index 2e927d5cd..7a452d076 100644 --- a/src/core/common/timer.cpp +++ b/src/core/common/timer.cpp @@ -69,7 +69,7 @@ void TimerScheduler::Add(Timer &aTimer) for (cur = mHead; cur; cur = cur->mNext) { - if (IsStrictlyBefore(aTimer.mFireTime, cur->mFireTime)) + if (aTimer.DoesFireBefore(*cur)) { if (prev) { @@ -185,4 +185,29 @@ bool TimerScheduler::IsStrictlyBefore(uint32_t aTimeA, uint32_t aTimeB) return ((diff & (1UL << 31)) != 0); } +bool Timer::DoesFireBefore(const Timer &aSecondTimer) +{ + bool retval; + uint32_t now = GetNow(); + bool isBeforeNow = TimerScheduler::IsStrictlyBefore(GetFireTime(), now); + + // Check if one timer is before `now` and the other one is not. + if (TimerScheduler::IsStrictlyBefore(aSecondTimer.GetFireTime(), now) != isBeforeNow) + { + // One timer is before `now` and the other one is not, so if this timer's fire time is before `now` then + // the second fire time would be after `now` and this timer would fire before the second timer. + + retval = isBeforeNow; + } + else + { + // Both timers are before `now` or both are after `now`. Either way the difference is guaranteed to be less + // than `kMaxDt` so we can safely compare the fire times directly. + + retval = TimerScheduler::IsStrictlyBefore(GetFireTime(), aSecondTimer.GetFireTime()); + } + + return retval; +} + } // namespace ot diff --git a/src/core/common/timer.hpp b/src/core/common/timer.hpp index e1e1f6dbf..18c3db12e 100644 --- a/src/core/common/timer.hpp +++ b/src/core/common/timer.hpp @@ -250,6 +250,17 @@ public: static uint32_t MsecToHours(uint32_t aMilliseconds) { return MsecToSec(aMilliseconds / 3600u); } private: + /** + * This method indicates if the fire time of this timer is strictly before the fire time of a second given timer. + * + * @param[in] aTimer A reference to the second timer object. + * + * @retval TRUE If the fire time of this timer object is strictly before aTimer's fire time + * @retval FALSE If the fire time of this timer object is the same or after aTimer's fire time. + * + */ + bool DoesFireBefore(const Timer &aTimer); + void Fired(void) { mHandler(mContext); } TimerScheduler &mScheduler; diff --git a/tests/unit/test_timer.cpp b/tests/unit/test_timer.cpp index f31114b3f..3f45c3517 100644 --- a/tests/unit/test_timer.cpp +++ b/tests/unit/test_timer.cpp @@ -212,6 +212,172 @@ int TestOneTimer(void) return 0; } +/** + * Test the TimerScheduler's behavior of two timers started and fired. + */ +int TestTwoTimers(void) +{ + const uint32_t kTimeT0 = 1000; + const uint32_t kTimerInterval = 10; + otInstance aInstance; + uint32_t timerContextHandleCounter[2] = {0}; + ot::Timer timer1(aInstance.mIp6.mTimerScheduler, TestTimerHandler, &timerContextHandleCounter[0]); + ot::Timer timer2(aInstance.mIp6.mTimerScheduler, TestTimerHandler, &timerContextHandleCounter[1]); + + InitTestTimer(); + printf("TestTwoTimers() "); + + // Test when second timer stars at the fire time of first timer (before alarm callback). + + InitCounters(); + memset(timerContextHandleCounter, 0, sizeof(timerContextHandleCounter)); + + sNow = kTimeT0; + timer1.Start(kTimerInterval); + + VerifyOrQuit(sCallCount[kCallCountIndexAlarmStart] == 1, "TestTwoTimers: Start CallCount Failed.\n"); + VerifyOrQuit(sCallCount[kCallCountIndexAlarmStop] == 0, "TestTwoTimers: Stop CallCount Failed.\n"); + VerifyOrQuit(sCallCount[kCallCountIndexTimerHandler] == 0, "TestTwoTimers: Handler CallCount Failed.\n"); + VerifyOrQuit(sPlatT0 == kTimeT0 && sPlatDt == kTimerInterval, "TestTwoTimers: Start params Failed.\n"); + VerifyOrQuit(timer1.IsRunning(), "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(timer2.IsRunning() == false, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(sTimerOn, "TestTwoTimers: Platform Timer State Failed.\n"); + + sNow += kTimerInterval; + + timer2.Start(kTimerInterval); + + VerifyOrQuit(sCallCount[kCallCountIndexAlarmStart] == 1, "TestTwoTimers: Start CallCount Failed.\n"); + VerifyOrQuit(sCallCount[kCallCountIndexAlarmStop] == 0, "TestTwoTimers: Stop CallCount Failed.\n"); + VerifyOrQuit(sCallCount[kCallCountIndexTimerHandler] == 0, "TestTwoTimers: Handler CallCount Failed.\n"); + VerifyOrQuit(sPlatT0 == kTimeT0 && sPlatDt == kTimerInterval, "TestTwoTimers: Start params Failed.\n"); + VerifyOrQuit(timer1.IsRunning() == true, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(timer2.IsRunning() == true, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(sTimerOn, "TestTwoTimers: Platform Timer State Failed.\n"); + + otPlatAlarmFired(&aInstance); + + VerifyOrQuit(sCallCount[kCallCountIndexAlarmStart] == 2, "TestTwoTimers: Start CallCount Failed.\n"); + VerifyOrQuit(sCallCount[kCallCountIndexAlarmStop] == 0, "TestTwoTimers: Stop CallCount Failed.\n"); + VerifyOrQuit(sCallCount[kCallCountIndexTimerHandler] == 1, "TestTwoTimers: Handler CallCount Failed.\n"); + VerifyOrQuit(timerContextHandleCounter[0] == 1, "TestTwoTimers: Context handler failed.\n"); + VerifyOrQuit(sPlatT0 == sNow && sPlatDt == kTimerInterval, "TestTwoTimers: Start params Failed.\n"); + VerifyOrQuit(timer1.IsRunning() == false, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(timer2.IsRunning() == true, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(sTimerOn == true, "TestTwoTimers: Platform Timer State Failed.\n"); + + sNow += kTimerInterval; + otPlatAlarmFired(&aInstance); + + VerifyOrQuit(sCallCount[kCallCountIndexAlarmStart] == 2, "TestTwoTimers: Start CallCount Failed.\n"); + VerifyOrQuit(sCallCount[kCallCountIndexAlarmStop] == 1, "TestTwoTimers: Stop CallCount Failed.\n"); + VerifyOrQuit(sCallCount[kCallCountIndexTimerHandler] == 2, "TestTwoTimers: Handler CallCount Failed.\n"); + VerifyOrQuit(timerContextHandleCounter[1] == 1, "TestTwoTimers: Context handler failed.\n"); + VerifyOrQuit(timer1.IsRunning() == false, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(timer2.IsRunning() == false, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(sTimerOn == false, "TestTwoTimers: Platform Timer State Failed.\n"); + + // Test when second timer starts at the fire time of first timer (before otPlatAlarmFired()) and its fire time + // is before the first timer. Ensure that the second timer handler is invoked before the first one. + + InitCounters(); + memset(timerContextHandleCounter, 0, sizeof(timerContextHandleCounter)); + + sNow = kTimeT0; + timer1.Start(kTimerInterval); + + VerifyOrQuit(sCallCount[kCallCountIndexAlarmStart] == 1, "TestTwoTimers: Start CallCount Failed.\n"); + VerifyOrQuit(sCallCount[kCallCountIndexAlarmStop] == 0, "TestTwoTimers: Stop CallCount Failed.\n"); + VerifyOrQuit(sCallCount[kCallCountIndexTimerHandler] == 0, "TestTwoTimers: Handler CallCount Failed.\n"); + VerifyOrQuit(sPlatT0 == kTimeT0 && sPlatDt == kTimerInterval, "TestTwoTimers: Start params Failed.\n"); + VerifyOrQuit(timer1.IsRunning(), "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(timer2.IsRunning() == false, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(sTimerOn, "TestTwoTimers: Platform Timer State Failed.\n"); + + sNow += kTimerInterval; + + timer2.StartAt(kTimeT0, kTimerInterval - 2); // Timer 2 is even before timer 1 + + VerifyOrQuit(sCallCount[kCallCountIndexTimerHandler] == 0, "TestTwoTimers: Handler CallCount Failed.\n"); + VerifyOrQuit(timer1.IsRunning() == true, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(timer2.IsRunning() == true, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(sTimerOn, "TestTwoTimers: Platform Timer State Failed.\n"); + + otPlatAlarmFired(&aInstance); + + VerifyOrQuit(sCallCount[kCallCountIndexAlarmStop] == 0, "TestTwoTimers: Stop CallCount Failed.\n"); + VerifyOrQuit(sCallCount[kCallCountIndexTimerHandler] == 1, "TestTwoTimers: Handler CallCount Failed.\n"); + VerifyOrQuit(timerContextHandleCounter[1] == 1, "TestTwoTimers: Context handler failed.\n"); + VerifyOrQuit(sPlatT0 == sNow && sPlatDt == 0, "TestTwoTimers: Start params Failed.\n"); + VerifyOrQuit(timer1.IsRunning() == true, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(timer2.IsRunning() == false, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(sTimerOn == true, "TestTwoTimers: Platform Timer State Failed.\n"); + + otPlatAlarmFired(&aInstance); + + VerifyOrQuit(sCallCount[kCallCountIndexAlarmStop] == 1, "TestTwoTimers: Stop CallCount Failed.\n"); + VerifyOrQuit(sCallCount[kCallCountIndexTimerHandler] == 2, "TestTwoTimers: Handler CallCount Failed.\n"); + VerifyOrQuit(timerContextHandleCounter[0] == 1, "TestTwoTimers: Context handler failed.\n"); + VerifyOrQuit(timer1.IsRunning() == false, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(timer2.IsRunning() == false, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(sTimerOn == false, "TestTwoTimers: Platform Timer State Failed.\n"); + + // Timer 1 fire callback is late by some ticks/ms, and second timer is scheduled (before call to otPlatAlarmFired) + // with a maximum interval. This is to test (corner-case) scenario where the fire time of two timers spanning over + // the maximum interval. + + InitCounters(); + memset(timerContextHandleCounter, 0, sizeof(timerContextHandleCounter)); + + sNow = kTimeT0; + timer1.Start(kTimerInterval); + + VerifyOrQuit(sCallCount[kCallCountIndexAlarmStart] == 1, "TestTwoTimers: Start CallCount Failed.\n"); + VerifyOrQuit(sCallCount[kCallCountIndexAlarmStop] == 0, "TestTwoTimers: Stop CallCount Failed.\n"); + VerifyOrQuit(sCallCount[kCallCountIndexTimerHandler] == 0, "TestTwoTimers: Handler CallCount Failed.\n"); + VerifyOrQuit(sPlatT0 == kTimeT0 && sPlatDt == kTimerInterval, "TestTwoTimers: Start params Failed.\n"); + VerifyOrQuit(timer1.IsRunning(), "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(timer2.IsRunning() == false, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(sTimerOn, "TestTwoTimers: Platform Timer State Failed.\n"); + + sNow += kTimerInterval + 5; + + timer2.Start(ot::Timer::kMaxDt); + + VerifyOrQuit(sCallCount[kCallCountIndexAlarmStart] == 1, "TestTwoTimers: Start CallCount Failed.\n"); + VerifyOrQuit(sCallCount[kCallCountIndexAlarmStop] == 0, "TestTwoTimers: Stop CallCount Failed.\n"); + VerifyOrQuit(sCallCount[kCallCountIndexTimerHandler] == 0, "TestTwoTimers: Handler CallCount Failed.\n"); + VerifyOrQuit(timer1.IsRunning() == true, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(timer2.IsRunning() == true, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(sTimerOn, "TestTwoTimers: Platform Timer State Failed.\n"); + + otPlatAlarmFired(&aInstance); + + VerifyOrQuit(sCallCount[kCallCountIndexAlarmStart] == 2, "TestTwoTimers: Start CallCount Failed.\n"); + VerifyOrQuit(sCallCount[kCallCountIndexAlarmStop] == 0, "TestTwoTimers: Stop CallCount Failed.\n"); + VerifyOrQuit(sCallCount[kCallCountIndexTimerHandler] == 1, "TestTwoTimers: Handler CallCount Failed.\n"); + VerifyOrQuit(timerContextHandleCounter[0] == 1, "TestTwoTimers: Context handler failed.\n"); + VerifyOrQuit(sPlatT0 == sNow, "TestTwoTimers: Start params Failed.\n"); + VerifyOrQuit(sPlatDt == ot::Timer::kMaxDt, "TestTwoTimers: Start params Failed.\n"); + VerifyOrQuit(timer1.IsRunning() == false, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(timer2.IsRunning() == true, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(sTimerOn == true, "TestTwoTimers: Platform Timer State Failed.\n"); + + sNow += ot::Timer::kMaxDt; + otPlatAlarmFired(&aInstance); + + VerifyOrQuit(sCallCount[kCallCountIndexAlarmStart] == 2, "TestTwoTimers: Start CallCount Failed.\n"); + VerifyOrQuit(sCallCount[kCallCountIndexAlarmStop] == 1, "TestTwoTimers: Stop CallCount Failed.\n"); + VerifyOrQuit(sCallCount[kCallCountIndexTimerHandler] == 2, "TestTwoTimers: Handler CallCount Failed.\n"); + VerifyOrQuit(timerContextHandleCounter[1] == 1, "TestTwoTimers: Context handler failed.\n"); + VerifyOrQuit(timer1.IsRunning() == false, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(timer2.IsRunning() == false, "TestTwoTimers: Timer running Failed.\n"); + VerifyOrQuit(sTimerOn == false, "TestTwoTimers: Platform Timer State Failed.\n"); + + printf(" --> PASSED\n"); + + return 0; +} /** * Test the TimerScheduler's behavior of ten timers started and fired. @@ -460,6 +626,7 @@ int TestTenTimers(void) void RunTimerTests(void) { TestOneTimer(); + TestTwoTimers(); TestTenTimers(); } diff --git a/tests/unit/test_windows.cpp b/tests/unit/test_windows.cpp index 84dc65a13..8bc151199 100644 --- a/tests/unit/test_windows.cpp +++ b/tests/unit/test_windows.cpp @@ -79,6 +79,7 @@ namespace ot // test_timer.cpp int TestOneTimer(); +int TestTwoTimers(); int TestTenTimers(); // test_toolchain.cpp @@ -159,6 +160,7 @@ namespace ot // test_timer.cpp TEST_METHOD(TestOneTimer) { ::TestOneTimer(); } + TEST_METHOD(TestTwoTimers) { ::TestTwoTimers(); } TEST_METHOD(TestTenTimers) { ::TestTenTimers(); } // test_ncp_buffer.cpp