Replace parking_lot Mutex with std::sync::Mutex

Author: nnmm

rclrs/Cargo.toml

@@ -15,8 +15,6 @@ path = "src/lib.rs"
 [dependencies]
 # Needed for FFI
 libc = "0.2.43"
-# Provides better concurrency primitives than std
-parking_lot = "0.11.2"
 # Needed for the Message trait, among others
 rosidl_runtime_rs = "0.2.0"
 # Needed for clients

rclrs/src/context.rs

@@ -4,11 +4,9 @@ use crate::{RclrsError, ToResult};
 use std::ffi::CString;
 use std::os::raw::c_char;
 use std::string::String;
-use std::sync::Arc;
+use std::sync::{Arc, Mutex};
 use std::vec::Vec;
 
-use parking_lot::Mutex;
-
 impl Drop for rcl_context_t {
     fn drop(&mut self) {
         unsafe {
@@ -112,7 +110,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();
+        let rcl_context = &mut *self.rcl_context_mtx.lock().unwrap();
         // SAFETY: No preconditions for this function.
         unsafe { rcl_context_is_valid(rcl_context) }
     }

rclrs/src/lib.rs

@@ -89,10 +89,12 @@ pub fn spin(node: &Node) -> Result<(), RclrsError> {
     // The context_is_valid functions exists only to abstract away ROS distro differences
     #[cfg(ros_distro = "foxy")]
     // SAFETY: No preconditions for this function.
-    let context_is_valid = || unsafe { rcl_context_is_valid(&mut *node.rcl_context_mtx.lock()) };
+    let context_is_valid =
+        || unsafe { rcl_context_is_valid(&mut *node.rcl_context_mtx.lock().unwrap()) };
     #[cfg(not(ros_distro = "foxy"))]
     // SAFETY: No preconditions for this function.
-    let context_is_valid = || unsafe { rcl_context_is_valid(&*node.rcl_context_mtx.lock()) };
+    let context_is_valid =
+        || unsafe { rcl_context_is_valid(&*node.rcl_context_mtx.lock().unwrap()) };
 
     while context_is_valid() {
         match spin_once(node, None) {

rclrs/src/node.rs

@@ -15,12 +15,10 @@ use crate::{Context, ParameterOverrideMap, QoSProfile, RclrsError, ToResult};
 use std::cmp::PartialEq;
 use std::ffi::CStr;
 use std::fmt;
-use std::sync::{Arc, Weak};
+use std::sync::{Arc, Mutex, Weak};
 use std::vec::Vec;
 
 use libc::c_char;
-use parking_lot::Mutex;
-
 use rosidl_runtime_rs::Message;
 
 impl Drop for rcl_node_t {
@@ -177,7 +175,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_mtx.lock(), getter) }
+        unsafe { call_string_getter_with_handle(&*self.rcl_node_mtx.lock().unwrap(), getter) }
     }
 
     /// Creates a [`Client`][1].
@@ -291,7 +289,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();
+        let rcl_node = &*self.rcl_node_mtx.lock().unwrap();
         let mut domain_id: usize = 0;
         let ret = unsafe {
             // SAFETY: No preconditions for this function.

rclrs/src/node/builder.rs

@@ -5,9 +5,7 @@ use crate::{
 };
 
 use std::ffi::CString;
-use std::sync::Arc;
-
-use parking_lot::Mutex;
+use std::sync::{Arc, Mutex};
 
 /// A builder for creating a [`Node`][1].
 ///
@@ -245,7 +243,7 @@ impl NodeBuilder {
                 s: self.namespace.clone(),
             })?;
         let rcl_node_options = self.create_rcl_node_options()?;
-        let rcl_context = &mut *self.context.lock();
+        let rcl_context = &mut *self.context.lock().unwrap();
 
         // SAFETY: Getting a zero-initialized value is always safe.
         let mut rcl_node = unsafe { rcl_get_zero_initialized_node() };

rclrs/src/node/client.rs

@@ -3,14 +3,13 @@ use std::boxed::Box;
 use std::collections::HashMap;
 use std::ffi::CString;
 use std::sync::atomic::AtomicBool;
-use std::sync::Arc;
+use std::sync::{Arc, Mutex, MutexGuard};
 
 use crate::error::{RclReturnCode, ToResult};
 use crate::MessageCow;
 use crate::Node;
 use crate::{rcl_bindings::*, RclrsError};
 
-use parking_lot::{Mutex, MutexGuard};
 use rosidl_runtime_rs::Message;
 
 // SAFETY: The functions accessing this type, including drop(), shouldn't care about the thread
@@ -26,17 +25,17 @@ pub struct ClientHandle {
 
 impl ClientHandle {
     pub(crate) fn lock(&self) -> MutexGuard<rcl_client_t> {
-        self.rcl_client_mtx.lock()
+        self.rcl_client_mtx.lock().unwrap()
     }
 }
 
 impl Drop for ClientHandle {
     fn drop(&mut self) {
-        let handle = self.rcl_client_mtx.get_mut();
-        let rcl_node_mtx = &mut *self.rcl_node_mtx.lock();
+        let rcl_client = self.rcl_client_mtx.get_mut().unwrap();
+        let rcl_node_mtx = &mut *self.rcl_node_mtx.lock().unwrap();
         // SAFETY: No preconditions for this function
         unsafe {
-            rcl_client_fini(handle, rcl_node_mtx);
+            rcl_client_fini(rcl_client, rcl_node_mtx);
         }
     }
 }
@@ -87,7 +86,7 @@ where
             err,
             s: topic.into(),
         })?;
-        let rcl_node = { &mut *node.rcl_node_mtx.lock() };
+        let rcl_node = { &mut *node.rcl_node_mtx.lock().unwrap() };
 
         // SAFETY: No preconditions for this function.
         let client_options = unsafe { rcl_client_get_default_options() };
@@ -153,7 +152,7 @@ where
             )
         }
         .ok()?;
