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.

Sign up for GitHub

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

Jump to bottom

protect subscriber_statistics_collectors_ with a mutex #1084

Merged
merged 6 commits into from
Apr 23, 2020
Merged

protect subscriber_statistics_collectors_ with a mutex #1084

Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9349a44
Merge remote-tracking branch 'upstream/master'
fujitatomoya Aug 30, 2019
01bd2f9
Merge remote-tracking branch 'upstream/master'
fujitatomoya Oct 1, 2019
f7b3ab5
Merge remote-tracking branch 'upstream/master'
fujitatomoya Apr 22, 2020
60efdfb
subscriber_statistics_collectors_ should be protected with mutex.
fujitatomoya Apr 22, 2020
7150067
reduce the scope of the lock for statistics collectors list.
fujitatomoya Apr 23, 2020
4498d24
Apply suggestions from code review
fujitatomoya Apr 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions rclcpp/include/rclcpp/topic_statistics/subscription_topic_statistics.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ class SubscriptionTopicStatistics
const CallbackMessageT & received_message,
const rclcpp::Time now_nanoseconds) const
{
std::lock_guard<std::mutex> lock(mutex_);
fujitatomoya marked this conversation as resolved.
Show resolved Hide resolved
for (const auto & collector : subscriber_statistics_collectors_) {
collector->OnMessageReceived(received_message, now_nanoseconds.nanoseconds());
}
Expand All @@ -120,6 +121,7 @@ class SubscriptionTopicStatistics
{
rclcpp::Time window_end{get_current_nanoseconds_since_epoch()};

std::lock_guard<std::mutex> lock(mutex_);
fujitatomoya marked this conversation as resolved.
Show resolved Hide resolved
for (auto & collector : subscriber_statistics_collectors_) {
const auto collected_stats = collector->GetStatisticsResults();

Expand All @@ -143,6 +145,7 @@ class SubscriptionTopicStatistics
std::vector<StatisticData> get_current_collector_data() const
{
std::vector<StatisticData> data;
std::lock_guard<std::mutex> lock(mutex_);
for (const auto & collector : subscriber_statistics_collectors_) {
data.push_back(collector->GetStatisticsResults());
}
Expand All @@ -155,6 +158,7 @@ class SubscriptionTopicStatistics
{
auto received_message_period = std::make_unique<ReceivedMessagePeriod>();
received_message_period->Start();
std::lock_guard<std::mutex> lock(mutex_);
subscriber_statistics_collectors_.emplace_back(std::move(received_message_period));

window_start_ = rclcpp::Time(get_current_nanoseconds_since_epoch());
Expand All @@ -163,6 +167,7 @@ class SubscriptionTopicStatistics
/// Stop all collectors, clear measurements, stop publishing timer, and reset publisher.
void tear_down()
{
std::lock_guard<std::mutex> lock(mutex_);
fujitatomoya marked this conversation as resolved.
Show resolved Hide resolved
for (auto & collector : subscriber_statistics_collectors_) {
collector->Stop();
}
Expand All @@ -187,6 +192,8 @@ class SubscriptionTopicStatistics
return std::chrono::duration_cast<std::chrono::nanoseconds>(now.time_since_epoch()).count();
}

/// Mutex to protect the subsequence vectors
mutable std::mutex mutex_;
/// Collection of statistics collectors
std::vector<std::unique_ptr<TopicStatsCollector>> subscriber_statistics_collectors_{};
/// Node name used to generate topic statistics messages to be published
Expand Down