diff --git a/source/tvision/ttimerqu.cpp b/source/tvision/ttimerqu.cpp index 6fd18e19..75314381 100644 --- a/source/tvision/ttimerqu.cpp +++ b/source/tvision/ttimerqu.cpp @@ -15,10 +15,10 @@ static TTimePoint systemTimeMs() struct TTimer { - void *collectId; TTimePoint expiresAt; int32_t period; TTimer *next; + void *collectId; }; TTimerQueue::TTimerQueue() noexcept : @@ -87,12 +87,16 @@ void TTimerQueue::collectExpiredTimers(void (&func)(TTimerId, void *), void *arg if (first == 0) return; + // Given that the timer list may be mutated while we process expired timers, + // we iterate it from the beginning every time. In order to know which + // timers have already been processed, we mark them with a 'collectId' + // which identifies the current invocation of 'collectExpiredTimers'. void *collectId = &collectId; TTimePoint now = getTimeMs(); while (True) { TTimer **p = &first; - while (*p != 0 && ((*p)->collectId == collectId || now < (*p)->expiresAt)) + while (*p != 0 && ((*p)->collectId != 0 || now < (*p)->expiresAt)) p = &(*p)->next; if (*p == 0) break; @@ -128,7 +132,7 @@ int32_t TTimerQueue::timeUntilNextTimeout() if (first == 0) return -1; TTimePoint now = getTimeMs(); - uint32_t maxValue = (uint32_t) -1 >> 1; + uint32_t maxValue = uint32_t(-1) >> 1; int32_t timeout = maxValue; TTimer *timer = first; do diff --git a/test/tvision/ttimerqu.test.cpp b/test/tvision/ttimerqu.test.cpp index f4a3bf15..2360efee 100644 --- a/test/tvision/ttimerqu.test.cpp +++ b/test/tvision/ttimerqu.test.cpp @@ -204,18 +204,18 @@ static void nestedHandleTimeout(TTimerId, void *args) (*(TTimerQueue *) args).collectExpiredTimers(handleTimeout, nullptr); } -TEST(TTimerQueue, ShouldCollectTimeoutsWithNestedInvocation) +TEST(TTimerQueue, ShouldNotCollectTimeoutsWithNestedInvocation) { currentTime = 0; TTimerQueue timerQueue(mockCurrentTime); - timerQueue.setTimer(1500); + timerQueue.setTimer(0, 0); timerQueue.setTimer(1000, 500); currentTime = 1000; timeouts = 0; timerQueue.collectExpiredTimers(nestedHandleTimeout, &timerQueue); - EXPECT_EQ(timeouts, 3); + EXPECT_EQ(timeouts, 2); } TEST(TTimerQueue, ShouldNotRequestCurrentTimeIfThereAreNoTimers)