diff --git a/rclrs/Cargo.toml b/rclrs/Cargo.toml index e58d3e91d..769b8065c 100644 --- a/rclrs/Cargo.toml +++ b/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 diff --git a/rclrs/src/context.rs b/rclrs/src/context.rs index c0116dd7b..781bfc782 100644 --- a/rclrs/src/context.rs +++ b/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) } } diff --git a/rclrs/src/lib.rs b/rclrs/src/lib.rs index 0f4f2e3e9..ca63e919b 100644 --- a/rclrs/src/lib.rs +++ b/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) { diff --git a/rclrs/src/node.rs b/rclrs/src/node.rs index 48852be38..611cecace 100644 --- a/rclrs/src/node.rs +++ b/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. diff --git a/rclrs/src/node/builder.rs b/rclrs/src/node/builder.rs index 47cf177f2..2af0982f1 100644 --- a/rclrs/src/node/builder.rs +++ b/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() }; diff --git a/rclrs/src/node/client.rs b/rclrs/src/node/client.rs index b8c46d2e0..08d5f6577 100644 --- a/rclrs/src/node/client.rs +++ b/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 { - 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::(); - 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) { diff --git a/rclrs/src/node/publisher.rs b/rclrs/src/node/publisher.rs index aa33a077f..34db5ac19 100644 --- a/rclrs/src/node/publisher.rs +++ b/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. diff --git a/rclrs/src/node/service.rs b/rclrs/src/node/service.rs index 5603149dd..41191e182 100644 --- a/rclrs/src/node/service.rs +++ b/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, - node_handle: Arc>, + rcl_service_mtx: Mutex, + rcl_node_mtx: Arc>, pub(crate) in_use_by_wait_set: Arc, } impl ServiceHandle { pub(crate) fn lock(&self) -> MutexGuard { - 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 = ::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 = ::into_rmw_message(res.into_cow()); let handle = &*self.handle.lock(); unsafe { diff --git a/rclrs/src/node/subscription.rs b/rclrs/src/node/subscription.rs index 5f94c99f4..1dbcea548 100644 --- a/rclrs/src/node/subscription.rs +++ b/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 { - 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(()) } } diff --git a/rclrs/src/parameter/value.rs b/rclrs/src/parameter/value.rs index 022af204e..f0d69652b 100644 --- a/rclrs/src/parameter/value.rs +++ b/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()?; diff --git a/rclrs/src/wait.rs b/rclrs/src/wait.rs index b8d173343..162d1ae80 100644 --- a/rclrs/src/wait.rs +++ b/rclrs/src/wait.rs @@ -19,12 +19,10 @@ use crate::error::{to_rclrs_result, RclReturnCode, RclrsError, ToResult}; use crate::rcl_bindings::*; use crate::{ClientBase, Context, ServiceBase, SubscriptionBase}; -use std::sync::Arc; +use std::sync::{Arc, Mutex}; use std::time::Duration; use std::vec::Vec; -use parking_lot::Mutex; - mod exclusivity_guard; use exclusivity_guard::*; @@ -88,7 +86,7 @@ impl WaitSet { number_of_clients, number_of_services, number_of_events, - &mut *context.rcl_context_mtx.lock(), + &mut *context.rcl_context_mtx.lock().unwrap(), rcutils_get_default_allocator(), ) .ok()?;