executor: return error when creating the spawntoken, not when spawning.

8 files changed, 43 insertions, 91 deletions

diff --git a/embassy-executor/src/raw/mod.rs b/embassy-executor/src/raw/mod.rs
index a7e65360d..bdaa32951 100644
--- a/embassy-executor/src/raw/mod.rs
+++ b/embassy-executor/src/raw/mod.rs
@@ -41,7 +41,7 @@ use self::state::State;
 use self::util::{SyncUnsafeCell, UninitCell};
 pub use self::waker::task_from_waker;
 use super::SpawnToken;
-use crate::Metadata;
+use crate::{Metadata, SpawnError};
 
 #[no_mangle]
 extern "Rust" fn __embassy_time_queue_item_from_waker(waker: &Waker) -> &'static mut TimerQueueItem {
@@ -220,11 +220,11 @@ impl<F: Future + 'static> TaskStorage<F> {
     ///
     /// Once the task has finished running, you may spawn it again. It is allowed to spawn it
     /// on a different executor.
-    pub fn spawn(&'static self, future: impl FnOnce() -> F) -> SpawnToken<impl Sized> {
+    pub fn spawn(&'static self, future: impl FnOnce() -> F) -> Result<SpawnToken<impl Sized>, SpawnError> {
         let task = AvailableTask::claim(self);
         match task {
-            Some(task) => task.initialize(future),
-            None => SpawnToken::new_failed(),
+            Some(task) => Ok(task.initialize(future)),
+            None => Err(SpawnError::Busy),
         }
     }
 
@@ -353,10 +353,10 @@ impl<F: Future + 'static, const N: usize> TaskPool<F, N> {
         }
     }
 
