Manage all rcl bindings with Handle structs

Signed-off-by: Michael X. Grey <[email protected]>

Author: mxgrey

rclrs/src/client.rs

@@ -9,32 +9,32 @@ use rosidl_runtime_rs::Message;
 
 use crate::error::{RclReturnCode, ToResult};
 use crate::MessageCow;
-use crate::{rcl_bindings::*, RclrsError};
+use crate::{rcl_bindings::*, RclrsError, NodeHandle};
 
 // SAFETY: The functions accessing this type, including drop(), shouldn't care about the thread
 // they are running in. Therefore, this type can be safely sent to another thread.
 unsafe impl Send for rcl_client_t {}
 
 /// Internal struct used by clients.
 pub struct ClientHandle {
-    rcl_client_mtx: Mutex<rcl_client_t>,
-    rcl_node_mtx: Arc<Mutex<rcl_node_t>>,
+    rcl_client: Mutex<rcl_client_t>,
+    node_handle: Arc<NodeHandle>,
     pub(crate) in_use_by_wait_set: Arc<AtomicBool>,
 }
 
 impl ClientHandle {
     pub(crate) fn lock(&self) -> MutexGuard<rcl_client_t> {
-        self.rcl_client_mtx.lock().unwrap()
+        self.rcl_client.lock().unwrap()
     }
 }
 
 impl Drop for ClientHandle {
     fn drop(&mut self) {
-        let rcl_client = self.rcl_client_mtx.get_mut().unwrap();
-        let rcl_node_mtx = &mut *self.rcl_node_mtx.lock().unwrap();
+        let rcl_client = self.rcl_client.get_mut().unwrap();
+        let rcl_node = &mut *self.node_handle.rcl_node.lock().unwrap();
         // SAFETY: No preconditions for this function
         unsafe {
-            rcl_client_fini(rcl_client, rcl_node_mtx);
+            rcl_client_fini(rcl_client, rcl_node);
         }
     }
 }
