Fix some lifecycle issues in the TimerBase class.

rclcpp/include/rclcpp/client.hpp

@@ -57,9 +57,11 @@ public:
   {
     if (client_handle_) {
       if (rmw_destroy_client(client_handle_) == RMW_RET_ERROR) {
-        std::cerr << "Error in destruction of rmw client handle: " <<
+        // *INDENT-OFF*
-        (rmw_get_error_string() ? rmw_get_error_string() : "") <<
+        std::cerr << "Error in destruction of rmw client handle: "
-          std::endl;
+                  << (rmw_get_error_string() ? rmw_get_error_string() : "")
+                  << std::endl;
+        // *INDENT-ON*
       }
     }
   }

rclcpp/include/rclcpp/node_impl.hpp

@@ -50,14 +50,16 @@ Node::Node(std::string node_name, context::Context::SharedPtr context)
     if (node_handle_) {
       auto ret = rmw_destroy_node(node);
       if (ret != RMW_RET_OK) {
+        // *INDENT-OFF*
         std::cerr << "Error in destruction of rmw node handle: "
                   << (rmw_get_error_string() ? rmw_get_error_string() : "")
                   << std::endl;
+        // *INDENT-ON*
       }
     }
   });
   if (!node_handle_) {
-    // *INDENT-OFF* (prevent uncrustify from making unecessary indents here)
+    // *INDENT-OFF*
     throw std::runtime_error(
       std::string("could not create node: ") +
       (rmw_get_error_string() ? rmw_get_error_string() : ""));
@@ -127,7 +129,8 @@ Node::create_subscription(
   using rosidl_generator_cpp::get_message_type_support_handle;
   auto type_support_handle = get_message_type_support_handle<MessageT>();
   rmw_subscription_t * subscriber_handle = rmw_create_subscription(
-    node_handle_.get(), type_support_handle, topic_name.c_str(), queue_size, ignore_local_publications);
+    node_handle_.get(), type_support_handle,
+    topic_name.c_str(), queue_size, ignore_local_publications);
   if (!subscriber_handle) {
     // *INDENT-OFF* (prevent uncrustify from making unecessary indents here)
     throw std::runtime_error(

rclcpp/include/rclcpp/publisher.hpp

@@ -48,9 +48,11 @@ public:
   {
     if (publisher_handle_) {
       if (rmw_destroy_publisher(node_handle_.get(), publisher_handle_) == RMW_RET_ERROR) {
+        // *INDENT-OFF*
         std::cerr << "Error in destruction of rmw publisher handle: "
                   << (rmw_get_error_string() ? rmw_get_error_string() : "")
                   << std::endl;
+        // *INDENT-ON*
       }
     }
   }

rclcpp/include/rclcpp/subscription.hpp

@@ -53,7 +53,10 @@ public:
     subscription_handle_(subscription_handle),
     topic_name_(topic_name),
     ignore_local_publications_(ignore_local_publications)
-  {}
+  {
+    // To avoid unused private member warnings.
+    (void)ignore_local_publications_;
+  }
 
   ~SubscriptionBase()
   {

rclcpp/include/rclcpp/timer.hpp

@@ -20,6 +20,7 @@
 #include <memory>
 #include <thread>
 
+#include <rmw/error_handling.h>
 #include <rmw/rmw.h>
 
 #include <rclcpp/macros.hpp>
@@ -50,9 +51,27 @@ public:
   TimerBase(std::chrono::nanoseconds period, CallbackType callback)
   : period_(period),
     callback_(callback),
+    guard_condition_(rmw_create_guard_condition()),
     canceled_(false)
   {
-    guard_condition_ = rmw_create_guard_condition();
+    if (!guard_condition_) {
+      // TODO(wjwwood): use custom exception
+      throw std::runtime_error(
+              std::string("failed to create guard condition: ") +
+              (rmw_get_error_string() ? rmw_get_error_string() : "")
+      );
+    }
+  }
+
+  ~TimerBase()
+  {
+    if (guard_condition_) {
+      if (rmw_destroy_guard_condition(guard_condition_) == RMW_RET_ERROR) {
+        std::cerr << "Error in TimerBase destructor, rmw_destroy_guard_condition failed: " <<
+        (rmw_get_error_string() ? rmw_get_error_string() : "") <<
+          std::endl;
+      }
+    }
   }
 
   void
@@ -89,6 +108,7 @@ public:
   ~GenericTimer()
   {
     cancel();
+    thread_.join();
   }
 
   void