Avoid race that triggers timer too often (#621)

The two distinct operations of acquiring and subsequent checking of a
timer have to be protected by one lock_guard against races with other
threads. The releasing of a timer has to be protected by the same lock.

Given this requirement there is no use for a second mutex.

Signed-off-by: Marko Durkovic <marko@ternaris.com>

rclcpp/include/rclcpp/executors/multi_threaded_executor.hpp

@@ -78,7 +78,6 @@ private:
   size_t number_of_threads_;
   bool yield_before_execute_;
 
-  std::mutex scheduled_timers_mutex_;
   std::set<TimerBase::SharedPtr> scheduled_timers_;
 };
 

rclcpp/src/rclcpp/executors/multi_threaded_executor.cpp

@@ -82,7 +82,6 @@ MultiThreadedExecutor::run(size_t)
       }
       if (any_exec.timer) {
         // Guard against multiple threads getting the same timer.
-        std::lock_guard<std::mutex> lock(scheduled_timers_mutex_);
         if (scheduled_timers_.count(any_exec.timer) != 0) {
           continue;
         }
@@ -96,7 +95,7 @@ MultiThreadedExecutor::run(size_t)
     execute_any_executable(any_exec);
 
     if (any_exec.timer) {
-      std::lock_guard<std::mutex> lock(scheduled_timers_mutex_);
+      std::lock_guard<std::mutex> wait_lock(wait_mutex_);
       auto it = scheduled_timers_.find(any_exec.timer);
       if (it != scheduled_timers_.end()) {
         scheduled_timers_.erase(it);