From 2b572f5376455ec37360b98c1042d3ed06112343 Mon Sep 17 00:00:00 2001
From: Francisco <francisco.paisana@softwareradiosystems.com>
Date: Tue, 20 Apr 2021 17:04:10 +0100
Subject: [PATCH] fix overwrite of timer timeout when it matches previous time
 wheel position

---
 lib/include/srsran/common/timers.h |  6 +++-
 lib/test/common/timer_test.cc      | 50 +++++++++++++++++++++++-------
 2 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/lib/include/srsran/common/timers.h b/lib/include/srsran/common/timers.h
index de4ce96eb..a0fb6fc32 100644
--- a/lib/include/srsran/common/timers.h
+++ b/lib/include/srsran/common/timers.h
@@ -265,6 +265,9 @@ public:
     timer.run();
   }
 
+  // useful for testing
+  static size_t get_wheel_size() { return WHEEL_SIZE; }
+
 private:
   timer_impl& alloc_timer()
   {
@@ -306,7 +309,8 @@ private:
     uint32_t timeout       = cur_time + timer.duration;
     size_t   new_wheel_pos = timeout & WHEEL_MASK;
     if (timer.is_running() and (timer.timeout & WHEEL_MASK) == new_wheel_pos) {
-      // If no change in timer wheel position
+      // If no change in timer wheel position. Just update absolute timeout
+      timer.timeout = timeout;
       return;
     }
 
diff --git a/lib/test/common/timer_test.cc b/lib/test/common/timer_test.cc
index 72a4c4605..f5820078a 100644
--- a/lib/test/common/timer_test.cc
+++ b/lib/test/common/timer_test.cc
@@ -25,6 +25,7 @@ int timers_test1()
   uint32_t      dur = 5;
 
   {
+    // TEST: default ctor places unique_timer in correct state
     timer_handler::unique_timer t = timers.get_unique_timer();
     TESTASSERT(not t.is_running() and not t.is_expired());
     TESTASSERT(t.id() == 0);
@@ -103,7 +104,6 @@ int timers_test1()
     timers.step_all();
     TESTASSERT(t.is_expired() and t2.is_expired() and t3.is_expired());
     TESTASSERT(first_id == 1);
-    printf("Last timer id=%d\n", last_id);
     TESTASSERT(last_id == 2);
   }
   // TEST: timer dtor is called and removes "timer" from "timers"
@@ -112,13 +112,13 @@ int timers_test1()
   return SRSRAN_SUCCESS;
 }
 
+/**
+ * Description:
+ * - calling stop() early, forbids the timer from getting expired
+ * - calling stop() after timer has expired should be a noop
+ */
 int timers_test2()
 {
-  /**
-   * Description:
-   * - calling stop() early, forbids the timer from getting expired
-   * - calling stop() after timer has expired should be a noop
-   */
   timer_handler timers;
   uint32_t      duration = 2;
 
@@ -147,12 +147,12 @@ int timers_test2()
   return SRSRAN_SUCCESS;
 }
 
+/**
+ * Description:
+ * - setting a new duration while the timer is already running should not stop timer, and should extend timeout
+ */
 int timers_test3()
 {
-  /**
-   * Description:
-   * - setting a new duration while the timer is already running should not stop timer, and should extend timeout
-   */
   timer_handler timers;
   uint32_t      duration = 5;
 
@@ -173,7 +173,7 @@ int timers_test3()
     TESTASSERT(utimer.is_running());
   }
   timers.step_all();
-  TESTASSERT(not utimer.is_running());
+  TESTASSERT(not utimer.is_running() and utimer.is_expired());
 
   return SRSRAN_SUCCESS;
 }
@@ -391,6 +391,33 @@ int timers_test6()
   return SRSRAN_SUCCESS;
 }
 
+/**
+ * Test if overwriting same position of the timers handler's time wheel is safe
+ */
+int timers_test7()
+{
+  timer_handler timers;
+  size_t        wheel_size = timer_handler::get_wheel_size();
+
+  unique_timer t = timers.get_unique_timer();
+  t.set(2);
+  t.run();
+
+  timers.step_all();
+  TESTASSERT(not t.is_expired() and t.is_running());
+
+  // should fall in same wheel position
+  t.set(1 + wheel_size);
+  for (size_t i = 0; i < wheel_size; ++i) {
+    timers.step_all();
+    TESTASSERT(not t.is_expired() and t.is_running());
+  }
+  timers.step_all();
+  TESTASSERT(t.is_expired() and not t.is_running());
+
+  return SRSRAN_SUCCESS;
+}
+
 int main()
 {
   TESTASSERT(timers_test1() == SRSRAN_SUCCESS);
@@ -399,6 +426,7 @@ int main()
   TESTASSERT(timers_test4() == SRSRAN_SUCCESS);
   TESTASSERT(timers_test5() == SRSRAN_SUCCESS);
   TESTASSERT(timers_test6() == SRSRAN_SUCCESS);
+  TESTASSERT(timers_test7() == SRSRAN_SUCCESS);
   printf("Success\n");
   return 0;
 }