@@ -74,7 +74,7 @@ where
     T: rosidl_runtime_rs::Service,
 {
     /// Creates a new client.
-    pub(crate) fn new(rcl_node_mtx: Arc<Mutex<rcl_node_t>>, topic: &str) -> Result<Self, RclrsError>
+    pub(crate) fn new(node_handle: Arc<NodeHandle>, topic: &str) -> Result<Self, RclrsError>
     // This uses pub(crate) visibility to avoid instantiating this struct outside
     // [`Node::create_client`], see the struct's documentation for the rationale
     where
@@ -99,7 +99,7 @@ where
             // afterwards.
             rcl_client_init(
                 &mut rcl_client,
-                &*rcl_node_mtx.lock().unwrap(),
+                &*node_handle.rcl_node.lock().unwrap(),
                 type_support,
                 topic_c_string.as_ptr(),
                 &client_options,
@@ -108,8 +108,8 @@ where
         }
 
         let handle = Arc::new(ClientHandle {
-            rcl_client_mtx: Mutex::new(rcl_client),
-            rcl_node_mtx,
+            rcl_client: Mutex::new(rcl_client),
+            node_handle,
             in_use_by_wait_set: Arc::new(AtomicBool::new(false)),
         });
 
@@ -245,8 +245,8 @@ where
     ///
     pub fn service_is_ready(&self) -> Result<bool, RclrsError> {
         let mut is_ready = false;
-        let client = &mut *self.handle.rcl_client_mtx.lock().unwrap();
-        let node = &mut *self.handle.rcl_node_mtx.lock().unwrap();
+        let client = &mut *self.handle.rcl_client.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/context.rs

@@ -49,7 +49,7 @@ pub struct Context {
 /// doing it to be consistent with the lifecycle management of other rcl
 /// bindings in this library.
 pub(crate) struct ContextHandle {
-    handle: Mutex<rcl_context_t>,
+    pub(crate) rcl_context: Mutex<rcl_context_t>,
 }
 
 impl Context {
@@ -109,7 +109,9 @@ impl Context {
             ret?;
         }
         Ok(Self {
-            rcl_context_mtx: Arc::new(Mutex::new(rcl_context)),
+            handle: Arc::new(ContextHandle {
+                rcl_context: Mutex::new(rcl_context),
+            })
         })
     }
 
@@ -120,7 +122,7 @@ impl Context {
     pub fn ok(&self) -> bool {
         // This will currently always return true, but once we have a signal handler, the signal
         // handler could call `rcl_shutdown()`, hence making the context invalid.
-        let rcl_context = &mut *self.rcl_context_mtx.lock().unwrap();
+        let rcl_context = &mut *self.handle.rcl_context.lock().unwrap();
         // SAFETY: No preconditions for this function.
         unsafe { rcl_context_is_valid(rcl_context) }
     }

rclrs/src/executor.rs

@@ -42,7 +42,7 @@ impl SingleThreadedExecutor {
         for node in { self.nodes_mtx.lock().unwrap() }
             .iter()
             .filter_map(Weak::upgrade)
-            .filter(|node| unsafe { rcl_context_is_valid(&*node.rcl_context_mtx.lock().unwrap()) })
+            .filter(|node| unsafe { rcl_context_is_valid(&*node.handle.context_handle.rcl_context.lock().unwrap()) })
         {
             let wait_set = WaitSet::new_for_node(&node)?;
             let ready_entities = wait_set.wait(timeout)?;

rclrs/src/node.rs

@@ -71,21 +71,21 @@ pub struct Node {
     pub(crate) subscriptions_mtx: Mutex<Vec<Weak<dyn SubscriptionBase>>>,
     time_source: TimeSource,
     parameter: ParameterInterface,
-    pub(crate) rcl_node: Arc<NodeHandle>,
+    pub(crate) handle: Arc<NodeHandle>,
 }
 
 /// This struct manages the lifetime of the rcl node, and accounts for its
 /// dependency on the lifetime of its context.
 pub(crate) struct NodeHandle {
-    handle: Mutex<rcl_node_t>,
-    rcl_context: Arc<ContextHandle>,
+    pub(crate) rcl_node: Mutex<rcl_node_t>,
+    pub(crate) context_handle: Arc<ContextHandle>,
 }
 
 impl Eq for Node {}
 
 impl PartialEq for Node {
     fn eq(&self, other: &Self) -> bool {
-        Arc::ptr_eq(&self.rcl_node, &other.rcl_node)
+        Arc::ptr_eq(&self.handle, &other.handle)
     }
 }
 
@@ -185,7 +185,7 @@ impl Node {
         &self,
         getter: unsafe extern "C" fn(*const rcl_node_t) -> *const c_char,
     ) -> String {
-        unsafe { call_string_getter_with_handle(&self.rcl_node.handle.lock().unwrap(), getter) }
+        unsafe { call_string_getter_with_handle(&self.handle.rcl_node.lock().unwrap(), getter) }
     }
 
     /// Creates a [`Client`][1].
@@ -196,7 +196,7 @@ impl Node {
     where
         T: rosidl_runtime_rs::Service,
     {
-        let client = Arc::new(Client::<T>::new(Arc::clone(&self.rcl_node_mtx), topic)?);
+        let client = Arc::new(Client::<T>::new(Arc::clone(&self.handle), topic)?);
         { self.clients_mtx.lock().unwrap() }.push(Arc::downgrade(&client) as Weak<dyn ClientBase>);
         Ok(client)
     }
@@ -212,7 +212,7 @@ impl Node {
     /// [2]: crate::spin_once
     pub fn create_guard_condition(&self) -> Arc<GuardCondition> {
         let guard_condition = Arc::new(GuardCondition::new_with_rcl_context(
-            &mut self.rcl_context_mtx.lock().unwrap(),
+            &mut self.handle.context_handle.rcl_context.lock().unwrap(),
             None,
         ));
         { self.guard_conditions_mtx.lock().unwrap() }
@@ -234,7 +234,7 @@ impl Node {
         F: Fn() + Send + Sync + 'static,
     {
         let guard_condition = Arc::new(GuardCondition::new_with_rcl_context(
-            &mut self.rcl_context_mtx.lock().unwrap(),
+            &mut self.handle.context_handle.rcl_context.lock().unwrap(),
             Some(Box::new(callback) as Box<dyn Fn() + Send + Sync>),
         ));
         { self.guard_conditions_mtx.lock().unwrap() }
@@ -255,7 +255,7 @@ impl Node {
         T: Message,
     {
         let publisher = Arc::new(Publisher::<T>::new(
-            Arc::clone(&self.rcl_node_mtx),
+            Arc::clone(&self.handle),
             topic,
             qos,
         )?);
@@ -276,7 +276,7 @@ impl Node {
         F: Fn(&rmw_request_id_t, T::Request) -> T::Response + 'static + Send,
     {
         let service = Arc::new(Service::<T>::new(
-            Arc::clone(&self.rcl_node_mtx),
+            Arc::clone(&self.handle),
             topic,
             callback,
         )?);
@@ -299,7 +299,7 @@ impl Node {
         T: Message,
     {
         let subscription = Arc::new(Subscription::<T>::new(
-            Arc::clone(&self.rcl_node_mtx),
+            Arc::clone(&self.handle),
             topic,
             qos,
             callback,
@@ -361,7 +361,7 @@ impl Node {
     // add description about this function is for getting actual domain_id
     // and about override of domain_id via node option
     pub fn domain_id(&self) -> usize {
-        let rcl_node = &*self.rcl_node_mtx.lock().unwrap();
+        let rcl_node = &*self.handle.rcl_node.lock().unwrap();
         let mut domain_id: usize = 0;
         let ret = unsafe {
             // SAFETY: No preconditions for this function.

rclrs/src/node/builder.rs

@@ -3,7 +3,7 @@ use std::sync::{Arc, Mutex};
 
 use crate::rcl_bindings::*;
 use crate::{
-    ClockType, Context, Node, ParameterInterface, QoSProfile, RclrsError, TimeSource, ToResult,
+    ClockType, Context, ContextHandle, Node, NodeHandle, ParameterInterface, QoSProfile, RclrsError, TimeSource, ToResult,
     QOS_PROFILE_CLOCK,
 };
 
@@ -42,7 +42,7 @@ use crate::{
 /// [1]: crate::Node
 /// [2]: crate::Node::builder
 pub struct NodeBuilder {
-    context: Arc<Mutex<rcl_context_t>>,
+    context: Arc<ContextHandle>,
     name: String,
     namespace: String,
     use_global_arguments: bool,
@@ -83,14 +83,14 @@ impl NodeBuilder {
     ///     RclrsError::RclError { code: RclReturnCode::NodeInvalidName, .. }
     /// ));
     /// # Ok::<(), RclrsError>(())
-    /// ```    
-    ///    
+    /// ```
+    ///
     /// [1]: crate::Node#naming
     /// [2]: https://docs.ros2.org/latest/api/rmw/validate__node__name_8h.html#a5690a285aed9735f89ef11950b6e39e3
     /// [3]: NodeBuilder::build
     pub fn new(context: &Context, name: &str) -> NodeBuilder {
         NodeBuilder {
-            context: context.rcl_context_mtx.clone(),
+            context: Arc::clone(&context.handle),
             name: name.to_string(),
             namespace: "/".to_string(),
             use_global_arguments: true,
@@ -193,7 +193,7 @@ impl NodeBuilder {
     /// used in creating the context.
     ///
     /// For more details about command line arguments, see [here][2].
-    ///    
+    ///
     /// # Example
     /// ```
     /// # use rclrs::{Context, Node, NodeBuilder, RclrsError};
@@ -261,7 +261,7 @@ impl NodeBuilder {
                 s: self.namespace.clone(),
             })?;
         let rcl_node_options = self.create_rcl_node_options()?;
-        let rcl_context = &mut *self.context.lock().unwrap();
+        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() };
@@ -280,15 +280,17 @@ impl NodeBuilder {
             .ok()?;
         };
 
-        let rcl_node_mtx = Arc::new(Mutex::new(rcl_node));
+        let handle = Arc::new(NodeHandle {
+            rcl_node: Mutex::new(rcl_node),
+            context_handle: Arc::clone(&self.context),
+        });
         let parameter = ParameterInterface::new(
-            &rcl_node_mtx,
+            &*handle.rcl_node.lock().unwrap(),
             &rcl_node_options.arguments,
             &rcl_context.global_arguments,
         )?;
         let node = Arc::new(Node {
-            rcl_node_mtx,
-            rcl_context_mtx: self.context.clone(),
+            handle,
             clients_mtx: Mutex::new(vec![]),
             guard_conditions_mtx: Mutex::new(vec![]),
             services_mtx: Mutex::new(vec![]),

rclrs/src/node/graph.rs

@@ -138,7 +138,7 @@ impl Node {
         // SAFETY: rcl_names_and_types is zero-initialized as expected by this call
         unsafe {
             rcl_get_topic_names_and_types(
-                &*self.rcl_node_mtx.lock().unwrap(),
+                &*self.handle.rcl_node.lock().unwrap(),
                 &mut rcutils_get_default_allocator(),
                 false,
                 &mut rcl_names_and_types,
@@ -167,7 +167,7 @@ impl Node {
         // SAFETY: node_names and node_namespaces are zero-initialized as expected by this call.
         unsafe {
             rcl_get_node_names(
-                &*self.rcl_node_mtx.lock().unwrap(),
+                &*self.handle.rcl_node.lock().unwrap(),
                 rcutils_get_default_allocator(),
                 &mut rcl_names,
                 &mut rcl_namespaces,
@@ -214,7 +214,7 @@ impl Node {
         // SAFETY: The node_names, namespaces, and enclaves are zero-initialized as expected by this call.
         unsafe {
             rcl_get_node_names_with_enclaves(
-                &*self.rcl_node_mtx.lock().unwrap(),
+                &*self.handle.rcl_node.lock().unwrap(),
                 rcutils_get_default_allocator(),
                 &mut rcl_names,
                 &mut rcl_namespaces,
@@ -263,7 +263,7 @@ impl Node {
         // SAFETY: The topic_name string was correctly allocated previously
         unsafe {
             rcl_count_publishers(
-                &*self.rcl_node_mtx.lock().unwrap(),
+                &*self.handle.rcl_node.lock().unwrap(),
                 topic_name.as_ptr(),
                 &mut count,
             )
@@ -283,7 +283,7 @@ impl Node {
         // SAFETY: The topic_name string was correctly allocated previously
         unsafe {
             rcl_count_subscribers(
-                &*self.rcl_node_mtx.lock().unwrap(),
+                &*self.handle.rcl_node.lock().unwrap(),
                 topic_name.as_ptr(),
                 &mut count,
             )
@@ -337,7 +337,7 @@ impl Node {
         // SAFETY: node_name and node_namespace have been zero-initialized.
         unsafe {
             getter(
-                &*self.rcl_node_mtx.lock().unwrap(),
+                &*self.handle.rcl_node.lock().unwrap(),
                 &mut rcutils_get_default_allocator(),
                 node_name.as_ptr(),
                 node_namespace.as_ptr(),
@@ -372,7 +372,7 @@ impl Node {
         // SAFETY: topic has been zero-initialized
         unsafe {
             getter(
-                &*self.rcl_node_mtx.lock().unwrap(),
+                &*self.handle.rcl_node.lock().unwrap(),
                 &mut rcutils_get_default_allocator(),
                 topic.as_ptr(),
                 false,

rclrs/src/parameter.rs

@@ -776,13 +776,12 @@ pub(crate) struct ParameterInterface {
 
 impl ParameterInterface {
     pub(crate) fn new(
-        rcl_node_mtx: &Arc<Mutex<rcl_node_t>>,
+        rcl_node: &rcl_node_t,
         node_arguments: &rcl_arguments_t,
         global_arguments: &rcl_arguments_t,
     ) -> Result<Self, RclrsError> {
-        let rcl_node = rcl_node_mtx.lock().unwrap();
         let override_map = unsafe {
-            let fqn = call_string_getter_with_handle(&rcl_node, rcl_node_get_fully_qualified_name);
+            let fqn = call_string_getter_with_handle(rcl_node, rcl_node_get_fully_qualified_name);
             resolve_parameter_overrides(&fqn, node_arguments, global_arguments)?
         };
 

rclrs/src/parameter/value.rs

@@ -433,7 +433,7 @@ mod tests {
             let mut rcl_params = std::ptr::null_mut();
             unsafe {
                 rcl_arguments_get_param_overrides(
-                    &ctx.rcl_context_mtx.lock().unwrap().global_arguments,
+                    &ctx.handle.rcl_context.lock().unwrap().global_arguments,
                     &mut rcl_params,
                 )
                 .ok()?;