-    fn spawn_impl<T>(&'static self, future: impl FnOnce() -> F) -> SpawnToken<T> {
+    fn spawn_impl<T>(&'static self, future: impl FnOnce() -> F) -> Result<SpawnToken<T>, SpawnError> {
         match self.pool.iter().find_map(AvailableTask::claim) {
-            Some(task) => task.initialize_impl::<T>(future),
-            None => SpawnToken::new_failed(),
+            Some(task) => Ok(task.initialize_impl::<T>(future)),
+            None => Err(SpawnError::Busy),
         }
     }
 
@@ -367,7 +367,7 @@ impl<F: Future + 'static, const N: usize> TaskPool<F, N> {
     /// This will loop over the pool and spawn the task in the first storage that
     /// is currently free. If none is free, a "poisoned" SpawnToken is returned,
     /// which will cause [`Spawner::spawn()`](super::Spawner::spawn) to return the error.
-    pub fn spawn(&'static self, future: impl FnOnce() -> F) -> SpawnToken<impl Sized> {
+    pub fn spawn(&'static self, future: impl FnOnce() -> F) -> Result<SpawnToken<impl Sized>, SpawnError> {
         self.spawn_impl::<F>(future)
     }
 
@@ -380,7 +380,7 @@ impl<F: Future + 'static, const N: usize> TaskPool<F, N> {
     /// SAFETY: `future` must be a closure of the form `move || my_async_fn(args)`, where `my_async_fn`
     /// is an `async fn`, NOT a hand-written `Future`.
     #[doc(hidden)]
-    pub unsafe fn _spawn_async_fn<FutFn>(&'static self, future: FutFn) -> SpawnToken<impl Sized>
+    pub unsafe fn _spawn_async_fn<FutFn>(&'static self, future: FutFn) -> Result<SpawnToken<impl Sized>, SpawnError>
     where
         FutFn: FnOnce() -> F,
     {
diff --git a/embassy-executor/src/spawner.rs b/embassy-executor/src/spawner.rs
index cd2113a28..83d896b76 100644
--- a/embassy-executor/src/spawner.rs
+++ b/embassy-executor/src/spawner.rs
@@ -23,39 +23,28 @@ use crate::Metadata;
 /// Once you've invoked a task function and obtained a SpawnToken, you *must* spawn it.
 #[must_use = "Calling a task function does nothing on its own. You must spawn the returned SpawnToken, typically with Spawner::spawn()"]
 pub struct SpawnToken<S> {
-    pub(crate) raw_task: Option<raw::TaskRef>,
+    pub(crate) raw_task: raw::TaskRef,
     phantom: PhantomData<*mut S>,
 }
 
 impl<S> SpawnToken<S> {
     pub(crate) unsafe fn new(raw_task: raw::TaskRef) -> Self {
         Self {
-            raw_task: Some(raw_task),
+            raw_task,
             phantom: PhantomData,
         }
     }
 
-    /// Return a SpawnToken that represents a failed spawn.
-    pub fn new_failed() -> Self {
-        Self {
-            raw_task: None,
-            phantom: PhantomData,
-        }
-    }
-
-    /// Returns the task ID if available, otherwise 0
+    /// Returns the task ID.
     /// This can be used in combination with rtos-trace to match task names with IDs
     pub fn id(&self) -> u32 {
-        match self.raw_task {
-            None => 0,
-            Some(t) => t.id(),
-        }
+        self.raw_task.id()
     }
 
     /// Get the metadata for this task. You can use this to set metadata fields
     /// prior to spawning it.
     pub fn metadata(&self) -> &Metadata {
-        self.raw_task.unwrap().metadata()
+        self.raw_task.metadata()
     }
 }
 
@@ -164,30 +153,10 @@ impl Spawner {
     /// Spawn a task into an executor.
     ///
     /// You obtain the `token` by calling a task function (i.e. one marked with `#[embassy_executor::task]`).
-    pub fn spawn<S>(&self, token: SpawnToken<S>) -> Result<(), SpawnError> {
+    pub fn spawn<S>(&self, token: SpawnToken<S>) {
         let task = token.raw_task;
         mem::forget(token);
-
-        match task {
-            Some(task) => {
-                unsafe { self.executor.spawn(task) };
-                Ok(())
-            }
-            None => Err(SpawnError::Busy),
-        }
-    }
-
-    // Used by the `embassy_executor_macros::main!` macro to throw an error when spawn
-    // fails. This is here to allow conditional use of `defmt::unwrap!`
-    // without introducing a `defmt` feature in the `embassy_executor_macros` package,
-    // which would require use of `-Z namespaced-features`.
-    /// Spawn a task into an executor, panicking on failure.
-    ///
-    /// # Panics
-    ///
-    /// Panics if the spawning fails.
-    pub fn must_spawn<S>(&self, token: SpawnToken<S>) {
-        unwrap!(self.spawn(token));
+        unsafe { self.executor.spawn(task) }
     }
 
     /// Convert this Spawner to a SendSpawner. This allows you to send the
@@ -245,25 +214,9 @@ impl SendSpawner {
     /// Spawn a task into an executor.
     ///
     /// You obtain the `token` by calling a task function (i.e. one marked with `#[embassy_executor::task]`).
-    pub fn spawn<S: Send>(&self, token: SpawnToken<S>) -> Result<(), SpawnError> {
+    pub fn spawn<S: Send>(&self, token: SpawnToken<S>) {
         let header = token.raw_task;
         mem::forget(token);
-
-        match header {
-            Some(header) => {
-                unsafe { self.executor.spawn(header) };
-                Ok(())
-            }
-            None => Err(SpawnError::Busy),
-        }
-    }
-
-    /// Spawn a task into an executor, panicking on failure.
-    ///
-    /// # Panics
-    ///
-    /// Panics if the spawning fails.
-    pub fn must_spawn<S: Send>(&self, token: SpawnToken<S>) {
-        unwrap!(self.spawn(token));
+        unsafe { self.executor.spawn(header) }
     }
 }
diff --git a/embassy-executor/tests/test.rs b/embassy-executor/tests/test.rs
index 530314ac3..85c5dc1d9 100644
--- a/embassy-executor/tests/test.rs
+++ b/embassy-executor/tests/test.rs
@@ -65,7 +65,7 @@ fn executor_task() {
     }
 
     let (executor, trace) = setup();
-    executor.spawner().spawn(task1(trace.clone())).unwrap();
+    executor.spawner().spawn(task1(trace.clone()).unwrap());
 
     unsafe { executor.poll() };
     unsafe { executor.poll() };
@@ -93,7 +93,7 @@ fn executor_task_rpit() {
     }
 
     let (executor, trace) = setup();
-    executor.spawner().spawn(task1(trace.clone())).unwrap();
+    executor.spawner().spawn(task1(trace.clone()).unwrap());
 
     unsafe { executor.poll() };
     unsafe { executor.poll() };
@@ -120,7 +120,7 @@ fn executor_task_self_wake() {
     }
 
     let (executor, trace) = setup();
-    executor.spawner().spawn(task1(trace.clone())).unwrap();
+    executor.spawner().spawn(task1(trace.clone()).unwrap());
 
     unsafe { executor.poll() };
     unsafe { executor.poll() };
@@ -152,7 +152,7 @@ fn executor_task_self_wake_twice() {
     }
 
     let (executor, trace) = setup();
-    executor.spawner().spawn(task1(trace.clone())).unwrap();
+    executor.spawner().spawn(task1(trace.clone()).unwrap());
 
     unsafe { executor.poll() };
     unsafe { executor.poll() };
@@ -188,7 +188,7 @@ fn waking_after_completion_does_not_poll() {
     let waker = Box::leak(Box::new(AtomicWaker::new()));
 
     let (executor, trace) = setup();
-    executor.spawner().spawn(task1(trace.clone(), waker)).unwrap();
+    executor.spawner().spawn(task1(trace.clone(), waker).unwrap());
 
     unsafe { executor.poll() };
     waker.wake();
@@ -200,7 +200,7 @@ fn waking_after_completion_does_not_poll() {
     unsafe { executor.poll() }; // Clears running status
 
     // Can respawn waken-but-dead task
-    executor.spawner().spawn(task1(trace.clone(), waker)).unwrap();
+    executor.spawner().spawn(task1(trace.clone(), waker).unwrap());
 
     unsafe { executor.poll() };
 
@@ -250,7 +250,7 @@ fn waking_with_old_waker_after_respawn() {
     let waker = Box::leak(Box::new(AtomicWaker::new()));
 
     let (executor, trace) = setup();
-    executor.spawner().spawn(task1(trace.clone(), waker)).unwrap();
+    executor.spawner().spawn(task1(trace.clone(), waker).unwrap());
 
     unsafe { executor.poll() };
     unsafe { executor.poll() }; // progress to registering the waker
@@ -273,8 +273,7 @@ fn waking_with_old_waker_after_respawn() {
     let (other_executor, other_trace) = setup();
     other_executor
         .spawner()
-        .spawn(task1(other_trace.clone(), waker))
-        .unwrap();
+        .spawn(task1(other_trace.clone(), waker).unwrap());
 
     unsafe { other_executor.poll() }; // just run to the yield_now
     waker.wake(); // trigger old waker registration
@@ -338,22 +337,22 @@ fn task_metadata() {
 
     // check no task name
     let (executor, _) = setup();
-    executor.spawner().spawn(task1(None)).unwrap();
+    executor.spawner().spawn(task1(None).unwrap());
     unsafe { executor.poll() };
 
     // check setting task name
-    let token = task1(Some("foo"));
+    let token = task1(Some("foo")).unwrap();
     token.metadata().set_name("foo");
-    executor.spawner().spawn(token).unwrap();
+    executor.spawner().spawn(token);
     unsafe { executor.poll() };
 
-    let token = task1(Some("bar"));
+    let token = task1(Some("bar")).unwrap();
     token.metadata().set_name("bar");
-    executor.spawner().spawn(token).unwrap();
+    executor.spawner().spawn(token);
     unsafe { executor.poll() };
 
     // check name is cleared if the task pool slot is recycled.
     let (executor, _) = setup();
-    executor.spawner().spawn(task1(None)).unwrap();
+    executor.spawner().spawn(task1(None).unwrap());
     unsafe { executor.poll() };
 }
diff --git a/embassy-executor/tests/ui/return_impl_future_nonsend.rs b/embassy-executor/tests/ui/return_impl_future_nonsend.rs
index b8c184b21..77b3119d6 100644
--- a/embassy-executor/tests/ui/return_impl_future_nonsend.rs
+++ b/embassy-executor/tests/ui/return_impl_future_nonsend.rs
@@ -15,7 +15,7 @@ fn task() -> impl Future<Output = ()> {
 }
 
 fn send_spawn(s: SendSpawner) {
-    s.spawn(task()).unwrap();
+    s.spawn(task().unwrap());
 }
 
 fn main() {}
diff --git a/embassy-executor/tests/ui/return_impl_future_nonsend.stderr b/embassy-executor/tests/ui/return_impl_future_nonsend.stderr
index 8aeb9738a..51944ad65 100644
--- a/embassy-executor/tests/ui/return_impl_future_nonsend.stderr
+++ b/embassy-executor/tests/ui/return_impl_future_nonsend.stderr
@@ -1,8 +1,8 @@
 error: future cannot be sent between threads safely
   --> tests/ui/return_impl_future_nonsend.rs:18:13
    |
-18 |     s.spawn(task()).unwrap();
-   |             ^^^^^^ future created by async block is not `Send`
+18 |     s.spawn(task().unwrap());
+   |             ^^^^^^^^^^^^^^^ future created by async block is not `Send`
    |
    = help: within `impl Sized`, the trait `Send` is not implemented for `*mut ()`
 note: captured value is not `Send`
@@ -13,5 +13,5 @@ note: captured value is not `Send`
 note: required by a bound in `SendSpawner::spawn`
   --> src/spawner.rs
    |
-   |     pub fn spawn<S: Send>(&self, token: SpawnToken<S>) -> Result<(), SpawnError> {
+   |     pub fn spawn<S: Send>(&self, token: SpawnToken<S>) {
    |                     ^^^^ required by this bound in `SendSpawner::spawn`
diff --git a/embassy-executor/tests/ui/return_impl_send.stderr b/embassy-executor/tests/ui/return_impl_send.stderr
index 759be1cde..5d19465ec 100644
--- a/embassy-executor/tests/ui/return_impl_send.stderr
+++ b/embassy-executor/tests/ui/return_impl_send.stderr
@@ -97,7 +97,7 @@ note: required by a bound in `TaskPool::<F, N>::spawn`
   | impl<F: Future + 'static, const N: usize> TaskPool<F, N> {
   |         ^^^^^^ required by this bound in `TaskPool::<F, N>::spawn`
 ...
-  |     pub fn spawn(&'static self, future: impl FnOnce() -> F) -> SpawnToken<impl Sized> {
+  |     pub fn spawn(&'static self, future: impl FnOnce() -> F) -> Result<SpawnToken<impl Sized>, SpawnError> {
   |            ----- required by a bound in this associated function
   = note: this error originates in the attribute macro `embassy_executor::task` (in Nightly builds, run with -Z macro-backtrace for more info)
 
diff --git a/embassy-executor/tests/ui/spawn_nonsend.rs b/embassy-executor/tests/ui/spawn_nonsend.rs
index 4c4cc7697..601041941 100644
--- a/embassy-executor/tests/ui/spawn_nonsend.rs
+++ b/embassy-executor/tests/ui/spawn_nonsend.rs
@@ -10,7 +10,7 @@ async fn task(non_send: *mut ()) {
 }
 
 fn send_spawn(s: SendSpawner) {
-    s.spawn(task(core::ptr::null_mut())).unwrap();
+    s.spawn(task(core::ptr::null_mut()).unwrap());
 }
 
 fn main() {}
diff --git a/embassy-executor/tests/ui/spawn_nonsend.stderr b/embassy-executor/tests/ui/spawn_nonsend.stderr
index 2a06c8b94..25bd7d78d 100644
--- a/embassy-executor/tests/ui/spawn_nonsend.stderr
+++ b/embassy-executor/tests/ui/spawn_nonsend.stderr
@@ -12,8 +12,8 @@ error[E0277]: `*mut ()` cannot be sent between threads safely
 7  | #[embassy_executor::task]
    | ------------------------- within this `impl Sized`
 ...
-13 |     s.spawn(task(core::ptr::null_mut())).unwrap();
-   |       ----- ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `*mut ()` cannot be sent between threads safely
+13 |     s.spawn(task(core::ptr::null_mut()).unwrap());
+   |       ----- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `*mut ()` cannot be sent between threads safely
    |       |
    |       required by a bound introduced by this call
    |
@@ -26,8 +26,8 @@ note: required because it's used within this closure
 note: required because it appears within the type `impl Sized`
   --> src/raw/mod.rs
    |
-   |     pub unsafe fn _spawn_async_fn<FutFn>(&'static self, future: FutFn) -> SpawnToken<impl Sized>
-   |                                                                                      ^^^^^^^^^^
+   |     pub unsafe fn _spawn_async_fn<FutFn>(&'static self, future: FutFn) -> Result<SpawnToken<impl Sized>, SpawnError>
+   |                                                                                             ^^^^^^^^^^
 note: required because it appears within the type `impl Sized`
   --> tests/ui/spawn_nonsend.rs:7:1
    |
@@ -36,6 +36,6 @@ note: required because it appears within the type `impl Sized`
 note: required by a bound in `SendSpawner::spawn`
   --> src/spawner.rs
    |
-   |     pub fn spawn<S: Send>(&self, token: SpawnToken<S>) -> Result<(), SpawnError> {
+   |     pub fn spawn<S: Send>(&self, token: SpawnToken<S>) {
    |                     ^^^^ required by this bound in `SendSpawner::spawn`
    = note: this error originates in the attribute macro `embassy_executor::task` (in Nightly builds, run with -Z macro-backtrace for more info)