Add SyncCell type to support Sync fix in embassy upstream

embedded-service/src/activity.rs

@@ -2,7 +2,7 @@
 
 use embassy_sync::once_lock::OnceLock;
 
-use crate::intrusive_list::*;
+use crate::{intrusive_list::*, sync_cell::SyncCell};
 
 /// potential activity service states
 #[derive(Copy, Clone, Debug)]
@@ -53,15 +53,15 @@ pub trait ActivitySubscriber {
 /// actual subscriber node instance for embedding within static or singleton type T
 pub struct Subscriber {
     node: Node,
-    instance: Cell<Option<&'static dyn ActivitySubscriber>>,
+    instance: SyncCell<Option<&'static dyn ActivitySubscriber>>,
 }
 
 impl Subscriber {
     /// use this when static initialization occurs, internal fields will be validated in register_subscriber() later
     pub const fn uninit() -> Self {
         Self {
             node: Node::uninit(),
-            instance: Cell::new(None),
+            instance: SyncCell::new(None),
         }
     }
 

embedded-service/src/intrusive_list.rs

@@ -2,7 +2,8 @@
 
 // Any type used for dynamic type coercion
 pub use core::any::Any;
-pub use core::cell::Cell;
+
+use crate::sync_cell::SyncCell;
 
 /// Interface error class information
 #[derive(Copy, Clone, Debug)]
@@ -29,7 +30,7 @@ pub struct IntrusiveNode {
 
 /// node type for list allocation. Embed this in the "list wrapper" object, and init with Node::uninit()
 pub struct Node {
-    inner: Cell<IntrusiveNode>,
+    inner: SyncCell<IntrusiveNode>,
 }
 
 struct Invalid {}
@@ -47,7 +48,7 @@ impl Node {
     /// construct an uninitialized node in place
     pub const fn uninit() -> Node {
         Node {
-            inner: Cell::new(Node::EMPTY),
+            inner: SyncCell::new(Node::EMPTY),
         }
     }
 }
@@ -61,7 +62,7 @@ pub trait NodeContainer: Any {
 /// List of intruded nodes of unknown type(s), must be allocated statically
 pub struct IntrusiveList {
     /// traditional head pointer on list. Static reference type is used to ensure static allocations (for safety)
-    head: Cell<Option<&'static IntrusiveNode>>,
+    head: SyncCell<Option<&'static IntrusiveNode>>,
 }
 
 impl IntrusiveNode {
@@ -94,7 +95,9 @@ impl Default for IntrusiveList {
 impl IntrusiveList {
     /// construct an empty intrusive list
     pub const fn new() -> IntrusiveList {
-        IntrusiveList { head: Cell::new(None) }
+        IntrusiveList {
+            head: SyncCell::new(None),
+        }
     }
 
     /// only allow pushing to the head of the list
@@ -213,12 +216,12 @@ mod test {
 
     struct RegistrationA {
         node: Node,
-        owner: Cell<Option<&'static dyn OpA>>,
+        owner: SyncCell<Option<&'static dyn OpA>>,
     }
 
     struct RegistrationB {
         node: Node,
-        owner: Cell<Option<&'static dyn OpB>>,
+        owner: SyncCell<Option<&'static dyn OpB>>,
     }
 
     impl NodeContainer for RegistrationA {
@@ -250,7 +253,7 @@ mod test {
         fn new() -> Self {
             Self {
                 node: Node::uninit(),
-                owner: Cell::new(None),
+                owner: SyncCell::new(None),
             }
         }
 
@@ -267,7 +270,7 @@ mod test {
         fn new() -> Self {
             Self {
                 node: Node::uninit(),
-                owner: Cell::new(None),
+                owner: SyncCell::new(None),
             }
         }
 
@@ -594,4 +597,9 @@ mod test {
         assert_eq!(A.len(), list.iter_only::<RegistrationA>().count());
         assert_eq!(B.len(), list.iter_only::<RegistrationB>().count());
     }
+
+    #[test]
+    fn test_static_alloc() {
+        static _LIST: IntrusiveList = IntrusiveList::new();
+    }
 }

embedded-service/src/lib.rs

@@ -6,6 +6,8 @@
 pub mod intrusive_list;
 pub use intrusive_list::*;
 
+pub mod sync_cell;
+
 /// short-hand include all pre-baked services
 pub mod activity;
 pub mod buffer;

embedded-service/src/sync_cell.rs

@@ -0,0 +1,201 @@
+//! # SyncCell: a cell-like API for static interior mutability scenarios. Backed by a critical section, implying it's usage may delay or defer interrupts. Recommended to use sparingly.
+use core::cell::Cell;
+
+/// A critical section backed Cell for sync scenarios where you want Cell behaviors, but need it to be thread safe (such as used in statics). Backed by a critical section, use sparingly.
+pub struct SyncCell<T: ?Sized> {
+    inner: Cell<T>,
+}
+
+impl<T> SyncCell<T> {
+    /// Constructs a SyncCell, initializing it with initial_value
+    pub const fn new(initial_value: T) -> Self {
+        Self {
+            inner: Cell::new(initial_value),
+        }
+    }
+
+    /// Sets the cell's content in a critical section. Note that this accounts
+    /// for read/write conditions but does not automatically handle logical data
+    /// race conditions. It is still possible for a user to read a value but have
+    /// it change after they've performed the read. This just ensures data integrity:
+    /// SyncCell<T> will always contain a valid T, even if it's been read "late"
+    pub fn set(&self, value: T) {
+        critical_section::with(|_cs| self.inner.set(value))
+    }
+
+    /// Swap contents between two SyncCell's    
+    /// # Panics
+    ///
+    /// This function will panic if `self` and `other` are different `Cell`s that partially overlap.
+    /// (Using just standard library methods, it is impossible to create such partially overlapping `Cell`s.
+    /// However, unsafe code is allowed to e.g. create two `&Cell<[i32; 2]>` that partially overlap.)
+    pub fn swap(&self, other: &Self) {
+        critical_section::with(|_cs| self.inner.swap(&other.inner));
+    }
+
+    /// consume the Synccell and return the inner value T
+    pub fn into_inner(self) -> T {
+        self.inner.into_inner()
+    }
+}
+
+impl<T: Copy> SyncCell<T> {
+    /// Reads the cell's content (in a critical section) and returns a copy
+    pub fn get(&self) -> T {
+        critical_section::with(|_cs| self.inner.get())
+    }
+}
+
+impl<T: ?Sized> SyncCell<T> {
+    /// Return an address to the backing type
+    /// Unsafe: allows reads and writes without critical section guard, violating Sync guarantees.
+    /// # Safety
+    /// This may be used safely if and only if the pointer is held during a critical section, or
+    /// all accessors to this Cell are blocked until the pointer is released.
+    pub const fn as_ptr(&self) -> *mut T {
+        self.inner.as_ptr()
+    }
+}
+
+impl<T: Default> SyncCell<T> {
+    /// consume the inner T, returning its value and replacing it with default()
+    pub fn take(&self) -> T {
+        critical_section::with(|_cs| self.inner.take())
+    }
+}
+
+// SAFETY: Sync is implemented here for SyncCell as T is only accessed via nestable critical sections
+unsafe impl<T> Sync for SyncCell<T> {}
+
+// SAFETY: Can implement send here due to critical section without T being explicitly Send
+unsafe impl<T> Send for SyncCell<T> where T: Send {}
+
+impl<T: Copy> Clone for SyncCell<T> {
+    #[inline]
+    fn clone(&self) -> SyncCell<T> {
+        SyncCell::new(self.get())
+    }
+}
+
+impl<T: Default> Default for SyncCell<T> {
+    /// Creates a `Cell<T>`, with the `Default` value for T.
+    #[inline]
+    fn default() -> SyncCell<T> {
+        SyncCell::new(Default::default())
+    }
+}
+
+impl<T: PartialOrd + Copy> PartialOrd for SyncCell<T> {
+    #[inline]
+    fn partial_cmp(&self, other: &SyncCell<T>) -> Option<core::cmp::Ordering> {
+        self.get().partial_cmp(&other.get())
+    }
+
+    #[inline]
+    fn lt(&self, other: &SyncCell<T>) -> bool {
+        self.get() < other.get()
+    }
+
+    #[inline]
+    fn le(&self, other: &SyncCell<T>) -> bool {
+        self.get() <= other.get()
+    }
+
+    #[inline]
+    fn gt(&self, other: &SyncCell<T>) -> bool {
+        self.get() > other.get()
+    }
+
+    #[inline]
+    fn ge(&self, other: &SyncCell<T>) -> bool {
+        self.get() >= other.get()
+    }
+}
+
+impl<T: PartialEq + Copy> PartialEq for SyncCell<T> {
+    #[inline]
+    fn eq(&self, other: &SyncCell<T>) -> bool {
+        self.get() == other.get()
+    }
+}
+
+impl<T: Eq + Copy> Eq for SyncCell<T> {}
+
+impl<T: Ord + Copy> Ord for SyncCell<T> {
+    #[inline]
+    fn cmp(&self, other: &SyncCell<T>) -> core::cmp::Ordering {
+        self.get().cmp(&other.get())
+    }
+}
+
+impl<T> From<T> for SyncCell<T> {
+    /// Creates a new `SyncCell<T>` containing the given value.
+    fn from(t: T) -> SyncCell<T> {
+        SyncCell::new(t)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_empty() {
+        let sc = SyncCell::<()>::new(());
+
+        assert_eq!(sc.get(), ());
+        sc.set(());
+        assert_eq!(sc.get(), ());
+    }
+
+    #[test]
+    fn test_primitive() {
+        let sc = SyncCell::new(0usize);
+
+        assert_eq!(sc.get(), 0);
+        sc.set(1);
+        assert_eq!(sc.get(), 1);
+    }
+
+    #[test]
+    fn test_struct() {
+        #[derive(Copy, Clone, PartialEq, Debug)]
+        struct Example {
+            a: u32,
+            b: u32,
+        }
+
+        let sc = SyncCell::new(Example { a: 0, b: 0 });
+
+        assert_eq!(sc.get(), Example { a: 0, b: 0 });
+        sc.set(Example { a: 1, b: 2 });
+        assert_eq!(sc.get(), Example { a: 1, b: 2 });
+    }
+
+    #[tokio::test]
+    async fn test_across_threads() {
+        static SC: SyncCell<bool> = SyncCell::new(false);
+        let scr = &SC;
+
+        let poller = tokio::spawn(async {
+            loop {
+                if scr.get() {
+                    break;
+                } else {
+                    let _ = tokio::time::sleep(tokio::time::Duration::from_millis(100)).await;
+                }
+            }
+        });
+
+        let updater = tokio::spawn(async {
+            let _ = tokio::time::sleep(tokio::time::Duration::from_millis(300));
+            scr.set(true);
+        });
+
+        let result = tokio::join!(poller, updater);
+        assert!(result.0.is_ok());
+        assert!(result.1.is_ok());
+
+        assert!(SC.get());
+    }
+}