selectabletimer: add mutex to start() and stop()

common/selectabletimer.cpp

@@ -21,6 +21,7 @@ SelectableTimer::SelectableTimer(const timespec& interval, int pri)
         SWSS_LOG_THROW("failed to create timerfd, errno: %s", strerror(errno));
     }
     setInterval(interval);
+    m_running = false;
 }
 
 SelectableTimer::~SelectableTimer()
@@ -36,22 +37,40 @@ SelectableTimer::~SelectableTimer()
 
 void SelectableTimer::start()
 {
-    // Set the timer interval and the timer is automatically started
-    int rc = timerfd_settime(m_tfd, 0, &m_interval, NULL);
-    if (rc == -1)
+    m_mutex.lock();
+    if (!m_running)
     {
-        SWSS_LOG_THROW("failed to set timerfd, errno: %s", strerror(errno));
+        // Set the timer interval and the timer is automatically started
+        int rc = timerfd_settime(m_tfd, 0, &m_interval, NULL);
+        if (rc == -1)
+        {
+            SWSS_LOG_THROW("failed to set timerfd, errno: %s", strerror(errno));
+        }
+        else
+        {
+            m_running = true;
+        }
     }
+    m_mutex.unlock();
 }
 
 void SelectableTimer::stop()
 {
-    // Set the timer interval and the timer is automatically started
-    int rc = timerfd_settime(m_tfd, 0, &m_zero, NULL);
-    if (rc == -1)
+    m_mutex.lock();
+    if (m_running)
     {
-        SWSS_LOG_THROW("failed to set timerfd to zero, errno: %s", strerror(errno));
+        // Set the timer interval and the timer is automatically started
+        int rc = timerfd_settime(m_tfd, 0, &m_zero, NULL);
+        if (rc == -1)
+        {
+            SWSS_LOG_THROW("failed to set timerfd to zero, errno: %s", strerror(errno));
+        }
+        else
+        {
+            m_running = false;
+        }
     }
+    m_mutex.unlock();
 }
 
 void SelectableTimer::reset()

common/selectabletimer.h

@@ -3,6 +3,7 @@
 #include <string>
 #include <vector>
 #include <limits>
+#include <mutex>
 #include <sys/timerfd.h>
 #include "selectable.h"
 
@@ -22,6 +23,8 @@ class SelectableTimer : public Selectable
     uint64_t readData() override;
 
 private:
+    std::mutex m_mutex;
+    bool m_running;
     int m_tfd;
     itimerspec m_interval;
     itimerspec m_zero;

tests/Makefile.am

@@ -35,6 +35,7 @@ tests_SOURCES = redis_ut.cpp                \
                 boolean_ut.cpp              \
                 status_code_util_test.cpp   \
                 saiaclschema_ut.cpp         \
+                timer_ut.cpp                \
                 main.cpp
 
 tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS)

tests/redis_ut.cpp

@@ -653,42 +653,6 @@ TEST(DBConnector, selectableevent)
     EXPECT_EQ(value, 2);
 }
 
-TEST(DBConnector, selectabletimer)
-{
-    timespec interval = { .tv_sec = 1, .tv_nsec = 0 };
-    SelectableTimer timer(interval);
-
-    Select s;
-    s.addSelectable(&timer);
-    Selectable *sel;
-    int result;
-
-    // Wait a non started timer
-    result = s.select(&sel, 2000);
-    ASSERT_EQ(result, Select::TIMEOUT);
-
-    // Wait long enough so we got timer notification first
-    timer.start();
-    result = s.select(&sel, 2000);
-    ASSERT_EQ(result, Select::OBJECT);
-    ASSERT_EQ(sel, &timer);
-
-    // Wait short so we got select timeout first
-    result = s.select(&sel, 10);
-    ASSERT_EQ(result, Select::TIMEOUT);
-
-    // Wait long enough so we got timer notification first
-    result = s.select(&sel, 10000);
-    ASSERT_EQ(result, Select::OBJECT);
-    ASSERT_EQ(sel, &timer);
-
-    // Reset and wait long enough so we got timer notification first
-    timer.reset();
-    result = s.select(&sel, 10000);
-    ASSERT_EQ(result, Select::OBJECT);
-    ASSERT_EQ(sel, &timer);
-}
-
 TEST(Table, basic)
 {
     TableBasicTest("TABLE_UT_TEST", true);

tests/timer_ut.cpp

@@ -0,0 +1,52 @@
+#include "common/select.h"
+#include "common/selectabletimer.h"
+#include "gtest/gtest.h"
+
+using namespace std;
+using namespace swss;
+
+TEST(TIMER, selectabletimer)
+{
+    timespec interval = { .tv_sec = 1, .tv_nsec = 0 };
+    SelectableTimer timer(interval);
+
+    Select s;
+    s.addSelectable(&timer);
+    Selectable *sel;
+    int result;
+
+    // Wait a non started timer
+    result = s.select(&sel, 2000);
+    ASSERT_EQ(result, Select::TIMEOUT);
+
+    // Wait long enough so we got timer notification first
+    timer.start();
+    result = s.select(&sel, 2000);
+    ASSERT_EQ(result, Select::OBJECT);
+    ASSERT_EQ(sel, &timer);
+
+    // Wait short so we got select timeout first
+    result = s.select(&sel, 10);
+    ASSERT_EQ(result, Select::TIMEOUT);
+
+    // Wait long enough so we got timer notification first
+    result = s.select(&sel, 10000);
+    ASSERT_EQ(result, Select::OBJECT);
+    ASSERT_EQ(sel, &timer);
+
+    // Reset and wait long enough so we got timer notification first
+    timer.reset();
+    result = s.select(&sel, 10000);
+    ASSERT_EQ(result, Select::OBJECT);
+    ASSERT_EQ(sel, &timer);
+
+    // Check if the timer gets reset by subsequent timer.start()
+    for (int t = 0; t < 2000; ++t)
+    {
+        timer.start();
+        usleep(1000);
+    }
+    result = s.select(&sel, 1);
+    ASSERT_EQ(result, Select::OBJECT);
+    ASSERT_EQ(sel, &timer);
+}