Skip to content

Commit 3bc1231

Browse files
committed
add pthread synchronization primitives to fix gcc8 bugs in std::condition_variable
Signed-off-by: Alberto Soragna <[email protected]>
1 parent c49b672 commit 3bc1231

File tree

3 files changed

+172
-14
lines changed

3 files changed

+172
-14
lines changed
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
// Copyright (C) 2022, iRobot Corporation.
2+
3+
#ifndef RCLCPP__SYNC_PRIMITIVES_HPP_
4+
#define RCLCPP__SYNC_PRIMITIVES_HPP_
5+
6+
#include <cstdint>
7+
#include <sys/time.h>
8+
#include <pthread.h>
9+
10+
namespace rclcpp
11+
{
12+
13+
class RecursiveMutex
14+
{
15+
public:
16+
RecursiveMutex()
17+
{
18+
pthread_mutexattr_t attr;
19+
int ret = pthread_mutexattr_init(&attr);
20+
assert(ret == 0);
21+
ret = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
22+
assert(ret == 0);
23+
ret = pthread_mutexattr_setprotocol(&attr, PTHREAD_PRIO_INHERIT);
24+
assert(ret == 0);
25+
ret = pthread_mutex_init(&m_mutex, &attr);
26+
assert(ret == 0);
27+
ret = pthread_mutexattr_destroy(&attr);
28+
assert(ret == 0);
29+
(void)ret;
30+
}
31+
32+
// Not copyable or movable
33+
RecursiveMutex(const RecursiveMutex &) = delete;
34+
RecursiveMutex(RecursiveMutex &&) = delete;
35+
RecursiveMutex &operator=(const RecursiveMutex &) = delete;
36+
RecursiveMutex &operator=(RecursiveMutex &&) = delete;
37+
38+
~RecursiveMutex()
39+
{
40+
int ret = pthread_mutex_destroy(&m_mutex);
41+
assert(ret == 0);
42+
(void)ret;
43+
}
44+
45+
void lock()
46+
{
47+
int ret = pthread_mutex_lock(&m_mutex);
48+
assert(ret == 0);
49+
(void)ret;
50+
}
51+
52+
void unlock()
53+
{
54+
int ret = pthread_mutex_unlock(&m_mutex);
55+
assert(ret == 0);
56+
(void)ret;
57+
}
58+
59+
pthread_mutex_t& native_handle()
60+
{
61+
return m_mutex;
62+
}
63+
64+
private:
65+
pthread_mutex_t m_mutex{};
66+
};
67+
68+
class Lock
69+
{
70+
public:
71+
explicit Lock(RecursiveMutex& mutex)
72+
: m_mutex(&mutex), m_lock_cnt(1)
73+
{
74+
m_mutex->lock();
75+
}
76+
77+
// Not copyable
78+
Lock(const Lock&) = delete;
79+
Lock &operator=(const Lock &) = delete;
80+
81+
/** Unlock the mutex associated with this lock. */
82+
~Lock()
83+
{
84+
while (m_lock_cnt > 0) {
85+
m_mutex->unlock();
86+
--m_lock_cnt;
87+
}
88+
}
89+
90+
private:
91+
RecursiveMutex *m_mutex;
92+
int m_lock_cnt;
93+
};
94+
95+
class ConditionVariable
96+
{
97+
public:
98+
ConditionVariable()
99+
{
100+
pthread_condattr_t attr;
101+
pthread_condattr_init(&attr);
102+
int ret = pthread_condattr_setclock(&attr, CLOCK_MONOTONIC);
103+
assert(ret == 0);
104+
ret = pthread_cond_init(&m_cond, &attr);
105+
assert(ret == 0);
106+
ret = pthread_condattr_destroy(&attr);
107+
assert(ret == 0);
108+
(void)ret;
109+
}
110+
111+
~ConditionVariable()
112+
{
113+
int ret = pthread_cond_destroy(&m_cond);
114+
assert(ret == 0);
115+
(void)ret;
116+
}
117+
118+
void notify_one()
119+
{
120+
pthread_cond_signal(&m_cond);
121+
}
122+
123+
void wait(RecursiveMutex& mutex)
124+
{
125+
pthread_cond_wait(&this->m_cond, &mutex.native_handle());
126+
}
127+
128+
bool wait(RecursiveMutex& mutex, uint64_t nanoseconds)
129+
{
130+
// Determine the absolute system time when timeout occurs.
131+
timespec start {};
132+
clock_gettime(CLOCK_MONOTONIC, &start);
133+
134+
static constexpr uint64_t NS_PER_SECONDS = 1000000000;
135+
136+
timespec wait {};
137+
wait.tv_nsec = nanoseconds % NS_PER_SECONDS;
138+
wait.tv_sec = nanoseconds / NS_PER_SECONDS;
139+
140+
timespec timeout {};
141+
const uint64_t nanoseconds_sum = start.tv_nsec + wait.tv_nsec;
142+
timeout.tv_nsec = nanoseconds_sum % NS_PER_SECONDS;
143+
timeout.tv_sec = start.tv_sec + wait.tv_sec + nanoseconds_sum / NS_PER_SECONDS;
144+
145+
return pthread_cond_timedwait(&this->m_cond, &mutex.native_handle(), &timeout) == 0;
146+
}
147+
148+
private:
149+
pthread_cond_t m_cond{};
150+
};
151+
152+
} // namespace rclcpp
153+
154+
#endif // RCLCPP__SYNC_PRIMITIVES_HPP_

irobot_events_executor/include/rclcpp/timers_manager.hpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#include "rclcpp/context.hpp"
1818
#include "rclcpp/timer.hpp"
19+
#include "rclcpp/sync_primitives.hpp"
1920

