Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support shared futures on no_std #2868

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

adavis628
Copy link

Currently, Shared futures are only available when the std feature is enabled because they use std::sync::Mutex internally. This PR changes this so that Shared futures will fall back to using a spinlock when std is not enabled.

Alternatively, Shared could be changed to be generic over a mutex trait (e.g. lock_api::RawMutex, though it isn't implemented for std::sync::Mutex) to allow arbitrary user implementations.

@taiki-e
Copy link
Member

taiki-e commented Oct 2, 2024

Thanks for the PR. I would prefer not to use spinlock by default around operations that would involve allocations even if it is no_std-only.

That said, it might be fine if this is an optional feature and spinlock is used only when std is disabled and the feature is explicitly enabled.

@taiki-e taiki-e added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author A-future Area: futures::future labels Oct 2, 2024
@adavis628
Copy link
Author

I moved the spinlock behind an optional spin feature that will use a spinlock when std is disabled.

futures-util/Cargo.toml Outdated Show resolved Hide resolved
@@ -107,9 +107,9 @@ mod remote_handle;
#[cfg(feature = "std")]
pub use self::remote_handle::{Remote, RemoteHandle};

#[cfg(feature = "std")]
#[cfg(any(feature = "std", feature = "spin"))]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be #[cfg(any(feature = "std", all(feature = "alloc", feature = "spin")))]

https://github.com/rust-lang/futures-rs/actions/runs/12735209519/job/35493701034?pr=2868#step:5:199

running `cargo check --no-default-features --features unstable,spin` on futures (24/193)
      Checking futures-util v0.4.0-alpha.0 (/home/runner/work/futures-rs/futures-rs/futures-util)
  error[E0433]: failed to resolve: use of undeclared crate or module `alloc`
   --> futures-util/src/future/future/shared.rs:2:5
    |
  2 | use alloc::sync::{Arc, Weak};
    |     ^^^^^ use of undeclared crate or module `alloc`
    |
    = help: add `extern crate alloc` to use the `alloc` crate
  
  error[E0432]: unresolved imports `crate::task::waker_ref`, `crate::task::ArcWake`
    --> futures-util/src/future/future/shared.rs:1:19
     |
  1  | use crate::task::{waker_ref, ArcWake};
     |                   ^^^^^^^^^  ^^^^^^^ no `ArcWake` in `task`
     |                   |
     |                   no `waker_ref` in `task`
     |
  note: found an item that was configured out
    --> futures-util/src/task/mod.rs:31:24
     |
  31 | pub use futures_task::{waker_ref, WakerRef};
     |                        ^^^^^^^^^
  note: the item is gated behind the `alloc` feature
    --> futures-util/src/task/mod.rs:30:7
     |
  30 | #[cfg(feature = "alloc")]
     |       ^^^^^^^^^^^^^^^^^
  note: found an item that was configured out
    --> futures-util/src/task/mod.rs:23:23
     |
  23 | pub use futures_task::ArcWake;
     |                       ^^^^^^^
  note: the item is gated behind the `alloc` feature
    --> futures-util/src/task/mod.rs:22:7
     |
  22 | #[cfg(feature = "alloc")]
     |       ^^^^^^^^^^^^^^^^^
  
  error[E0432]: unresolved import `slab`
    --> futures-util/src/future/future/shared.rs:12:5
     |
  12 | use slab::Slab;
     |     ^^^^ use of undeclared crate or module `slab`
 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-future Area: futures::future S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants