diff --git a/common/selectabletimer.cpp b/common/selectabletimer.cpp index 6f6bc0171..087c40621 100644 --- a/common/selectabletimer.cpp +++ b/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() diff --git a/common/selectabletimer.h b/common/selectabletimer.h index f7b416b2e..2572881f1 100644 --- a/common/selectabletimer.h +++ b/common/selectabletimer.h @@ -3,6 +3,7 @@ #include #include #include +#include #include #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; diff --git a/tests/Makefile.am b/tests/Makefile.am index a84a6df95..19e04250c 100644 --- a/tests/Makefile.am +++ b/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) diff --git a/tests/redis_ut.cpp b/tests/redis_ut.cpp index dd6292f0c..f33cf52ab 100644 --- a/tests/redis_ut.cpp +++ b/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); diff --git a/tests/timer_ut.cpp b/tests/timer_ut.cpp new file mode 100644 index 000000000..2cb6db6ec --- /dev/null +++ b/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); +}