From 0a73c84df0936facecb3e1a97cf6f4795d321b87 Mon Sep 17 00:00:00 2001 From: Dániel Buga Date: Mon, 21 Aug 2023 13:55:30 +0200 Subject: Make AvailableTask public, deduplicate --- embassy-executor/CHANGELOG.md | 2 + embassy-executor/src/raw/mod.rs | 113 ++++++++++++++++++++++------------------ embassy-executor/src/spawner.rs | 3 +- 3 files changed, 67 insertions(+), 51 deletions(-) diff --git a/embassy-executor/CHANGELOG.md b/embassy-executor/CHANGELOG.md index e2e7bce3a..43d94e540 100644 --- a/embassy-executor/CHANGELOG.md +++ b/embassy-executor/CHANGELOG.md @@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased - Replaced Pender. Implementations now must define an extern function called `__pender`. +- Made `raw::AvailableTask` public +- Made `SpawnToken::new_failed` public ## 0.2.1 - 2023-08-10 diff --git a/embassy-executor/src/raw/mod.rs b/embassy-executor/src/raw/mod.rs index 7caa3302f..c1d82e18a 100644 --- a/embassy-executor/src/raw/mod.rs +++ b/embassy-executor/src/raw/mod.rs @@ -147,10 +147,7 @@ impl TaskStorage { pub fn spawn(&'static self, future: impl FnOnce() -> F) -> SpawnToken { let task = AvailableTask::claim(self); match task { - Some(task) => { - let task = task.initialize(future); - unsafe { SpawnToken::::new(task) } - } + Some(task) => task.initialize(future), None => SpawnToken::new_failed(), } } @@ -186,12 +183,16 @@ impl TaskStorage { } } -struct AvailableTask { +/// An uninitialized [`TaskStorage`]. +pub struct AvailableTask { task: &'static TaskStorage, } impl AvailableTask { - fn claim(task: &'static TaskStorage) -> Option { + /// Try to claim a [`TaskStorage`]. + /// + /// This function returns `None` if a task has already been spawned and has not finished running. + pub fn claim(task: &'static TaskStorage) -> Option { task.raw .state .compare_exchange(0, STATE_SPAWNED | STATE_RUN_QUEUED, Ordering::AcqRel, Ordering::Acquire) @@ -199,12 +200,56 @@ impl AvailableTask { .map(|_| Self { task }) } - fn initialize(self, future: impl FnOnce() -> F) -> TaskRef { + fn initialize_impl(self, future: impl FnOnce() -> F) -> SpawnToken { unsafe { self.task.raw.poll_fn.set(Some(TaskStorage::::poll)); self.task.future.write(future()); + + let task = TaskRef::new(self.task); + + SpawnToken::new(task) } - TaskRef::new(self.task) + } + + /// Initialize the [`TaskStorage`] to run the given future. + pub fn initialize(self, future: impl FnOnce() -> F) -> SpawnToken { + self.initialize_impl::(future) + } + + /// Initialize the [`TaskStorage`] to run the given future. + /// + /// # 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 __initialize_async_fn(self, future: impl FnOnce() -> F) -> SpawnToken { + // When send-spawning a task, we construct the future in this thread, and effectively + // "send" it to the executor thread by enqueuing it in its queue. Therefore, in theory, + // send-spawning should require the future `F` to be `Send`. + // + // The problem is this is more restrictive than needed. Once the future is executing, + // it is never sent to another thread. It is only sent when spawning. It should be + // enough for the task's arguments to be Send. (and in practice it's super easy to + // accidentally make your futures !Send, for example by holding an `Rc` or a `&RefCell` across an `.await`.) + // + // We can do it by sending the task args and constructing the future in the executor thread + // on first poll. However, this cannot be done in-place, so it'll waste stack space for a copy + // of the args. + // + // Luckily, an `async fn` future contains just the args when freshly constructed. So, if the + // args are Send, it's OK to send a !Send future, as long as we do it before first polling it. + // + // (Note: this is how the generators are implemented today, it's not officially guaranteed yet, + // but it's possible it'll be guaranteed in the future. See zulip thread: + // https://rust-lang.zulipchat.com/#narrow/stream/187312-wg-async/topic/.22only.20before.20poll.22.20Send.20futures ) + // + // The `FutFn` captures all the args, so if it's Send, the task can be send-spawned. + // This is why we return `SpawnToken` below. + // + // This ONLY holds for `async fn` futures. The other `spawn` methods can be called directly + // by the user, with arbitrary hand-implemented futures. This is why these return `SpawnToken`. + self.initialize_impl::(future) } } @@ -223,6 +268,13 @@ impl TaskPool { } } + fn spawn_impl(&'static self, future: impl FnOnce() -> F) -> SpawnToken { + match self.pool.iter().find_map(AvailableTask::claim) { + Some(task) => task.initialize_impl::(future), + None => SpawnToken::new_failed(), + } + } + /// Try to spawn a task in the pool. /// /// See [`TaskStorage::spawn()`] for details. @@ -231,14 +283,7 @@ impl TaskPool { /// 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 { - let task = self.pool.iter().find_map(AvailableTask::claim); - match task { - Some(task) => { - let task = task.initialize(future); - unsafe { SpawnToken::::new(task) } - } - None => SpawnToken::new_failed(), - } + self.spawn_impl::(future) } /// Like spawn(), but allows the task to be send-spawned if the args are Send even if @@ -254,40 +299,8 @@ impl TaskPool { where FutFn: FnOnce() -> F, { - // When send-spawning a task, we construct the future in this thread, and effectively - // "send" it to the executor thread by enqueuing it in its queue. Therefore, in theory, - // send-spawning should require the future `F` to be `Send`. - // - // The problem is this is more restrictive than needed. Once the future is executing, - // it is never sent to another thread. It is only sent when spawning. It should be - // enough for the task's arguments to be Send. (and in practice it's super easy to - // accidentally make your futures !Send, for example by holding an `Rc` or a `&RefCell` across an `.await`.) - // - // We can do it by sending the task args and constructing the future in the executor thread - // on first poll. However, this cannot be done in-place, so it'll waste stack space for a copy - // of the args. - // - // Luckily, an `async fn` future contains just the args when freshly constructed. So, if the - // args are Send, it's OK to send a !Send future, as long as we do it before first polling it. - // - // (Note: this is how the generators are implemented today, it's not officially guaranteed yet, - // but it's possible it'll be guaranteed in the future. See zulip thread: - // https://rust-lang.zulipchat.com/#narrow/stream/187312-wg-async/topic/.22only.20before.20poll.22.20Send.20futures ) - // - // The `FutFn` captures all the args, so if it's Send, the task can be send-spawned. - // This is why we return `SpawnToken` below. - // - // This ONLY holds for `async fn` futures. The other `spawn` methods can be called directly - // by the user, with arbitrary hand-implemented futures. This is why these return `SpawnToken`. - - let task = self.pool.iter().find_map(AvailableTask::claim); - match task { - Some(task) => { - let task = task.initialize(future); - unsafe { SpawnToken::::new(task) } - } - None => SpawnToken::new_failed(), - } + // See the comment in AvailableTask::__initialize_async_fn for explanation. + self.spawn_impl::(future) } } diff --git a/embassy-executor/src/spawner.rs b/embassy-executor/src/spawner.rs index 2b6224045..5a3a0dee1 100644 --- a/embassy-executor/src/spawner.rs +++ b/embassy-executor/src/spawner.rs @@ -33,7 +33,8 @@ impl SpawnToken { } } - pub(crate) fn new_failed() -> Self { + /// Return a SpawnToken that represents a failed spawn. + pub fn new_failed() -> Self { Self { raw_task: None, phantom: PhantomData, -- cgit