-        let requests = &mut *self.requests.lock();
+        let requests = &mut *self.requests.lock().unwrap();
         requests.insert(sequence_number, Box::new(callback));
         Ok(())
     }
@@ -189,7 +188,7 @@ where
         }
         .ok()?;
         let (tx, rx) = oneshot::channel::<T::Response>();
-        self.futures.lock().insert(sequence_number, tx);
+        self.futures.lock().unwrap().insert(sequence_number, tx);
         // It is safe to call unwrap() here since the `Canceled` error will only happen when the
         // `Sender` is dropped
         // https://docs.rs/futures/latest/futures/channel/oneshot/struct.Canceled.html
@@ -261,8 +260,8 @@ where
             }
             Err(e) => return Err(e),
         };
-        let requests = &mut *self.requests.lock();
-        let futures = &mut *self.futures.lock();
+        let requests = &mut *self.requests.lock().unwrap();
+        let futures = &mut *self.futures.lock().unwrap();
         if let Some(callback) = requests.remove(&req_id.sequence_number) {
             callback(res);
         } else if let Some(future) = futures.remove(&req_id.sequence_number) {

rclrs/src/node/publisher.rs

@@ -7,9 +7,7 @@ use std::borrow::Cow;
 use std::ffi::CStr;
 use std::ffi::CString;
 use std::marker::PhantomData;
-use std::sync::Arc;
-
-use parking_lot::Mutex;
+use std::sync::{Arc, Mutex};
 
 use rosidl_runtime_rs::{Message, RmwMessage};
 
@@ -44,8 +42,8 @@ where
         unsafe {
             // SAFETY: No preconditions for this function (besides the arguments being valid).
             rcl_publisher_fini(
-                self.rcl_publisher_mtx.get_mut(),
-                &mut *self.rcl_node_mtx.lock(),
+                self.rcl_publisher_mtx.get_mut().unwrap(),
+                &mut *self.rcl_node_mtx.lock().unwrap(),
             );
         }
     }
@@ -70,7 +68,7 @@ where
             err,
             s: topic.into(),
         })?;
-        let rcl_node = &mut *node.rcl_node_mtx.lock();
+        let rcl_node = &mut *node.rcl_node_mtx.lock().unwrap();
 
         // SAFETY: No preconditions for this function.
         let mut publisher_options = unsafe { rcl_publisher_get_default_options() };
