Primary change to fix builder's node variable going out of scope

rclrs/src/client.rs

@@ -43,7 +43,7 @@ impl Drop for ClientHandle {
         // SAFETY: The entity lifecycle mutex is locked to protect against the risk of
         // global variables in the rmw implementation being unsafely modified during cleanup.
         unsafe {
-            rcl_client_fini(rcl_client, &mut *rcl_node);
+            rcl_client_fini(rcl_client, &mut **rcl_node);
         }
     }
 }
@@ -115,7 +115,7 @@ where
             unsafe {
                 rcl_client_init(
                     &mut rcl_client,
-                    &*rcl_node,
+                    &**rcl_node,
                     type_support,
                     topic_c_string.as_ptr(),
                     &client_options,
@@ -263,7 +263,7 @@ where
     pub fn service_is_ready(&self) -> Result<bool, RclrsError> {
         let mut is_ready = false;
         let client = &mut *self.handle.rcl_client.lock().unwrap();
-        let node = &mut *self.handle.node_handle.rcl_node.lock().unwrap();
+        let node = &mut **self.handle.node_handle.rcl_node.lock().unwrap();
 
         unsafe {
             // SAFETY both node and client are guaranteed to be valid here

rclrs/src/logging.rs

@@ -196,14 +196,24 @@ impl LogConditions {
 /// # Examples
 ///
 /// ```
+/// use rclrs::{log_debug, log_error, log_fatal, log_info, log_warn, LogConditions, LoggingOccurrence};
+/// use std::sync::Mutex;
+/// use std::time::Duration;
+/// use std::env;
+///
+/// let context = rclrs::Context::new(env::args()).unwrap();
+/// let node = rclrs::Node::new(&context, "test_node").unwrap();
+///
 /// log_debug!(&node.name(), "Simple message");
+/// let some_variable = 43;
 /// log_debug!(&node.name(), "Simple message {some_variable}");
 /// log_fatal!(&node.name(), "Simple message from {}", node.name());
 /// log_warn!(&node.name(), LogConditions::once(), "Only log this the first time");
 /// log_error!(&node.name(),
 ///            LogConditions::skip_first_throttle(Duration::from_millis(1000)),
 ///            "Noisy error that we expect the first time");
 ///
+/// let count = 0;
 /// log_info!(&node.name(), LogConditions { occurs: LoggingOccurrence::Always,
 ///                                         publish_interval: Duration::from_millis(1000),
 ///                                         log_if_true: count % 10 == 0, },

rclrs/src/node.rs

@@ -71,10 +71,15 @@ pub struct Node {
 /// This struct manages the lifetime of an `rcl_node_t`, and accounts for its
 /// dependency on the lifetime of its `rcl_context_t` by ensuring that this
 /// dependency is [dropped after][1] the `rcl_node_t`.
+/// Note: we capture the rcl_node_t returned from rcl_get_zero_initialized_node()
+/// to guarantee that the node handle exists until we drop the NodeHandle
+/// instance. This addresses an issue where previously the address of the variable
+/// in the builder.rs was being used, and whose lifespan was (just) shorter than the
+/// NodeHandle instance.
 ///
 /// [1]: <https://doc.rust-lang.org/reference/destructors.html>
 pub(crate) struct NodeHandle {
-    pub(crate) rcl_node: Mutex<rcl_node_t>,
+    pub(crate) rcl_node: Mutex<Box<rcl_node_t>>,
     pub(crate) context_handle: Arc<ContextHandle>,
 }
 
@@ -83,9 +88,10 @@ impl Drop for NodeHandle {
         let _context_lock = self.context_handle.rcl_context.lock().unwrap();
         let mut rcl_node = self.rcl_node.lock().unwrap();
         let _lifecycle_lock = ENTITY_LIFECYCLE_MUTEX.lock().unwrap();
+
         // SAFETY: The entity lifecycle mutex is locked to protect against the risk of
         // global variables in the rmw implementation being unsafely modified during cleanup.
-        unsafe { rcl_node_fini(&mut *rcl_node) };
+        unsafe { rcl_node_fini(&mut **rcl_node) };
     }
 }
 
@@ -370,7 +376,7 @@ impl Node {
         let mut domain_id: usize = 0;
         let ret = unsafe {
             // SAFETY: No preconditions for this function.
-            rcl_node_get_domain_id(&*rcl_node, &mut domain_id)
+            rcl_node_get_domain_id(&**rcl_node, &mut domain_id)
         };
 
         debug_assert_eq!(ret, 0);
@@ -445,7 +451,7 @@ impl Node {
     pub fn logger_name(&self) -> &str {
         let rcl_node = self.handle.rcl_node.lock().unwrap();
         // SAFETY: No pre-conditions for this function
-        let name_raw_ptr = unsafe { rcl_node_get_logger_name(&*rcl_node) };
+        let name_raw_ptr = unsafe { rcl_node_get_logger_name(&**rcl_node) };
         if name_raw_ptr.is_null() {
             return "";
         }
@@ -535,8 +541,14 @@ mod tests {
         // Use helper to create 2 nodes for us
         let graph = construct_test_graph("test_topics_graph")?;
 
-        assert_eq!(graph.node1.logger_name(), "graph_test_node_1");
-        assert_eq!(graph.node2.logger_name(), "graph_test_node_2");
+        assert_eq!(
+            graph.node1.logger_name(),
+            "test_topics_graph.graph_test_node_1"
+        );
+        assert_eq!(
+            graph.node2.logger_name(),
+            "test_topics_graph.graph_test_node_2"
+        );
 
         Ok(())
     }

rclrs/src/node/builder.rs

@@ -277,7 +277,7 @@ impl NodeBuilder {
         let rcl_context = &mut *self.context.rcl_context.lock().unwrap();
 
         // SAFETY: Getting a zero-initialized value is always safe.
-        let mut rcl_node = unsafe { rcl_get_zero_initialized_node() };
+        let mut rcl_node = Box::new(unsafe { rcl_get_zero_initialized_node() });
         unsafe {
             // SAFETY:
             // * The rcl_node is zero-initialized as mandated by this function.
@@ -287,7 +287,7 @@ impl NodeBuilder {
             //   global variables in the rmw implementation being unsafely modified during cleanup.
             let _lifecycle_lock = ENTITY_LIFECYCLE_MUTEX.lock().unwrap();
             rcl_node_init(
-                &mut rcl_node,
+                &mut *rcl_node,
                 node_name.as_ptr(),
                 node_namespace.as_ptr(),
                 rcl_context,

rclrs/src/node/graph.rs

@@ -139,7 +139,7 @@ impl Node {
         unsafe {
             let rcl_node = self.handle.rcl_node.lock().unwrap();
             rcl_get_topic_names_and_types(
-                &*rcl_node,
+                &**rcl_node,
                 &mut rcutils_get_default_allocator(),
                 false,
                 &mut rcl_names_and_types,
@@ -169,7 +169,7 @@ impl Node {
         unsafe {
             let rcl_node = self.handle.rcl_node.lock().unwrap();
             rcl_get_node_names(
-                &*rcl_node,
+                &**rcl_node,
                 rcutils_get_default_allocator(),
                 &mut rcl_names,
                 &mut rcl_namespaces,
@@ -217,7 +217,7 @@ impl Node {
         unsafe {
             let rcl_node = self.handle.rcl_node.lock().unwrap();
             rcl_get_node_names_with_enclaves(
-                &*rcl_node,
+                &**rcl_node,
                 rcutils_get_default_allocator(),
                 &mut rcl_names,
                 &mut rcl_namespaces,
@@ -266,7 +266,7 @@ impl Node {
         // SAFETY: The topic_name string was correctly allocated previously
         unsafe {
             let rcl_node = self.handle.rcl_node.lock().unwrap();
-            rcl_count_publishers(&*rcl_node, topic_name.as_ptr(), &mut count).ok()?
+            rcl_count_publishers(&**rcl_node, topic_name.as_ptr(), &mut count).ok()?
         };
         Ok(count)
     }
@@ -282,7 +282,7 @@ impl Node {
         // SAFETY: The topic_name string was correctly allocated previously
         unsafe {
             let rcl_node = self.handle.rcl_node.lock().unwrap();
-            rcl_count_subscribers(&*rcl_node, topic_name.as_ptr(), &mut count).ok()?
+            rcl_count_subscribers(&**rcl_node, topic_name.as_ptr(), &mut count).ok()?
         };
         Ok(count)
     }
@@ -333,7 +333,7 @@ impl Node {
         unsafe {
             let rcl_node = self.handle.rcl_node.lock().unwrap();
             getter(
-                &*rcl_node,
+                &**rcl_node,
                 &mut rcutils_get_default_allocator(),
                 node_name.as_ptr(),
                 node_namespace.as_ptr(),
@@ -369,7 +369,7 @@ impl Node {
         unsafe {
             let rcl_node = self.handle.rcl_node.lock().unwrap();
             getter(
-                &*rcl_node,
+                &**rcl_node,
                 &mut rcutils_get_default_allocator(),
                 topic.as_ptr(),
                 false,

rclrs/src/publisher.rs

@@ -38,7 +38,7 @@ impl Drop for PublisherHandle {
         // SAFETY: The entity lifecycle mutex is locked to protect against the risk of
         // global variables in the rmw implementation being unsafely modified during cleanup.
         unsafe {
-            rcl_publisher_fini(self.rcl_publisher.get_mut().unwrap(), &mut *rcl_node);
+            rcl_publisher_fini(self.rcl_publisher.get_mut().unwrap(), &mut **rcl_node);
         }
     }
 }
@@ -111,7 +111,7 @@ where
                 //   variables in the rmw implementation being unsafely modified during cleanup.
                 rcl_publisher_init(
                     &mut rcl_publisher,
-                    &*rcl_node,
+                    &**rcl_node,
                     type_support_ptr,
                     topic_c_string.as_ptr(),
                     &publisher_options,

rclrs/src/service.rs

@@ -41,7 +41,7 @@ impl Drop for ServiceHandle {
         // SAFETY: The entity lifecycle mutex is locked to protect against the risk of
         // global variables in the rmw implementation being unsafely modified during cleanup.
         unsafe {
-            rcl_service_fini(rcl_service, &mut *rcl_node);
+            rcl_service_fini(rcl_service, &mut **rcl_node);
         }
     }
 }
@@ -116,7 +116,7 @@ where
                 //   variables in the rmw implementation being unsafely modified during initialization.
                 rcl_service_init(
                     &mut rcl_service,
-                    &*rcl_node,
+                    &**rcl_node,
                     type_support,
                     topic_c_string.as_ptr(),
                     &service_options as *const _,

rclrs/src/subscription.rs

@@ -49,7 +49,7 @@ impl Drop for SubscriptionHandle {
         // SAFETY: The entity lifecycle mutex is locked to protect against the risk of
         // global variables in the rmw implementation being unsafely modified during cleanup.
         unsafe {
-            rcl_subscription_fini(rcl_subscription, &mut *rcl_node);
+            rcl_subscription_fini(rcl_subscription, &mut **rcl_node);
         }
     }
 }
@@ -129,7 +129,7 @@ where
                 //   variables in the rmw implementation being unsafely modified during cleanup.
                 rcl_subscription_init(
                     &mut rcl_subscription,
-                    &*rcl_node,
+                    &**rcl_node,
                     type_support,
                     topic_c_string.as_ptr(),
                     &subscription_options,