Test for Send and Sync implementations, and add missing implementations (#254)

Author: nnmm

rclrs/src/context.rs

@@ -120,6 +120,15 @@ impl Context {
 mod tests {
     use super::*;
 
+    fn assert_send<T: Send>() {}
+    fn assert_sync<T: Sync>() {}
+
+    #[test]
+    fn context_is_send_and_sync() {
+        assert_send::<Context>();
+        assert_sync::<Context>();
+    }
+
     #[test]
     fn test_create_context() -> Result<(), RclrsError> {
         // If the context fails to be created, this will cause a panic

rclrs/src/node.rs

@@ -328,3 +328,17 @@ unsafe fn call_string_getter_with_handle(
     let cstr = CStr::from_ptr(char_ptr);
     cstr.to_string_lossy().into_owned()
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn assert_send<T: Send>() {}
+    fn assert_sync<T: Sync>() {}
+
+    #[test]
+    fn node_is_send_and_sync() {
+        assert_send::<Node>();
+        assert_sync::<Node>();
+    }
+}

rclrs/src/publisher.rs

@@ -55,6 +55,13 @@ where
     }
 }
 
+// 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<T> Send for Publisher<T> where T: Message {}
+// SAFETY: The type_support_ptr prevents the default Sync impl.
+// rosidl_message_type_support_t is a read-only type without interior mutability.
+unsafe impl<T> Sync for Publisher<T> where T: Message {}
+
 impl<T> Publisher<T>
 where
     T: Message,

rclrs/src/publisher/loaned_message.rs

@@ -65,6 +65,12 @@ where
     }
 }
 
+// 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<'a, T> Send for LoanedMessage<'a, T> where T: RmwMessage {}
+// SAFETY: There is no interior mutability in this type. All mutation happens through &mut references.
+unsafe impl<'a, T> Sync for LoanedMessage<'a, T> where T: RmwMessage {}
+
 impl<'a, T> LoanedMessage<'a, T>
 where
     T: RmwMessage,

rclrs/src/subscription/readonly_loaned_message.rs

@@ -45,3 +45,9 @@ where
         }
     }
 }
+
+// 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<'a, T> Send for ReadOnlyLoanedMessage<'a, T> where T: RmwMessage {}
+// SAFETY: This type has no interior mutability, in fact it has no mutability at all.
+unsafe impl<'a, T> Sync for ReadOnlyLoanedMessage<'a, T> where T: RmwMessage {}

rclrs/src/wait.rs

@@ -59,6 +59,15 @@ impl Drop for rcl_wait_set_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_wait_set_t {}
+
+// SAFETY: While the rcl_wait_set_t does have some interior mutability (because it has
+// members of non-const pointer type), this interior mutability is hidden/not used by
+// the WaitSet type. Therefore, sharing &WaitSet between threads does not risk data races.
+unsafe impl Sync for WaitSet {}
+
 impl WaitSet {
     /// Creates a new wait set.
     ///
@@ -287,3 +296,17 @@ impl WaitSet {
         Ok(ready_entities)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn assert_send<T: Send>() {}
+    fn assert_sync<T: Sync>() {}
+
+    #[test]
+    fn wait_set_is_send_and_sync() {
+        assert_send::<WaitSet>();
+        assert_sync::<WaitSet>();
+    }
+}

rclrs_tests/src/client_service_tests.rs

@@ -0,0 +1,16 @@
+use rclrs::{Client, Service};
+
+fn assert_send<T: Send>() {}
+fn assert_sync<T: Sync>() {}
+
+#[test]
+fn client_is_send_and_sync() {
+    assert_send::<Client<test_msgs::srv::Arrays>>();
+    assert_sync::<Client<test_msgs::srv::Arrays>>();
+}
+
+#[test]
+fn service_is_send_and_sync() {
+    assert_send::<Service<test_msgs::srv::Arrays>>();
+    assert_sync::<Service<test_msgs::srv::Arrays>>();
+}

rclrs_tests/src/lib.rs

@@ -1,3 +1,5 @@
 #![cfg(test)]
 
+mod client_service_tests;
 mod graph_tests;
+mod pub_sub_tests;

rclrs_tests/src/pub_sub_tests.rs

@@ -0,0 +1,28 @@
+use rclrs::{LoanedMessage, Publisher, ReadOnlyLoanedMessage, Subscription};
+
+fn assert_send<T: Send>() {}
+fn assert_sync<T: Sync>() {}
+
+#[test]
+fn publisher_is_send_and_sync() {
+    assert_send::<Publisher<test_msgs::msg::BoundedSequences>>();
+    assert_sync::<Publisher<test_msgs::msg::BoundedSequences>>();
+}
+
+#[test]
+fn subscription_is_send_and_sync() {
+    assert_send::<Subscription<test_msgs::msg::BoundedSequences>>();
+    assert_sync::<Subscription<test_msgs::msg::BoundedSequences>>();
+}
+
+#[test]
+fn loaned_message_is_send_and_sync() {
+    assert_send::<LoanedMessage<test_msgs::msg::rmw::BoundedSequences>>();
+    assert_sync::<LoanedMessage<test_msgs::msg::rmw::BoundedSequences>>();
+}
+
+#[test]
+fn readonly_loaned_message_is_send_and_sync() {
+    assert_send::<ReadOnlyLoanedMessage<test_msgs::msg::rmw::BoundedSequences>>();
+    assert_sync::<ReadOnlyLoanedMessage<test_msgs::msg::rmw::BoundedSequences>>();
+}