@@ -106,7 +104,8 @@ where
         // SAFETY: No preconditions for the functions called.
         // The unsafe variables created get converted to safe types before being returned
         unsafe {
-            let raw_topic_pointer = rcl_publisher_get_topic_name(&*self.rcl_publisher_mtx.lock());
+            let raw_topic_pointer =
+                rcl_publisher_get_topic_name(&*self.rcl_publisher_mtx.lock().unwrap());
             CStr::from_ptr(raw_topic_pointer)
                 .to_string_lossy()
                 .into_owned()
@@ -131,7 +130,7 @@ where
     /// [1]: https://github.com/ros2/ros2/issues/255
     pub fn publish<'a, M: MessageCow<'a, T>>(&self, message: M) -> Result<(), RclrsError> {
         let rmw_message = T::into_rmw_message(message.into_cow());
-        let rcl_publisher = &mut *self.rcl_publisher_mtx.lock();
+        let rcl_publisher = &mut *self.rcl_publisher_mtx.lock().unwrap();
         unsafe {
             // SAFETY: The message type is guaranteed to match the publisher type by the type system.
             // The message does not need to be valid beyond the duration of this function call.

rclrs/src/node/service.rs

@@ -1,7 +1,7 @@
 use std::boxed::Box;
 use std::ffi::CString;
 use std::sync::atomic::AtomicBool;
-use std::sync::Arc;
+use std::sync::{Arc, Mutex, MutexGuard};
 
 use crate::error::{RclReturnCode, ToResult};
 use crate::Node;
@@ -11,32 +11,30 @@ use rosidl_runtime_rs::Message;
 
 use crate::node::publisher::MessageCow;
 
-use parking_lot::{Mutex, MutexGuard};
-
 // 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_service_t {}
 
 /// Internal struct used by services.
 pub struct ServiceHandle {
-    handle: Mutex<rcl_service_t>,
-    node_handle: Arc<Mutex<rcl_node_t>>,
+    rcl_service_mtx: Mutex<rcl_service_t>,
+    rcl_node_mtx: Arc<Mutex<rcl_node_t>>,
     pub(crate) in_use_by_wait_set: Arc<AtomicBool>,
 }
 
 impl ServiceHandle {
     pub(crate) fn lock(&self) -> MutexGuard<rcl_service_t> {
-        self.handle.lock()
+        self.rcl_service_mtx.lock().unwrap()
     }
 }
 
 impl Drop for ServiceHandle {
     fn drop(&mut self) {
-        let handle = self.handle.get_mut();
-        let node_handle = &mut *self.node_handle.lock();
+        let rcl_service = self.rcl_service_mtx.get_mut().unwrap();
+        let rcl_node = &mut *self.rcl_node_mtx.lock().unwrap();
         // SAFETY: No preconditions for this function
         unsafe {
-            rcl_service_fini(handle, node_handle);
+            rcl_service_fini(rcl_service, rcl_node);
         }
     }
 }
@@ -80,14 +78,14 @@ where
         F: Fn(&rmw_request_id_t, T::Request) -> T::Response + 'static + Send,
     {
         // SAFETY: Getting a zero-initialized value is always safe.
-        let mut service_handle = unsafe { rcl_get_zero_initialized_service() };
+        let mut rcl_service = unsafe { rcl_get_zero_initialized_service() };
         let type_support = <T as rosidl_runtime_rs::Service>::get_type_support()
             as *const rosidl_service_type_support_t;
         let topic_c_string = CString::new(topic).map_err(|err| RclrsError::StringContainsNul {
             err,
             s: topic.into(),
         })?;
-        let node_handle = &mut *node.rcl_node_mtx.lock();
+        let rcl_node = &mut *node.rcl_node_mtx.lock().unwrap();
 
         // SAFETY: No preconditions for this function.
         let service_options = unsafe { rcl_service_get_default_options() };
@@ -98,8 +96,8 @@ where
             // The topic name and the options are copied by this function, so they can be dropped
             // afterwards.
             rcl_service_init(
-                &mut service_handle as *mut _,
-                node_handle as *mut _,
+                &mut rcl_service as *mut _,
+                rcl_node as *mut _,
                 type_support,
                 topic_c_string.as_ptr(),
                 &service_options as *const _,
@@ -108,8 +106,8 @@ where
         }
 
         let handle = Arc::new(ServiceHandle {
-            handle: Mutex::new(service_handle),
-            node_handle: node.rcl_node_mtx.clone(),
+            rcl_service_mtx: Mutex::new(rcl_service),
+            rcl_node_mtx: node.rcl_node_mtx.clone(),
             in_use_by_wait_set: Arc::new(AtomicBool::new(false)),
         });
 
@@ -184,7 +182,7 @@ where
             }
             Err(e) => return Err(e),
         };
-        let res = (*self.callback.lock())(&req_id, req);
+        let res = (*self.callback.lock().unwrap())(&req_id, req);
         let rmw_message = <T::Response as Message>::into_rmw_message(res.into_cow());
         let handle = &*self.handle.lock();
         unsafe {

rclrs/src/node/subscription.rs

@@ -8,12 +8,10 @@ use std::ffi::CStr;
 use std::ffi::CString;
 use std::marker::PhantomData;
 use std::sync::atomic::AtomicBool;
-use std::sync::Arc;
+use std::sync::{Arc, Mutex, MutexGuard};
 
 use rosidl_runtime_rs::{Message, RmwMessage};
 
-use parking_lot::{Mutex, MutexGuard};
-
 // 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_subscription_t {}
@@ -27,14 +25,14 @@ pub struct SubscriptionHandle {
 
 impl SubscriptionHandle {
     pub(crate) fn lock(&self) -> MutexGuard<rcl_subscription_t> {
-        self.rcl_subscription_mtx.lock()
+        self.rcl_subscription_mtx.lock().unwrap()
     }
 }
 
 impl Drop for SubscriptionHandle {
     fn drop(&mut self) {
-        let rcl_subscription = self.rcl_subscription_mtx.get_mut();
-        let rcl_node = &mut *self.rcl_node_mtx.lock();
+        let rcl_subscription = self.rcl_subscription_mtx.get_mut().unwrap();
+        let rcl_node = &mut *self.rcl_node_mtx.lock().unwrap();
         // SAFETY: No preconditions for this function (besides the arguments being valid).
         unsafe {
             rcl_subscription_fini(rcl_subscription, rcl_node);
@@ -99,7 +97,7 @@ where
             err,
             s: topic.into(),
         })?;
-        let rcl_node = &mut *node.rcl_node_mtx.lock();
+        let rcl_node = &mut *node.rcl_node_mtx.lock().unwrap();
 
         // SAFETY: No preconditions for this function.
         let mut subscription_options = unsafe { rcl_subscription_get_default_options() };
@@ -210,7 +208,7 @@ where
             }
             Err(e) => return Err(e),
         };
-        (*self.callback.lock())(msg);
+        (*self.callback.lock().unwrap())(msg);
         Ok(())
     }
 }

rclrs/src/parameter/value.rs

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