Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from May 11, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 27 additions & 8 deletions common/selectabletimer.cpp
Expand Up @@ -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_alive = false;
Copy link
Contributor

@qiluo-msft qiluo-msft May 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m_alive

better name m_running. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}

SelectableTimer::~SelectableTimer()
Expand All @@ -36,22 +37,40 @@ SelectableTimer::~SelectableTimer()

void SelectableTimer::start()
Copy link
Contributor

@qiluo-msft qiluo-msft May 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

start

Could you add new test case which will fail with original code? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor Author

@ds952811 ds952811 May 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please refer to timer_ut.cpp: line 43-51

{
// 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_alive)
{
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_alive = 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_alive)
{
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_alive = false;
}
}
m_mutex.unlock();
}

void SelectableTimer::reset()
Expand Down
3 changes: 3 additions & 0 deletions common/selectabletimer.h
Expand Up @@ -3,6 +3,7 @@
#include <string>
#include <vector>
#include <limits>
#include <mutex>
#include <sys/timerfd.h>
#include "selectable.h"

Expand All @@ -22,6 +23,8 @@ class SelectableTimer : public Selectable
uint64_t readData() override;

private:
std::mutex m_mutex;
bool m_alive;
int m_tfd;
itimerspec m_interval;
itimerspec m_zero;
Expand Down