Skip to content

Commit c4a68b4

Browse files
authored
Use a different implementation of mutex two priorities (ros2#1628)
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
1 parent 085f161 commit c4a68b4

File tree

2 files changed

+42
-12
lines changed

2 files changed

+42
-12
lines changed

rclcpp/include/rclcpp/detail/mutex_two_priorities.hpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#ifndef RCLCPP__DETAIL__MUTEX_TWO_PRIORITIES_HPP_
1616
#define RCLCPP__DETAIL__MUTEX_TWO_PRIORITIES_HPP_
1717

18+
#include <condition_variable>
1819
#include <mutex>
1920

2021
namespace rclcpp
@@ -62,11 +63,11 @@ class MutexTwoPriorities
6263
get_low_priority_lockable();
6364

6465
private:
65-
// Implementation detail: the idea here is that only one low priority thread can be
66-
// trying to take the data_ mutex while the others are excluded by the barrier_ mutex.
67-
// All high priority threads are already waiting for the data_ mutex.
68-
std::mutex barrier_;
69-
std::mutex data_;
66+
std::condition_variable hp_cv_;
67+
std::condition_variable lp_cv_;
68+
std::mutex cv_mutex_;
69+
size_t hp_waiting_count_{0u};
70+
bool data_taken_{false};
7071
};
7172

7273
} // namespace detail

rclcpp/src/rclcpp/detail/mutex_two_priorities.cpp

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,31 @@ HighPriorityLockable::HighPriorityLockable(MutexTwoPriorities & parent)
3131
void
3232
HighPriorityLockable::lock()
3333
{
34-
parent_.data_.lock();
34+
std::unique_lock<std::mutex> guard{parent_.cv_mutex_};
35+
if (parent_.data_taken_) {
36+
++parent_.hp_waiting_count_;
37+
while (parent_.data_taken_) {
38+
parent_.hp_cv_.wait(guard);
39+
}
40+
--parent_.hp_waiting_count_;
41+
}
42+
parent_.data_taken_ = true;
3543
}
3644

3745
void
3846
HighPriorityLockable::unlock()
3947
{
40-
parent_.data_.unlock();
48+
bool notify_lp{false};
49+
{
50+
std::lock_guard<std::mutex> guard{parent_.cv_mutex_};
51+
parent_.data_taken_ = false;
52+
notify_lp = 0u == parent_.hp_waiting_count_;
53+
}
54+
if (notify_lp) {
55+
parent_.lp_cv_.notify_one();
56+
} else {
57+
parent_.hp_cv_.notify_one();
58+
}
4159
}
4260

4361
LowPriorityLockable::LowPriorityLockable(MutexTwoPriorities & parent)
@@ -47,16 +65,27 @@ LowPriorityLockable::LowPriorityLockable(MutexTwoPriorities & parent)
4765
void
4866
LowPriorityLockable::lock()
4967
{
50-
std::unique_lock<std::mutex> barrier_guard{parent_.barrier_};
51-
parent_.data_.lock();
52-
barrier_guard.release();
68+
std::unique_lock<std::mutex> guard{parent_.cv_mutex_};
69+
while (parent_.data_taken_ || parent_.hp_waiting_count_) {
70+
parent_.lp_cv_.wait(guard);
71+
}
72+
parent_.data_taken_ = true;
5373
}
5474

5575
void
5676
LowPriorityLockable::unlock()
5777
{
58-
std::lock_guard<std::mutex> barrier_guard{parent_.barrier_, std::adopt_lock};
59-
parent_.data_.unlock();
78+
bool notify_lp{false};
79+
{
80+
std::lock_guard<std::mutex> guard{parent_.cv_mutex_};
81+
parent_.data_taken_ = false;
82+
notify_lp = 0u == parent_.hp_waiting_count_;
83+
}
84+
if (notify_lp) {
85+
parent_.lp_cv_.notify_one();
86+
} else {
87+
parent_.hp_cv_.notify_one();
88+
}
6089
}
6190

6291
HighPriorityLockable

0 commit comments

Comments
 (0)