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

Remove StreamObj, caution against using FutureObj #1494

Merged
merged 1 commit into from
Apr 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 10 additions & 11 deletions futures-core/src/future/future_obj.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ use core::{
/// A custom trait object for polling futures, roughly akin to
/// `Box<dyn Future<Output = T> + 'a>`.
///
/// This custom trait object was introduced for two reasons:
/// - Currently it is not possible to take `dyn Trait` by value and
/// `Box<dyn Trait>` is not available in no_std contexts.
/// This custom trait object was introduced as currently it is not possible to
/// take `dyn Trait` by value and `Box<dyn Trait>` is not available in no_std
/// contexts.
pub struct LocalFutureObj<'a, T> {
ptr: *mut (),
poll_fn: unsafe fn(*mut (), &mut Context<'_>) -> Poll<T>,
Expand Down Expand Up @@ -79,14 +79,13 @@ impl<T> Drop for LocalFutureObj<'_, T> {
/// A custom trait object for polling futures, roughly akin to
/// `Box<dyn Future<Output = T> + Send + 'a>`.
///
/// This custom trait object was introduced for two reasons:
/// - Currently it is not possible to take `dyn Trait` by value and
/// `Box<dyn Trait>` is not available in no_std contexts.
/// - The `Future` trait is currently not object safe: The `Future::poll`
/// method makes uses the arbitrary self types feature and traits in which
/// this feature is used are currently not object safe due to current compiler
/// limitations. (See tracking issue for arbitrary self types for more
/// information #44874)
/// This custom trait object was introduced as currently it is not possible to
/// take `dyn Trait` by value and `Box<dyn Trait>` is not available in no_std
/// contexts.
///
/// You should generally not need to use this type outside of `no_std` or when
/// implementing `Spawn`, consider using [`BoxFuture`](crate::future::BoxFuture)
/// instead.
pub struct FutureObj<'a, T>(LocalFutureObj<'a, T>);

impl<T> Unpin for FutureObj<'_, T> {}
Expand Down
5 changes: 5 additions & 0 deletions futures-core/src/future/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ pub use core::future::Future;
mod future_obj;
pub use self::future_obj::{FutureObj, LocalFutureObj, UnsafeFutureObj};

#[cfg(feature = "alloc")]
/// An owned dynamically typed [`Future`] for use in cases where you can't
/// statically type your result or need to add some indirection.
pub type BoxFuture<'a, T> = Pin<alloc::boxed::Box<dyn Future<Output = T> + Send + 'a>>;
Copy link
Member

Choose a reason for hiding this comment

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

I'd also like to see non-Send versions of these-- maybe BoxFutureLocal? unsure-- we can sort that out in a separate change :)

Copy link
Member

Choose a reason for hiding this comment

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

(similarly, adding a boxed_local would be great)


/// A `Future` or `TryFuture` which tracks whether or not the underlying future
/// should no longer be polled.
///
Expand Down
6 changes: 4 additions & 2 deletions futures-core/src/stream/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ use core::ops;
use core::pin::Pin;
use core::task::{Context, Poll};

mod stream_obj;
pub use self::stream_obj::{StreamObj,LocalStreamObj,UnsafeStreamObj};
#[cfg(feature = "alloc")]
/// An owned dynamically typed [`Stream`] for use in cases where you can't
/// statically type your result or need to add some indirection.
pub type BoxStream<'a, T> = Pin<alloc::boxed::Box<dyn Stream<Item = T> + Send + 'a>>;

/// A stream of values produced asynchronously.
///
Expand Down
258 changes: 0 additions & 258 deletions futures-core/src/stream/stream_obj.rs

This file was deleted.

6 changes: 4 additions & 2 deletions futures-util/src/future/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ use futures_core::stream::Stream;
use futures_core::task::{Context, Poll};
#[cfg(feature = "alloc")]
use alloc::boxed::Box;
#[cfg(feature = "alloc")]
use futures_core::future::BoxFuture;

// re-export for `select!`
#[doc(hidden)]
Expand Down Expand Up @@ -491,8 +493,8 @@ pub trait FutureExt: Future {

/// Wrap the future in a Box, pinning it.
#[cfg(feature = "alloc")]
fn boxed(self) -> Pin<Box<Self>>
where Self: Sized
fn boxed(self) -> BoxFuture<'static, Self::Output>
where Self: Sized + Send + 'static
{
Box::pin(self)
}
Expand Down
7 changes: 6 additions & 1 deletion futures/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,9 @@ pub mod future {
FutureObj, LocalFutureObj, UnsafeFutureObj,
};

#[cfg(feature = "alloc")]
pub use futures_core::future::BoxFuture;

pub use futures_util::future::{
empty, Empty,
lazy, Lazy,
Expand Down Expand Up @@ -335,9 +338,11 @@ pub mod stream {

pub use futures_core::stream::{
Stream, TryStream, FusedStream,
StreamObj, LocalStreamObj, UnsafeStreamObj,
};

#[cfg(feature = "alloc")]
pub use futures_core::stream::BoxStream;

pub use futures_util::stream::{
iter, Iter,
repeat, Repeat,
Expand Down
4 changes: 2 additions & 2 deletions futures/tests/future_obj.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use futures::task::{Context, Poll};

#[test]
fn dropping_does_not_segfault() {
FutureObj::new(async { String::new() }.boxed());
FutureObj::new(Box::new(async { String::new() }));
}

#[test]
Expand All @@ -29,7 +29,7 @@ fn dropping_drops_the_future() {
}
}

FutureObj::new(Inc(&mut times_dropped).boxed());
FutureObj::new(Box::new(Inc(&mut times_dropped)));

assert_eq!(times_dropped, 1);
}
Loading