From 100417a98d804f85d45d264aa11e2fcf5e26e349 Mon Sep 17 00:00:00 2001
From: gerkey <brian@gerkey.org>
Date: Thu, 27 Apr 2017 18:47:13 -0700
Subject: [PATCH] fix race conditions in guard condition handling (#325)

---
 rclcpp/src/rclcpp/client.cpp                     | 12 ++++++------
 rclcpp/src/rclcpp/node_interfaces/node_graph.cpp | 13 ++++++++-----
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/rclcpp/src/rclcpp/client.cpp b/rclcpp/src/rclcpp/client.cpp
index c15a0b1..0d29715 100644
--- a/rclcpp/src/rclcpp/client.cpp
+++ b/rclcpp/src/rclcpp/client.cpp
@@ -70,6 +70,12 @@ bool
 ClientBase::wait_for_service_nanoseconds(std::chrono::nanoseconds timeout)
 {
   auto start = std::chrono::steady_clock::now();
+  // make an event to reuse, rather than create a new one each time
+  auto node_ptr = node_graph_.lock();
+  if (!node_ptr) {
+    throw InvalidNodeError();
+  }
+  auto event = node_ptr->get_graph_event();
   // check to see if the server is ready immediately
   if (this->service_is_ready()) {
     return true;
@@ -78,12 +84,6 @@ ClientBase::wait_for_service_nanoseconds(std::chrono::nanoseconds timeout)
     // check was non-blocking, return immediately
     return false;
   }
-  // make an event to reuse, rather than create a new one each time
-  auto node_ptr = node_graph_.lock();
-  if (!node_ptr) {
-    throw InvalidNodeError();
-  }
-  auto event = node_ptr->get_graph_event();
   // update the time even on the first loop to account for time spent in the first call
   // to this->server_is_ready()
   std::chrono::nanoseconds time_to_wait = timeout - (std::chrono::steady_clock::now() - start);
diff --git a/rclcpp/src/rclcpp/node_interfaces/node_graph.cpp b/rclcpp/src/rclcpp/node_interfaces/node_graph.cpp
index bc7a5ad..ce2963b 100644
--- a/rclcpp/src/rclcpp/node_interfaces/node_graph.cpp
+++ b/rclcpp/src/rclcpp/node_interfaces/node_graph.cpp
@@ -205,13 +205,13 @@ NodeGraph::get_graph_event()
 {
   auto event = rclcpp::event::Event::make_shared();
   std::lock_guard<std::mutex> graph_changed_lock(graph_mutex_);
+  graph_events_.push_back(event);
+  graph_users_count_++;
   // on first call, add node to graph_listener_
   if (should_add_to_graph_listener_.exchange(false)) {
     graph_listener_->add_node(this);
     graph_listener_->start_if_not_started();
   }
-  graph_events_.push_back(event);
-  graph_users_count_++;
   return event;
 }
 
@@ -238,10 +238,13 @@ NodeGraph::wait_for_graph_change(
       throw EventNotRegisteredError();
     }
   }
+  auto pred = [&event]() {
+      return event->check() || !rclcpp::utilities::ok();
+    };
   std::unique_lock<std::mutex> graph_lock(graph_mutex_);
-  graph_cv_.wait_for(graph_lock, timeout, [&event]() {
-    return event->check() || !rclcpp::utilities::ok();
-  });
+  if (!pred()) {
+    graph_cv_.wait_for(graph_lock, timeout, pred);
+  }
 }
 
 size_t