From 1b7ad7080e6a8c96f2622d75b99a4d3bdbc7c394 Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Sat, 24 Jul 2021 12:53:57 +1000 Subject: [PATCH] Add `Send/Sync` bounds to `PeripheralState` --- embassy-extras/src/peripheral.rs | 18 +++++++++++++----- embassy-extras/src/peripheral_shared.rs | 17 ++++++++++++----- embassy-extras/src/usb/mod.rs | 24 ++++++++++++------------ embassy-nrf/src/timer.rs | 2 +- embassy-nrf/src/uarte.rs | 2 +- embassy/src/util/waker.rs | 5 +++++ 6 files changed, 44 insertions(+), 24 deletions(-) diff --git a/embassy-extras/src/peripheral.rs b/embassy-extras/src/peripheral.rs index edc5fe955..2358ab96f 100644 --- a/embassy-extras/src/peripheral.rs +++ b/embassy-extras/src/peripheral.rs @@ -5,18 +5,26 @@ use core::ptr; use embassy::interrupt::{Interrupt, InterruptExt}; +/// A version of `PeripheralState` without the `'static` bound, +/// for cases where the compiler can't statically make sure +/// that `on_interrupt` doesn't reference anything which might be invalidated. +/// /// # Safety /// When types implementing this trait are used with `PeripheralMutex`, /// no fields referenced by `on_interrupt`'s lifetimes must end without first calling `Drop` on the `PeripheralMutex`. -pub unsafe trait PeripheralStateUnchecked { +pub unsafe trait PeripheralStateUnchecked: Send { type Interrupt: Interrupt; fn on_interrupt(&mut self); } -// `PeripheralMutex` is safe because `Pin` guarantees that the memory it references will not be invalidated or reused -// without calling `Drop`. However, it provides no guarantees about references contained within the state still being valid, -// so this `'static` bound is necessary. -pub trait PeripheralState: 'static { +/// A type which can be used as state with `PeripheralMutex`. +/// +/// It needs to be `Send` because `&mut` references are sent back and forth between the 'thread' which owns the `PeripheralMutex` and the interrupt, +/// and `&mut T` is `Send` where `T: Send`. +/// +/// It also requires `'static`, because although `Pin` guarantees that the memory of the state won't be invalidated, +/// it doesn't guarantee that the lifetime will last. +pub trait PeripheralState: Send + 'static { type Interrupt: Interrupt; fn on_interrupt(&mut self); } diff --git a/embassy-extras/src/peripheral_shared.rs b/embassy-extras/src/peripheral_shared.rs index 820622bb9..5961441e0 100644 --- a/embassy-extras/src/peripheral_shared.rs +++ b/embassy-extras/src/peripheral_shared.rs @@ -4,18 +4,25 @@ use core::ptr; use embassy::interrupt::{Interrupt, InterruptExt}; +/// A version of `PeripheralState` without the `'static` bound, +/// for cases where the compiler can't statically make sure +/// that `on_interrupt` doesn't reference anything which might be invalidated. +/// /// # Safety /// When types implementing this trait are used with `Peripheral`, /// no fields referenced by `on_interrupt`'s lifetimes must end without first calling `Drop` on the `Peripheral`. -pub unsafe trait PeripheralStateUnchecked { +pub unsafe trait PeripheralStateUnchecked: Sync { type Interrupt: Interrupt; fn on_interrupt(&self); } -// `Peripheral` is safe because `Pin` guarantees that the memory it references will not be invalidated or reused -// without calling `Drop`. However, it provides no guarantees about references contained within the state still being valid, -// so this `'static` bound is necessary. -pub trait PeripheralState: 'static { +/// A type which can be used as state with `Peripheral`. +/// +/// It needs to be `Sync` because references are shared between the 'thread' which owns the `Peripheral` and the interrupt. +/// +/// It also requires `'static`, because although `Pin` guarantees that the memory of the state won't be invalidated, +/// it doesn't guarantee that the lifetime will last. +pub trait PeripheralState: Sync + 'static { type Interrupt: Interrupt; fn on_interrupt(&self); } diff --git a/embassy-extras/src/usb/mod.rs b/embassy-extras/src/usb/mod.rs index 330eb9220..481987a6c 100644 --- a/embassy-extras/src/usb/mod.rs +++ b/embassy-extras/src/usb/mod.rs @@ -14,7 +14,7 @@ use embassy::interrupt::Interrupt; use usb_serial::{ReadInterface, UsbSerial, WriteInterface}; /// Marker trait to mark an interrupt to be used with the [`Usb`] abstraction. -pub unsafe trait USBInterrupt: Interrupt {} +pub unsafe trait USBInterrupt: Interrupt + Send {} pub(crate) struct State<'bus, B, T, I> where @@ -140,7 +140,7 @@ where } } -pub trait ClassSet { +pub trait ClassSet: Send { fn poll_all(&mut self, device: &mut UsbDevice<'_, B>) -> bool; } @@ -176,8 +176,8 @@ pub struct Index1; impl ClassSet for ClassSet1 where - B: UsbBus, - C1: UsbClass, + B: UsbBus + Send, + C1: UsbClass + Send, { fn poll_all(&mut self, device: &mut UsbDevice<'_, B>) -> bool { device.poll(&mut [&mut self.class]) @@ -186,9 +186,9 @@ where impl ClassSet for ClassSet2 where - B: UsbBus, - C1: UsbClass, - C2: UsbClass, + B: UsbBus + Send, + C1: UsbClass + Send, + C2: UsbClass + Send, { fn poll_all(&mut self, device: &mut UsbDevice<'_, B>) -> bool { device.poll(&mut [&mut self.class1, &mut self.class2]) @@ -197,8 +197,8 @@ where impl IntoClassSet> for C1 where - B: UsbBus, - C1: UsbClass, + B: UsbBus + Send, + C1: UsbClass + Send, { fn into_class_set(self) -> ClassSet1 { ClassSet1 { @@ -210,9 +210,9 @@ where impl IntoClassSet> for (C1, C2) where - B: UsbBus, - C1: UsbClass, - C2: UsbClass, + B: UsbBus + Send, + C1: UsbClass + Send, + C2: UsbClass + Send, { fn into_class_set(self) -> ClassSet2 { ClassSet2 { diff --git a/embassy-nrf/src/timer.rs b/embassy-nrf/src/timer.rs index a6e91f228..7ff35c320 100644 --- a/embassy-nrf/src/timer.rs +++ b/embassy-nrf/src/timer.rs @@ -29,7 +29,7 @@ pub(crate) mod sealed { pub trait ExtendedInstance {} } -pub trait Instance: Unborrow + sealed::Instance + 'static { +pub trait Instance: Unborrow + sealed::Instance + 'static + Send { type Interrupt: Interrupt; } pub trait ExtendedInstance: Instance + sealed::ExtendedInstance {} diff --git a/embassy-nrf/src/uarte.rs b/embassy-nrf/src/uarte.rs index 67ec5d73f..985854a5f 100644 --- a/embassy-nrf/src/uarte.rs +++ b/embassy-nrf/src/uarte.rs @@ -461,7 +461,7 @@ pub(crate) mod sealed { } } -pub trait Instance: Unborrow + sealed::Instance + 'static { +pub trait Instance: Unborrow + sealed::Instance + 'static + Send { type Interrupt: Interrupt; } diff --git a/embassy/src/util/waker.rs b/embassy/src/util/waker.rs index 393155099..1ac6054f9 100644 --- a/embassy/src/util/waker.rs +++ b/embassy/src/util/waker.rs @@ -48,6 +48,11 @@ impl WakerRegistration { } } +// SAFETY: `WakerRegistration` effectively contains an `Option`, +// which is `Send` and `Sync`. +unsafe impl Send for WakerRegistration {} +unsafe impl Sync for WakerRegistration {} + pub struct AtomicWaker { waker: AtomicPtr, }