2021
namespace rclcpp
2122
{
@@ -485,9 +486,9 @@ class TimersManager
485486
// Thread used to run the timers execution task
486487
std::thread timers_thread_;
487488
// Protects access to timers
488-
std::mutex timers_mutex_;
489+
rclcpp::RecursiveMutex timers_mutex_;
489490
// Notifies the timers thread whenever timers are added/removed
490-
std::condition_variable timers_cv_;
491+
rclcpp::ConditionVariable timers_cv_;
491492
// Flag used as predicate by timers_cv_ that denotes one or more timers being added/removed
492493
bool timers_updated_ {false};
493494
// Indicates whether the timers thread is currently running or not

irobot_events_executor/src/rclcpp/timers_manager.cpp

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ void TimersManager::add_timer(rclcpp::TimerBase::SharedPtr timer)
3636

3737
bool added = false;
3838
{
39-
std::unique_lock<std::mutex> lock(timers_mutex_);
39+
rclcpp::Lock lock(timers_mutex_);
4040
added = weak_timers_heap_.add_timer(timer);
4141
timers_updated_ = timers_updated_ || added;
4242
}
@@ -67,7 +67,7 @@ void TimersManager::stop()
6767

6868
// Notify the timers manager thread to wake up
6969
{
70-
std::unique_lock<std::mutex> lock(timers_mutex_);
70+
rclcpp::Lock lock(timers_mutex_);
7171
timers_updated_ = true;
7272
}
7373
timers_cv_.notify_one();
@@ -86,7 +86,7 @@ std::chrono::nanoseconds TimersManager::get_head_timeout()
8686
"get_head_timeout() can't be used while timers thread is running");
8787
}
8888

89-
std::unique_lock<std::mutex> lock(timers_mutex_);
89+
rclcpp::Lock lock(timers_mutex_);
9090
return this->get_head_timeout_unsafe();
9191
}
9292

@@ -98,7 +98,7 @@ size_t TimersManager::get_number_ready_timers()
9898
"get_number_ready_timers() can't be used while timers thread is running");
9999
}
100100

101-
std::unique_lock<std::mutex> lock(timers_mutex_);
101+
rclcpp::Lock lock(timers_mutex_);
102102
TimersHeap locked_heap = weak_timers_heap_.validate_and_lock();
103103
return locked_heap.get_number_ready_timers();
104104
}
@@ -111,7 +111,7 @@ void TimersManager::execute_ready_timers()
111111
"execute_ready_timers() can't be used while timers thread is running");
112112
}
113113

114-
std::unique_lock<std::mutex> lock(timers_mutex_);
114+
rclcpp::Lock lock(timers_mutex_);
115115
this->execute_ready_timers_unsafe();
116116
}
117117

@@ -123,7 +123,7 @@ bool TimersManager::execute_head_timer()
123123
"execute_head_timer() can't be used while timers thread is running");
124124
}
125125

126-
std::unique_lock<std::mutex> lock(timers_mutex_);
126+
rclcpp::Lock lock(timers_mutex_);
127127

128128
TimersHeap timers_heap = weak_timers_heap_.validate_and_lock();
129129

@@ -153,7 +153,7 @@ void TimersManager::execute_ready_timer(const void * timer_id)
153153
{
154154
TimerPtr ready_timer;
155155
{
156-
std::unique_lock<std::mutex> lock(timers_mutex_);
156+
rclcpp::Lock lock(timers_mutex_);
157157
ready_timer = weak_timers_heap_.get_timer(timer_id);
158158
}
159159
if (ready_timer) {
@@ -228,18 +228,21 @@ void TimersManager::run_timers()
228228
{
229229
while (rclcpp::ok(context_) && running_) {
230230
// Lock mutex
231-
std::unique_lock<std::mutex> lock(timers_mutex_);
231+
rclcpp::Lock lock(timers_mutex_);
232232

233233
std::chrono::nanoseconds time_to_sleep = get_head_timeout_unsafe();
234234

235235
// No need to wait if a timer is already available
236236
if (time_to_sleep > std::chrono::nanoseconds::zero()) {
237237
if (time_to_sleep != std::chrono::nanoseconds::max()) {
238238
// Wait until timeout or notification that timers have been updated
239-
timers_cv_.wait_for(lock, time_to_sleep, [this]() {return timers_updated_;});
239+
uint64_t timeout = time_to_sleep.count();
240+
timers_cv_.wait(timers_mutex_, timeout);
240241
} else {
241242
// Wait until notification that timers have been updated
242-
timers_cv_.wait(lock, [this]() {return timers_updated_;});
243+
while (!timers_updated_) {
244+
timers_cv_.wait(timers_mutex_);
245+
}
243246
}
244247
}
245248

@@ -259,7 +262,7 @@ void TimersManager::clear()
259262
{
260263
{
261264
// Lock mutex and then clear all data structures
262-
std::unique_lock<std::mutex> lock(timers_mutex_);
265+
rclcpp::Lock lock(timers_mutex_);
263266
weak_timers_heap_.clear();
264267

265268
timers_updated_ = true;
@@ -273,7 +276,7 @@ void TimersManager::remove_timer(TimerPtr timer)
273276
{
274277
bool removed = false;
275278
{
276-
std::unique_lock<std::mutex> lock(timers_mutex_);
279+
rclcpp::Lock lock(timers_mutex_);
277280
removed = weak_timers_heap_.remove_timer(timer);
278281

279282
timers_updated_ = timers_updated_ || removed;

0 commit comments

Comments
 (0)