Skip to content

Commit

Permalink
Fix Lost Wake Bug in ROSOutAppender (#2033)
Browse files Browse the repository at this point in the history
This fixes a bug in ROSOutAppender that consists of waiting on the condition_variable `queue_condition_` without checking if the `log_queue_` is empty.

In this situation if the `log_queue_` had some messages that were inserted while `ROSOutAppender::logThread` was publishing other messages, the new messages in the queue won't be published, until another message eventually is added.

Note that the `notify_all` sent in the destructor would not cause the unpublished messages to get published, as when the `queue_condition_`  is awaken by this notification, `shutting_down_` would be true and would cause `ROSOutAppender::logThread`  to return immediately.

Co-authored-by: Adel Fakih <adel.fakih@avidbots.com>
  • Loading branch information
2 people authored and jacobperron committed Oct 16, 2020
1 parent 3b71094 commit 86d002b
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion clients/roscpp/src/libros/rosout_appender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,10 @@ void ROSOutAppender::logThread()
return;
}

queue_condition_.wait(lock);
if (log_queue_.empty())
{
queue_condition_.wait(lock);
}

if (shutting_down_)
{
Expand Down

0 comments on commit 86d002b

Please sign in to comment.