From 52ed08cf9563b6b52e5991acd43614448b089850 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 25 Apr 2022 16:38:56 +0200 Subject: [PATCH 1/4] executor: remove useless not_send in SendSpwaner. --- embassy/src/executor/spawner.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/embassy/src/executor/spawner.rs b/embassy/src/executor/spawner.rs index ee0e60c2..d1e4ff17 100644 --- a/embassy/src/executor/spawner.rs +++ b/embassy/src/executor/spawner.rs @@ -105,7 +105,6 @@ impl Spawner { pub fn make_send(&self) -> SendSpawner { SendSpawner { executor: self.executor, - not_send: PhantomData, } } } @@ -120,7 +119,6 @@ impl Spawner { #[derive(Copy, Clone)] pub struct SendSpawner { executor: &'static raw::Executor, - not_send: PhantomData<*mut ()>, } unsafe impl Send for SendSpawner {} From b27feb061936d191f456edc22b2f89d4fc172520 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 25 Apr 2022 22:09:04 +0200 Subject: [PATCH 2/4] executor: fix unsoundness in InterruptExecutor::start. The initial closure is not actually called in the interrupt, so this is illegally sending non-Send futures to the interrupt. Remove the closure, and return a SendSpawner instead. --- embassy/src/executor/arch/cortex_m.rs | 20 ++++++++++---------- examples/nrf/src/bin/multiprio.rs | 10 ++++------ examples/stm32f3/src/bin/multiprio.rs | 10 ++++------ examples/stm32f4/src/bin/multiprio.rs | 10 ++++------ 4 files changed, 22 insertions(+), 28 deletions(-) diff --git a/embassy/src/executor/arch/cortex_m.rs b/embassy/src/executor/arch/cortex_m.rs index d23a595f..87c9c3c8 100644 --- a/embassy/src/executor/arch/cortex_m.rs +++ b/embassy/src/executor/arch/cortex_m.rs @@ -1,7 +1,7 @@ use core::marker::PhantomData; use core::ptr; -use super::{raw, Spawner}; +use super::{raw, SendSpawner, Spawner}; use crate::interrupt::{Interrupt, InterruptExt}; /// Thread mode executor, using WFE/SEV. @@ -107,13 +107,13 @@ impl InterruptExecutor { /// Start the executor. /// - /// The `init` closure is called from interrupt mode, with a [`Spawner`] that spawns tasks on - /// this executor. Use it to spawn the initial task(s). After `init` returns, - /// the interrupt is configured so that the executor starts running the tasks. - /// Once the executor is started, `start` returns. + /// This initializes the executor, configures and enables the interrupt, and returns. + /// The executor keeps running in the background through the interrupt. /// - /// To spawn more tasks later, you may keep copies of the [`Spawner`] (it is `Copy`), - /// for example by passing it as an argument to the initial tasks. + /// This returns a [`SendSpawner`] you can use to spawn tasks on it. A [`SendSpawner`] + /// is returned instead of a [`Spawner`] because the executor effectively runs in a + /// different "thread" (the interrupt), so spawning tasks on it is effectively + /// sending them. /// /// This function requires `&'static mut self`. This means you have to store the /// Executor instance in a place where it'll live forever and grants you mutable @@ -122,16 +122,16 @@ impl InterruptExecutor { /// - a [Forever](crate::util::Forever) (safe) /// - a `static mut` (unsafe) /// - a local variable in a function you know never returns (like `fn main() -> !`), upgrading its lifetime with `transmute`. (unsafe) - pub fn start(&'static mut self, init: impl FnOnce(Spawner) + Send) { + pub fn start(&'static mut self) -> SendSpawner { self.irq.disable(); - init(self.inner.spawner()); - self.irq.set_handler(|ctx| unsafe { let executor = &*(ctx as *const raw::Executor); executor.poll(); }); self.irq.set_handler_context(&self.inner as *const _ as _); self.irq.enable(); + + self.inner.spawner().make_send() } } diff --git a/examples/nrf/src/bin/multiprio.rs b/examples/nrf/src/bin/multiprio.rs index e69f87d8..54f6606a 100644 --- a/examples/nrf/src/bin/multiprio.rs +++ b/examples/nrf/src/bin/multiprio.rs @@ -124,17 +124,15 @@ fn main() -> ! { let irq = interrupt::take!(SWI1_EGU1); irq.set_priority(interrupt::Priority::P6); let executor = EXECUTOR_HIGH.put(InterruptExecutor::new(irq)); - executor.start(|spawner| { - unwrap!(spawner.spawn(run_high())); - }); + let spawner = executor.start(); + unwrap!(spawner.spawn(run_high())); // Medium-priority executor: SWI0_EGU0, priority level 7 let irq = interrupt::take!(SWI0_EGU0); irq.set_priority(interrupt::Priority::P7); let executor = EXECUTOR_MED.put(InterruptExecutor::new(irq)); - executor.start(|spawner| { - unwrap!(spawner.spawn(run_med())); - }); + let spawner = executor.start(); + unwrap!(spawner.spawn(run_med())); // Low priority executor: runs in thread mode, using WFE/SEV let executor = EXECUTOR_LOW.put(Executor::new()); diff --git a/examples/stm32f3/src/bin/multiprio.rs b/examples/stm32f3/src/bin/multiprio.rs index 1c940154..02380de7 100644 --- a/examples/stm32f3/src/bin/multiprio.rs +++ b/examples/stm32f3/src/bin/multiprio.rs @@ -124,17 +124,15 @@ fn main() -> ! { let irq = interrupt::take!(UART4); irq.set_priority(interrupt::Priority::P6); let executor = EXECUTOR_HIGH.put(InterruptExecutor::new(irq)); - executor.start(|spawner| { - unwrap!(spawner.spawn(run_high())); - }); + let spawner = executor.start(); + unwrap!(spawner.spawn(run_high())); // Medium-priority executor: SWI0_EGU0, priority level 7 let irq = interrupt::take!(UART5); irq.set_priority(interrupt::Priority::P7); let executor = EXECUTOR_MED.put(InterruptExecutor::new(irq)); - executor.start(|spawner| { - unwrap!(spawner.spawn(run_med())); - }); + let spawner = executor.start(); + unwrap!(spawner.spawn(run_med())); // Low priority executor: runs in thread mode, using WFE/SEV let executor = EXECUTOR_LOW.put(Executor::new()); diff --git a/examples/stm32f4/src/bin/multiprio.rs b/examples/stm32f4/src/bin/multiprio.rs index 1c940154..02380de7 100644 --- a/examples/stm32f4/src/bin/multiprio.rs +++ b/examples/stm32f4/src/bin/multiprio.rs @@ -124,17 +124,15 @@ fn main() -> ! { let irq = interrupt::take!(UART4); irq.set_priority(interrupt::Priority::P6); let executor = EXECUTOR_HIGH.put(InterruptExecutor::new(irq)); - executor.start(|spawner| { - unwrap!(spawner.spawn(run_high())); - }); + let spawner = executor.start(); + unwrap!(spawner.spawn(run_high())); // Medium-priority executor: SWI0_EGU0, priority level 7 let irq = interrupt::take!(UART5); irq.set_priority(interrupt::Priority::P7); let executor = EXECUTOR_MED.put(InterruptExecutor::new(irq)); - executor.start(|spawner| { - unwrap!(spawner.spawn(run_med())); - }); + let spawner = executor.start(); + unwrap!(spawner.spawn(run_med())); // Low priority executor: runs in thread mode, using WFE/SEV let executor = EXECUTOR_LOW.put(Executor::new()); From c4cecec10ce262d605e65b47a4772d3b7ec3a543 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 25 Apr 2022 22:18:52 +0200 Subject: [PATCH 3/4] macros: isolate the TAIT into its own mod. This fixes type inference issues due to the TAIT's defining scope being the whole parent mod. --- embassy-macros/src/macros/task.rs | 67 +++++++++++++++---------------- 1 file changed, 33 insertions(+), 34 deletions(-) diff --git a/embassy-macros/src/macros/task.rs b/embassy-macros/src/macros/task.rs index 0cf6b423..c37ad212 100644 --- a/embassy-macros/src/macros/task.rs +++ b/embassy-macros/src/macros/task.rs @@ -10,12 +10,10 @@ struct Args { #[darling(default)] pool_size: Option, #[darling(default)] - send: bool, - #[darling(default)] embassy_prefix: ModulePrefix, } -pub fn run(args: syn::AttributeArgs, mut f: syn::ItemFn) -> Result { +pub fn run(args: syn::AttributeArgs, f: syn::ItemFn) -> Result { let args = Args::from_list(&args).map_err(|e| e.write_errors())?; let embassy_prefix = args.embassy_prefix.append("embassy"); @@ -35,24 +33,27 @@ pub fn run(args: syn::AttributeArgs, mut f: syn::ItemFn) -> Result = - syn::punctuated::Punctuated::new(); + let mut arg_types = Vec::new(); + let mut arg_names = Vec::new(); + let mut arg_indexes = Vec::new(); let mut fargs = f.sig.inputs.clone(); - for arg in fargs.iter_mut() { + for (i, arg) in fargs.iter_mut().enumerate() { match arg { syn::FnArg::Receiver(_) => { ctxt.error_spanned_by(arg, "task functions must not have receiver arguments"); } syn::FnArg::Typed(t) => match t.pat.as_mut() { - syn::Pat::Ident(i) => { - arg_names.push(i.ident.clone()); - i.mutability = None; + syn::Pat::Ident(id) => { + arg_names.push(id.ident.clone()); + arg_types.push(t.ty.clone()); + arg_indexes.push(syn::Index::from(i)); + id.mutability = None; } _ => { ctxt.error_spanned_by( arg, - "pattern matching in task arguments is not yet supporteds", + "pattern matching in task arguments is not yet supported", ); } }, @@ -63,40 +64,38 @@ pub fn run(args: syn::AttributeArgs, mut f: syn::ItemFn) -> Result #spawn_token<#future_ident> { - #f + mod #mod_ident { + use #embassy_path::executor::SpawnToken; + use #embassy_path::executor::raw::TaskStorage; + + type Fut = impl ::core::future::Future + 'static; - #[allow(non_upper_case_globals)] #[allow(clippy::declare_interior_mutable_const)] - const #new_ts_ident: #task_storage<#future_ident> = #task_storage::new(); + const NEW_TS: TaskStorage = TaskStorage::new(); - #[allow(non_upper_case_globals)] - static #pool_ident: [#task_storage<#future_ident>; #pool_size] = [#new_ts_ident; #pool_size]; + static POOL: [TaskStorage; #pool_size] = [NEW_TS; #pool_size]; - unsafe { #task_storage::spawn_pool(&#pool_ident, move || #task_inner_ident(#arg_names)) } + pub(super) fn task(args: super::#args_ident) -> SpawnToken { + unsafe { TaskStorage::spawn_pool(&POOL, move || super::#task_inner_ident(#(args.#arg_indexes),*)) } + } + } + + #visibility fn #task_ident(#fargs) -> #embassy_path::executor::SpawnToken { + #mod_ident::task((#(#arg_names,)*)) } }; From 2b0e8a330b0f2be7a8943a9e5acadf8fc7f92275 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 25 Apr 2022 22:19:40 +0200 Subject: [PATCH 4/4] examples/nrf: add self_spawn example. This serves as a compile-test of possible typecheck loops due to TAIT shenanigans. --- examples/nrf/src/bin/self_spawn.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 examples/nrf/src/bin/self_spawn.rs diff --git a/examples/nrf/src/bin/self_spawn.rs b/examples/nrf/src/bin/self_spawn.rs new file mode 100644 index 00000000..35e73a8d --- /dev/null +++ b/examples/nrf/src/bin/self_spawn.rs @@ -0,0 +1,24 @@ +#![no_std] +#![no_main] +#![feature(type_alias_impl_trait)] + +use defmt::{info, unwrap}; +use embassy::executor::Spawner; +use embassy::time::{Duration, Timer}; +use embassy_nrf::Peripherals; + +use defmt_rtt as _; // global logger +use panic_probe as _; + +#[embassy::task(pool_size = 2)] +async fn my_task(spawner: Spawner, n: u32) { + Timer::after(Duration::from_secs(1)).await; + info!("Spawning self! {}", n); + unwrap!(spawner.spawn(my_task(spawner, n + 1))); +} + +#[embassy::main] +async fn main(spawner: Spawner, _p: Peripherals) { + info!("Hello World!"); + unwrap!(spawner.spawn(my_task(spawner, 0))); +}