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

Change ArcWake::into_waker to a free function #1676

Merged
merged 2 commits into from Jul 15, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions futures-test/src/task/wake_counter.rs
@@ -1,7 +1,7 @@
use futures_core::task::{Waker};
use futures_core::task::Waker;
use futures_util::task::{self, ArcWake};
use std::sync::Arc;
use std::sync::atomic::{AtomicUsize, Ordering};
use futures_util::task::ArcWake;

/// Number of times the waker was awoken.
///
Expand Down Expand Up @@ -55,5 +55,5 @@ impl ArcWake for WakerInner {
/// ```
pub fn new_count_waker() -> (Waker, AwokenCount) {
let inner = Arc::new(WakerInner { count: AtomicUsize::new(0) });
(ArcWake::into_waker(inner.clone()), AwokenCount { inner })
(task::waker(inner.clone()), AwokenCount { inner })
}
16 changes: 8 additions & 8 deletions futures-util/src/compat/compat03as01.rs
Expand Up @@ -7,17 +7,17 @@ use futures_01::{
AsyncSink as AsyncSink01, Sink as Sink01, StartSend as StartSend01,
};
use futures_core::{
task::{
self as task03,
RawWaker,
RawWakerVTable,
},
task::{RawWaker, RawWakerVTable},
TryFuture as TryFuture03,
TryStream as TryStream03,
};
#[cfg(feature = "sink")]
use futures_sink::Sink as Sink03;
use crate::task::{ArcWake as ArcWake03, WakerRef};
use crate::task::{
self as task03,
ArcWake as ArcWake03,
WakerRef,
};
#[cfg(feature = "sink")]
use std::marker::PhantomData;
use std::{
Expand Down Expand Up @@ -85,7 +85,7 @@ impl<T, Item> CompatSink<T, Item> {
_phantom: PhantomData,
}
}

/// Get a reference to 0.3 Sink contained within.
pub fn get_ref(&self) -> &T {
&self.inner
Expand Down Expand Up @@ -192,7 +192,7 @@ impl Current {
// FIXME: remove `transmute` when a `Waker` -> `RawWaker` conversion
// function is landed in `core`.
mem::transmute::<task03::Waker, RawWaker>(
Arc::new(ptr_to_current(ptr).clone()).into_waker()
task03::waker(Arc::new(ptr_to_current(ptr).clone()))
)
}
unsafe fn drop(_: *const ()) {}
Expand Down
71 changes: 17 additions & 54 deletions futures-util/src/task/arc_wake.rs
@@ -1,24 +1,33 @@
use core::mem;
use core::task::{Waker, RawWaker, RawWakerVTable};
use alloc::sync::Arc;

/// A way of waking up a specific task.
///
/// By implementing this trait, types that are expected to be wrapped in an `Arc`
/// can be converted into `Waker` objects.
/// can be converted into [`Waker`] objects.
/// Those Wakers can be used to signal executors that a task it owns
/// is ready to be `poll`ed again.
///
/// Currently, there are two ways to convert `ArcWake` into [`Waker`]:
///
/// * [`waker`](crate::task::waker()) converts `Arc<impl ArcWake>` into [`Waker`].
/// * [`waker_ref`](crate::task::waker_ref()) converts `&Arc<impl ArcWake>` into [`WakerRef`] that
/// provides access to a [`&Waker`][`Waker`].
///
/// [`Waker`]: std::task::Waker
/// [`WakerRef`]: crate::task::WakerRef
// Note: Send + Sync required because `Arc<T>` doesn't automatically imply
// those bounds, but `Waker` implements them.
pub trait ArcWake: Send + Sync {
/// Indicates that the associated task is ready to make progress and should
/// be `poll`ed.
///
/// This function can be called from an arbitrary thread, including threads which
/// did not create the `ArcWake` based `Waker`.
/// did not create the `ArcWake` based [`Waker`].
///
/// Executors generally maintain a queue of "ready" tasks; `wake` should place
/// the associated task onto this queue.
///
/// [`Waker`]: std::task::Waker
fn wake(self: Arc<Self>) {
Self::wake_by_ref(&self)
}
Expand All @@ -27,60 +36,14 @@ pub trait ArcWake: Send + Sync {
/// be `poll`ed.
///
/// This function can be called from an arbitrary thread, including threads which
/// did not create the `ArcWake` based `Waker`.
/// did not create the `ArcWake` based [`Waker`].
///
/// Executors generally maintain a queue of "ready" tasks; `wake_by_ref` should place
/// the associated task onto this queue.
///
/// This function is similar to `wake`, but must not consume the provided data
/// This function is similar to [`wake`](ArcWake::wake), but must not consume the provided data
/// pointer.
fn wake_by_ref(arc_self: &Arc<Self>);

/// Creates a `Waker` from an Arc<T>, if T implements `ArcWake`.
///
/// If `wake()` is called on the returned `Waker`,
/// the `wake()` function that is defined inside this trait will get called.
fn into_waker(self: Arc<Self>) -> Waker where Self: Sized
{
let ptr = Arc::into_raw(self) as *const ();

unsafe {
Waker::from_raw(RawWaker::new(ptr, waker_vtable!(Self)))
}
}
}

// FIXME: panics on Arc::clone / refcount changes could wreak havoc on the
// code here. We should guard against this by aborting.

unsafe fn increase_refcount<T: ArcWake>(data: *const ()) {
// Retain Arc by creating a copy
let arc: Arc<T> = Arc::from_raw(data as *const T);
let arc_clone = arc.clone();
// Forget the Arcs again, so that the refcount isn't decrased
mem::forget(arc);
mem::forget(arc_clone);
}

// used by `waker_ref`
pub(super) unsafe fn clone_arc_raw<T: ArcWake>(data: *const ()) -> RawWaker {
increase_refcount::<T>(data);
RawWaker::new(data, waker_vtable!(T))
}

unsafe fn drop_arc_raw<T: ArcWake>(data: *const ()) {
drop(Arc::<T>::from_raw(data as *const T))
}

// used by `waker_ref`
pub(super) unsafe fn wake_arc_raw<T: ArcWake>(data: *const ()) {
let arc: Arc<T> = Arc::from_raw(data as *const T);
ArcWake::wake(arc);
}

// used by `waker_ref`
pub(super) unsafe fn wake_by_ref_arc_raw<T: ArcWake>(data: *const ()) {
let arc: Arc<T> = Arc::from_raw(data as *const T);
ArcWake::wake_by_ref(&arc);
mem::forget(arc);
/// [`Waker`]: std::task::Waker
fn wake_by_ref(arc_self: &Arc<Self>);
}
5 changes: 5 additions & 0 deletions futures-util/src/task/mod.rs
Expand Up @@ -20,6 +20,11 @@ cfg_target_has_atomic! {
#[cfg(feature = "alloc")]
pub use self::arc_wake::ArcWake;

#[cfg(feature = "alloc")]
mod waker;
#[cfg(feature = "alloc")]
pub use self::waker::waker;

#[cfg(feature = "alloc")]
mod waker_ref;
#[cfg(feature = "alloc")]
Expand Down
4 changes: 2 additions & 2 deletions futures-util/src/task/noop_waker.rs
Expand Up @@ -16,7 +16,7 @@ fn noop_raw_waker() -> RawWaker {
RawWaker::new(null(), &NOOP_WAKER_VTABLE)
}

/// Create a new [`Waker`](futures_core::task::Waker) which does
/// Create a new [`Waker`] which does
/// nothing when `wake()` is called on it.
///
/// # Examples
Expand All @@ -33,7 +33,7 @@ pub fn noop_waker() -> Waker {
}
}

/// Get a static reference to a [`Waker`](futures_core::task::Waker) which
/// Get a static reference to a [`Waker`] which
/// does nothing when `wake()` is called on it.
///
/// # Examples
Expand Down
53 changes: 53 additions & 0 deletions futures-util/src/task/waker.rs
@@ -0,0 +1,53 @@
use super::arc_wake::ArcWake;
use core::mem;
use core::task::{Waker, RawWaker, RawWakerVTable};
use alloc::sync::Arc;

/// Creates a [`Waker`] from an `Arc<impl ArcWake>`.
///
/// The returned [`Waker`] will call
/// [`ArcWake.wake()`](ArcWake::wake) if awoken.
pub fn waker<W>(wake: Arc<W>) -> Waker
where
W: ArcWake,
{
let ptr = Arc::into_raw(wake) as *const ();

unsafe {
Waker::from_raw(RawWaker::new(ptr, waker_vtable!(W)))
}
}

// FIXME: panics on Arc::clone / refcount changes could wreak havoc on the
// code here. We should guard against this by aborting.

unsafe fn increase_refcount<T: ArcWake>(data: *const ()) {
// Retain Arc by creating a copy
let arc: Arc<T> = Arc::from_raw(data as *const T);
let arc_clone = arc.clone();
// Forget the Arcs again, so that the refcount isn't decrased
mem::forget(arc);
mem::forget(arc_clone);
}

// used by `waker_ref`
pub(super) unsafe fn clone_arc_raw<T: ArcWake>(data: *const ()) -> RawWaker {
increase_refcount::<T>(data);
RawWaker::new(data, waker_vtable!(T))
}

unsafe fn wake_arc_raw<T: ArcWake>(data: *const ()) {
let arc: Arc<T> = Arc::from_raw(data as *const T);
ArcWake::wake(arc);
}

// used by `waker_ref`
pub(super) unsafe fn wake_by_ref_arc_raw<T: ArcWake>(data: *const ()) {
let arc: Arc<T> = Arc::from_raw(data as *const T);
ArcWake::wake_by_ref(&arc);
mem::forget(arc);
}

unsafe fn drop_arc_raw<T: ArcWake>(data: *const ()) {
drop(Arc::<T>::from_raw(data as *const T))
}
18 changes: 7 additions & 11 deletions futures-util/src/task/waker_ref.rs
@@ -1,17 +1,13 @@
#![allow(clippy::cast_ptr_alignment)] // clippy is too strict here

use super::arc_wake::{ArcWake, clone_arc_raw, wake_by_ref_arc_raw};
use super::arc_wake::ArcWake;
use super::waker::{clone_arc_raw, wake_by_ref_arc_raw};
use alloc::sync::Arc;
use core::marker::PhantomData;
use core::ops::Deref;
use core::task::{Waker, RawWaker, RawWakerVTable};

// TODO: The link to Waker below points to futures::task::Waker and not to std. Is that a
// bug in rustdoc?
//
/// A [`Waker`](::std::task::Waker) that is only valid for a given lifetime.
/// A [`Waker`] that is only valid for a given lifetime.
///
/// Note: this type implements [`Deref<Target = Waker>`](::std::ops::Deref),
/// Note: this type implements [`Deref<Target = Waker>`](std::ops::Deref),
/// so it can be used to get a `&Waker`.
#[derive(Debug)]
pub struct WakerRef<'a> {
Expand Down Expand Up @@ -56,10 +52,9 @@ unsafe fn wake_unreachable(_data: *const ()) {
unreachable!("WakerRef::wake");
}

/// Creates a reference to a [`Waker`](::std::task::Waker)
/// from a local [`ArcWake`].
/// Creates a reference to a [`Waker`] from a reference to `Arc<impl ArcWake>`.
///
/// The resulting [`Waker`](::std::task::Waker) will call
/// The resulting [`Waker`] will call
/// [`ArcWake.wake()`](ArcWake::wake) if awoken.
#[inline]
pub fn waker_ref<W>(wake: &Arc<W>) -> WakerRef<'_>
Expand All @@ -68,6 +63,7 @@ where
{
// This uses the same mechanism as Arc::into_raw, without needing a reference.
// This is potentially not stable
#![allow(clippy::cast_ptr_alignment)]
let ptr = &*wake as &W as *const W as *const ();

// Similar to `waker_vtable`, but with a no-op `drop` function.
Expand Down
2 changes: 1 addition & 1 deletion futures/src/lib.rs
Expand Up @@ -491,7 +491,7 @@ pub mod task {
cfg(all(target_has_atomic = "cas", target_has_atomic = "ptr"))
)]
#[cfg(feature = "alloc")]
pub use futures_util::task::{WakerRef, waker_ref, ArcWake};
pub use futures_util::task::{waker, waker_ref, WakerRef, ArcWake};

#[cfg_attr(
feature = "cfg-target-has-atomic",
Expand Down
4 changes: 2 additions & 2 deletions futures/tests/arc_wake.rs
@@ -1,4 +1,4 @@
use futures::task::{ArcWake, Waker};
use futures::task::{self, ArcWake, Waker};
use std::sync::{Arc, Mutex};

struct CountingWaker {
Expand Down Expand Up @@ -28,7 +28,7 @@ impl ArcWake for CountingWaker {
fn create_waker_from_arc() {
let some_w = Arc::new(CountingWaker::new());

let w1: Waker = ArcWake::into_waker(some_w.clone());
let w1: Waker = task::waker(some_w.clone());
assert_eq!(2, Arc::strong_count(&some_w));
w1.wake_by_ref();
assert_eq!(1, some_w.wakes());
Expand Down