Disallow respawning on different executor

embassy-executor/CHANGELOG.md

@@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+- `integrated-timers` disallows respawning a task on an other executor.
+- `Executor::spawn` is now fallible.
 - `raw::Executor` now has an `fn initialize` that must be called once before starting to poll it.
 
 ## 0.6.3 - 2024-11-12

embassy-executor/src/raw/mod.rs

@@ -38,7 +38,7 @@ use self::run_queue::{RunQueue, RunQueueItem};
 use self::state::State;
 use self::util::{SyncUnsafeCell, UninitCell};
 pub use self::waker::task_from_waker;
-use super::SpawnToken;
+use super::{SpawnError, SpawnToken};
 
 /// Raw task header for use in task pointers.
 ///
@@ -423,13 +423,23 @@ impl SyncExecutor {
         this.pender.pend();
     }
 
-    pub(super) unsafe fn spawn(&'static self, task: TaskRef) {
-        task.header().executor.set(Some(self));
+    pub(super) unsafe fn spawn(&'static self, task: TaskRef) -> Result<(), SpawnError> {
+        let executor = &task.header().executor;
+
+        #[cfg(feature = "integrated-timers")]
+        if let Some(executor) = executor.get() {
+            if core::ptr::from_ref(executor) != self {
+                return Err(SpawnError::BoundToDifferentExecutor);
+            }
+        }
+
+        executor.set(Some(self));
 
         #[cfg(feature = "rtos-trace")]
         trace::task_new(task.as_ptr() as u32);
 
         self.enqueue(task);
+        Ok(())
     }
 
     /// # Safety
@@ -570,7 +580,7 @@ impl Executor {
     /// It is OK to use `unsafe` to call this from a thread that's not the executor thread.
     /// In this case, the task's Future must be Send. This is because this is effectively
     /// sending the task to the executor thread.
-    pub(super) unsafe fn spawn(&'static self, task: TaskRef) {
+    pub(super) unsafe fn spawn(&'static self, task: TaskRef) -> Result<(), SpawnError> {
         self.inner.spawn(task)
     }
 
@@ -613,9 +623,6 @@ pub fn wake_task(task: TaskRef) {
     let header = task.header();
     if header.state.run_enqueue() {
         // We have just marked the task as scheduled, so enqueue it.
-        // FIXME: there is currently a data race between re-spawning a task and waking it using an
-        // old waker. If the task is being spawned on a different executor, then reading and writing
-        // the executor field may happen concurrently.
         unsafe {
             let executor = header.executor.get().unwrap_unchecked();
             executor.enqueue(task);
@@ -630,9 +637,6 @@ pub fn wake_task_no_pend(task: TaskRef) {
     let header = task.header();
     if header.state.run_enqueue() {
         // We have just marked the task as scheduled, so enqueue it.
-        // FIXME: there is currently a data race between re-spawning a task and waking it using an
-        // old waker. If the task is being spawned on a different executor, then reading and writing
-        // the executor field may happen concurrently.
         unsafe {
             let executor = header.executor.get().unwrap_unchecked();
             executor.run_queue.enqueue(task);

embassy-executor/src/spawner.rs

@@ -59,6 +59,13 @@ pub enum SpawnError {
     /// running at a time. You may allow multiple instances to run in parallel with
     /// `#[embassy_executor::task(pool_size = 4)]`, at the cost of higher RAM usage.
     Busy,
+
+    /// The task has already been assigned to an executor and can't be moved.
+    ///
+    /// When integrated timers are enabled, tasks must not move between executors. If you need
+    /// to move a task between executors
+    #[cfg(feature = "integrated-timers")]
+    BoundToDifferentExecutor,
 }
 
 /// Handle to spawn tasks into an executor.
@@ -107,10 +114,7 @@ impl Spawner {
         mem::forget(token);
 
         match task {
-            Some(task) => {
-                unsafe { self.executor.spawn(task) };
-                Ok(())
-            }
+            Some(task) => unsafe { self.executor.spawn(task) },
             None => Err(SpawnError::Busy),
         }
     }
@@ -178,10 +182,7 @@ impl SendSpawner {
         mem::forget(token);
 
         match header {
-            Some(header) => {
-                unsafe { self.executor.spawn(header) };
-                Ok(())
-            }
+            Some(header) => unsafe { self.executor.spawn(header) },
             None => Err(SpawnError::Busy),
         }
     }