Skip to content

Make all the things Send, and messages Sync as well #171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions rclrs/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ impl Drop for rcl_context_t {
}
}

// 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_context_t {}

/// Shared state between nodes and similar entities.
///
/// It is possible, but not usually necessary, to have several contexts in an application.
Expand Down
6 changes: 5 additions & 1 deletion rclrs/src/node/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ impl Drop for rcl_node_t {
}
}

// 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_node_t {}

/// A processing unit that can communicate with other nodes.
///
/// Nodes are a core concept in ROS 2. Refer to the official ["Understanding ROS 2 nodes"][1]
Expand Down Expand Up @@ -263,7 +267,7 @@ impl Node {
) -> Result<Arc<Subscription<T>>, RclrsError>
where
T: Message,
F: FnMut(T) + 'static,
F: FnMut(T) + 'static + Send,
{
let subscription = Arc::new(Subscription::<T>::new(self, topic, qos, callback)?);
self.subscriptions
Expand Down
4 changes: 4 additions & 0 deletions rclrs/src/node/publisher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ use parking_lot::{Mutex, MutexGuard};

use rosidl_runtime_rs::{Message, RmwMessage};

// 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_publisher_t {}

pub(crate) struct PublisherHandle {
handle: Mutex<rcl_publisher_t>,
node_handle: Arc<Mutex<rcl_node_t>>,
Expand Down
10 changes: 7 additions & 3 deletions rclrs/src/node/subscription.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ 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 {}

/// Internal struct used by subscriptions.
pub struct SubscriptionHandle {
handle: Mutex<rcl_subscription_t>,
Expand All @@ -37,7 +41,7 @@ impl Drop for SubscriptionHandle {
}

/// Trait to be implemented by concrete [`Subscription`]s.
pub trait SubscriptionBase {
pub trait SubscriptionBase: Send + Sync {
/// Internal function to get a reference to the `rcl` handle.
fn handle(&self) -> &SubscriptionHandle;
/// Tries to take a new message and run the callback with it.
Expand All @@ -61,7 +65,7 @@ where
{
pub(crate) handle: Arc<SubscriptionHandle>,
/// The callback function that runs when a message was received.
pub callback: Mutex<Box<dyn FnMut(T) + 'static>>,
pub callback: Mutex<Box<dyn FnMut(T) + 'static + Send>>,
message: PhantomData<T>,
}

Expand All @@ -81,7 +85,7 @@ where
) -> Result<Self, RclrsError>
where
T: Message,
F: FnMut(T) + 'static,
F: FnMut(T) + 'static + Send,
{
// SAFETY: Getting a zero-initialized value is always safe.
let mut subscription_handle = unsafe { rcl_get_zero_initialized_subscription() };
Expand Down
5 changes: 5 additions & 0 deletions rosidl_runtime_rs/src/sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,11 @@ impl<T: SequenceAlloc + PartialOrd> PartialOrd for Sequence<T> {
}
}

// SAFETY: A sequence is a simple data structure, and therefore not thread-specific.
unsafe impl<T: Send + SequenceAlloc> Send for Sequence<T> {}
// SAFETY: A sequence does not have interior mutability, so it can be shared.
unsafe impl<T: Sync + SequenceAlloc> Sync for Sequence<T> {}

impl<T> Sequence<T>
where
T: SequenceAlloc,
Expand Down
5 changes: 5 additions & 0 deletions rosidl_runtime_rs/src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,11 @@ macro_rules! string_impl {
}
}

// SAFETY: A string is a simple data structure, and therefore not thread-specific.
unsafe impl Send for $string {}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgive the potentially silly question, but I am not familiar with the $string syntax. What does it mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is inside a macro. $string can be either the String or WString type.

// SAFETY: A string does not have interior mutability, so it can be shared.
unsafe impl Sync for $string {}

impl SequenceAlloc for $string {
fn sequence_init(seq: &mut Sequence<Self>, size: libc::size_t) -> bool {
// SAFETY: There are no special preconditions to the sequence_init function.
Expand Down
4 changes: 2 additions & 2 deletions rosidl_runtime_rs/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub trait SequenceAlloc: Sized {
/// used by user code.
///
/// User code never needs to call this trait's method, much less implement this trait.
pub trait RmwMessage: Clone + Debug + Default {
pub trait RmwMessage: Clone + Debug + Default + Send + Sync {
/// Get a pointer to the correct `rosidl_message_type_support_t` structure.
fn get_type_support() -> libc::uintptr_t;
}
Expand Down Expand Up @@ -126,7 +126,7 @@ pub trait RmwMessage: Clone + Debug + Default {
/// problem, since nothing is allocated this way.
/// The `Drop` impl for any sequence or string will call `fini()`.

pub trait Message: Clone + Debug + Default + 'static {
pub trait Message: Clone + Debug + Default + 'static + Send + Sync {
/// The corresponding RMW-native message type.
type RmwMsg: RmwMessage;

Expand Down