From d9a4b22d3291913a8f2158a1b7c195bc30c9286e Mon Sep 17 00:00:00 2001 From: Matthias Einwag Date: Tue, 29 Jan 2019 19:02:42 -0800 Subject: [PATCH 1/9] Update the future/task API This change updates the future and task API as discussed in the stabilization RFC at https://github.com/rust-lang/rfcs/pull/2592. Changes: - Replacing UnsafeWake with RawWaker and RawWakerVtable - Removal of LocalWaker - Removal of Arc-based Wake trait --- src/liballoc/boxed.rs | 6 +- src/liballoc/lib.rs | 4 - src/liballoc/task.rs | 130 -------------- src/libcore/future/future.rs | 36 ++-- src/libcore/task/mod.rs | 2 +- src/libcore/task/wake.rs | 316 +++++++++-------------------------- src/libstd/future.rs | 20 +-- src/libstd/lib.rs | 2 - src/libstd/panic.rs | 6 +- 9 files changed, 111 insertions(+), 411 deletions(-) delete mode 100644 src/liballoc/task.rs diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index 8e01e12e0b8de..51549f92d4dbf 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -71,7 +71,7 @@ use core::ops::{ CoerceUnsized, DispatchFromDyn, Deref, DerefMut, Receiver, Generator, GeneratorState }; use core::ptr::{self, NonNull, Unique}; -use core::task::{LocalWaker, Poll}; +use core::task::{Waker, Poll}; use crate::vec::Vec; use crate::raw_vec::RawVec; @@ -896,7 +896,7 @@ impl Generator for Pin> { impl Future for Box { type Output = F::Output; - fn poll(mut self: Pin<&mut Self>, lw: &LocalWaker) -> Poll { - F::poll(Pin::new(&mut *self), lw) + fn poll(mut self: Pin<&mut Self>, waker: &Waker) -> Poll { + F::poll(Pin::new(&mut *self), waker) } } diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index 80097a128a5f4..cc73e282b6f6f 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -133,10 +133,6 @@ mod macros; pub mod alloc; -#[unstable(feature = "futures_api", - reason = "futures in libcore are unstable", - issue = "50547")] -pub mod task; // Primitive types using the heaps above // Need to conditionally define the mod from `boxed.rs` to avoid diff --git a/src/liballoc/task.rs b/src/liballoc/task.rs deleted file mode 100644 index 2261dabe2779a..0000000000000 --- a/src/liballoc/task.rs +++ /dev/null @@ -1,130 +0,0 @@ -//! Types and Traits for working with asynchronous tasks. - -pub use core::task::*; - -#[cfg(all(target_has_atomic = "ptr", target_has_atomic = "cas"))] -pub use if_arc::*; - -#[cfg(all(target_has_atomic = "ptr", target_has_atomic = "cas"))] -mod if_arc { - use super::*; - use core::marker::PhantomData; - use core::mem; - use core::ptr::{self, NonNull}; - use crate::sync::Arc; - - /// A way of waking up a specific task. - /// - /// Any task executor must provide a way of signaling that a task it owns - /// is ready to be `poll`ed again. Executors do so by implementing this trait. - pub trait Wake: Send + Sync { - /// Indicates that the associated task is ready to make progress and should - /// be `poll`ed. - /// - /// Executors generally maintain a queue of "ready" tasks; `wake` should place - /// the associated task onto this queue. - fn wake(arc_self: &Arc); - - /// Indicates that the associated task is ready to make progress and should - /// be `poll`ed. This function is like `wake`, but can only be called from the - /// thread on which this `Wake` was created. - /// - /// Executors generally maintain a queue of "ready" tasks; `wake_local` should place - /// the associated task onto this queue. - #[inline] - unsafe fn wake_local(arc_self: &Arc) { - Self::wake(arc_self); - } - } - - #[cfg(all(target_has_atomic = "ptr", target_has_atomic = "cas"))] - struct ArcWrapped(PhantomData); - - unsafe impl UnsafeWake for ArcWrapped { - #[inline] - unsafe fn clone_raw(&self) -> Waker { - let me: *const ArcWrapped = self; - let arc = (*(&me as *const *const ArcWrapped as *const Arc)).clone(); - Waker::from(arc) - } - - #[inline] - unsafe fn drop_raw(&self) { - let mut me: *const ArcWrapped = self; - let me = &mut me as *mut *const ArcWrapped as *mut Arc; - ptr::drop_in_place(me); - } - - #[inline] - unsafe fn wake(&self) { - let me: *const ArcWrapped = self; - T::wake(&*(&me as *const *const ArcWrapped as *const Arc)) - } - - #[inline] - unsafe fn wake_local(&self) { - let me: *const ArcWrapped = self; - T::wake_local(&*(&me as *const *const ArcWrapped as *const Arc)) - } - } - - impl From> for Waker - where T: Wake + 'static, - { - fn from(rc: Arc) -> Self { - unsafe { - let ptr = mem::transmute::, NonNull>>(rc); - Waker::new(ptr) - } - } - } - - /// Creates a `LocalWaker` from a local `wake`. - /// - /// This function requires that `wake` is "local" (created on the current thread). - /// The resulting `LocalWaker` will call `wake.wake_local()` when awoken, and - /// will call `wake.wake()` if awoken after being converted to a `Waker`. - #[inline] - pub unsafe fn local_waker(wake: Arc) -> LocalWaker { - let ptr = mem::transmute::, NonNull>>(wake); - LocalWaker::new(ptr) - } - - struct NonLocalAsLocal(ArcWrapped); - - unsafe impl UnsafeWake for NonLocalAsLocal { - #[inline] - unsafe fn clone_raw(&self) -> Waker { - self.0.clone_raw() - } - - #[inline] - unsafe fn drop_raw(&self) { - self.0.drop_raw() - } - - #[inline] - unsafe fn wake(&self) { - self.0.wake() - } - - #[inline] - unsafe fn wake_local(&self) { - // Since we're nonlocal, we can't call wake_local - self.0.wake() - } - } - - /// Creates a `LocalWaker` from a non-local `wake`. - /// - /// This function is similar to `local_waker`, but does not require that `wake` - /// is local to the current thread. The resulting `LocalWaker` will call - /// `wake.wake()` when awoken. - #[inline] - pub fn local_waker_from_nonlocal(wake: Arc) -> LocalWaker { - unsafe { - let ptr = mem::transmute::, NonNull>>(wake); - LocalWaker::new(ptr) - } - } -} diff --git a/src/libcore/future/future.rs b/src/libcore/future/future.rs index 539b07fc21eea..c40045245425b 100644 --- a/src/libcore/future/future.rs +++ b/src/libcore/future/future.rs @@ -5,7 +5,7 @@ use marker::Unpin; use ops; use pin::Pin; -use task::{Poll, LocalWaker}; +use task::{Poll, Waker}; /// A future represents an asynchronous computation. /// @@ -25,7 +25,7 @@ use task::{Poll, LocalWaker}; /// `await!` the value. #[must_use = "futures do nothing unless polled"] pub trait Future { - /// The result of the `Future`. + /// The type of value produced on completion. type Output; /// Attempt to resolve the future to a final value, registering @@ -42,16 +42,16 @@ pub trait Future { /// Once a future has finished, clients should not `poll` it again. /// /// When a future is not ready yet, `poll` returns `Poll::Pending` and - /// stores a clone of the [`LocalWaker`] to be woken once the future can + /// stores a clone of the [`Waker`] to be woken once the future can /// make progress. For example, a future waiting for a socket to become - /// readable would call `.clone()` on the [`LocalWaker`] and store it. + /// readable would call `.clone()` on the [`Waker`] and store it. /// When a signal arrives elsewhere indicating that the socket is readable, - /// `[LocalWaker::wake]` is called and the socket future's task is awoken. + /// `[Waker::wake]` is called and the socket future's task is awoken. /// Once a task has been woken up, it should attempt to `poll` the future /// again, which may or may not produce a final value. /// /// Note that on multiple calls to `poll`, only the most recent - /// [`LocalWaker`] passed to `poll` should be scheduled to receive a + /// [`Waker`] passed to `poll` should be scheduled to receive a /// wakeup. /// /// # Runtime characteristics @@ -74,16 +74,6 @@ pub trait Future { /// thread pool (or something similar) to ensure that `poll` can return /// quickly. /// - /// # [`LocalWaker`], [`Waker`] and thread-safety - /// - /// The `poll` function takes a [`LocalWaker`], an object which knows how to - /// awaken the current task. [`LocalWaker`] is not `Send` nor `Sync`, so in - /// order to make thread-safe futures the [`LocalWaker::into_waker`] method - /// should be used to convert the [`LocalWaker`] into a thread-safe version. - /// [`LocalWaker::wake`] implementations have the ability to be more - /// efficient, however, so when thread safety is not necessary, - /// [`LocalWaker`] should be preferred. - /// /// # Panics /// /// Once a future has completed (returned `Ready` from `poll`), @@ -93,18 +83,16 @@ pub trait Future { /// /// [`Poll::Pending`]: ../task/enum.Poll.html#variant.Pending /// [`Poll::Ready(val)`]: ../task/enum.Poll.html#variant.Ready - /// [`LocalWaker`]: ../task/struct.LocalWaker.html - /// [`LocalWaker::into_waker`]: ../task/struct.LocalWaker.html#method.into_waker - /// [`LocalWaker::wake`]: ../task/struct.LocalWaker.html#method.wake /// [`Waker`]: ../task/struct.Waker.html - fn poll(self: Pin<&mut Self>, lw: &LocalWaker) -> Poll; + /// [`Waker::wake`]: ../task/struct.Waker.html#method.wake + fn poll(self: Pin<&mut Self>, waker: &Waker) -> Poll; } impl<'a, F: ?Sized + Future + Unpin> Future for &'a mut F { type Output = F::Output; - fn poll(mut self: Pin<&mut Self>, lw: &LocalWaker) -> Poll { - F::poll(Pin::new(&mut **self), lw) + fn poll(mut self: Pin<&mut Self>, waker: &Waker) -> Poll { + F::poll(Pin::new(&mut **self), waker) } } @@ -115,7 +103,7 @@ where { type Output = <

::Target as Future>::Output; - fn poll(self: Pin<&mut Self>, lw: &LocalWaker) -> Poll { - Pin::get_mut(self).as_mut().poll(lw) + fn poll(self: Pin<&mut Self>, waker: &Waker) -> Poll { + Pin::get_mut(self).as_mut().poll(waker) } } diff --git a/src/libcore/task/mod.rs b/src/libcore/task/mod.rs index 9552e53ebf849..9b8f598116200 100644 --- a/src/libcore/task/mod.rs +++ b/src/libcore/task/mod.rs @@ -8,4 +8,4 @@ mod poll; pub use self::poll::Poll; mod wake; -pub use self::wake::{Waker, LocalWaker, UnsafeWake}; +pub use self::wake::{Waker, RawWaker, RawWakerVTable}; diff --git a/src/libcore/task/wake.rs b/src/libcore/task/wake.rs index 3f7098f1ef934..bb91bf2ecfb44 100644 --- a/src/libcore/task/wake.rs +++ b/src/libcore/task/wake.rs @@ -4,16 +4,76 @@ use fmt; use marker::Unpin; -use ptr::NonNull; + +/// A `RawWaker` allows the implementor of a task executor to create a `Waker` +/// which provides customized wakeup behavior. +/// +/// It consists of a data pointer and a virtual function pointer table (vtable) that +/// customizes the behavior of the `RawWaker`. +#[derive(PartialEq)] +pub struct RawWaker { + /// A data pointer, which can be used to store arbitrary data as required + /// by the executor. This could be e.g. a type-erased pointer to an `Arc` + /// that is associated with the task. + /// The value of this field gets passed to all functions that are part of + /// the vtable as first parameter. + pub data: *const (), + /// Virtual function pointer table that customizes the behavior of this waker. + pub vtable: &'static RawWakerVTable, +} + +impl fmt::Debug for RawWaker { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.debug_struct("RawWaker") + .finish() + } +} + +/// A virtual function pointer table (vtable) that specifies the behavior +/// of a `RawWaker`. +/// +/// The pointer passed to all functions inside the vtable is the `data` pointer +/// from the enclosing `RawWaker` object. +#[derive(PartialEq, Copy, Clone)] +pub struct RawWakerVTable { + /// This function will be called when the `RawWaker` gets cloned, e.g. when + /// the `Waker` in which the `RawWaker` is stored gets cloned. + /// + /// The implementation of this function must retain all resources that are + /// required for this additional instance of a `RawWaker` and associated + /// task. Calling `wake` on the resulting `RawWaker` should result in a wakeup + /// of the same task that would have been awoken by the original `RawWaker`. + pub clone: unsafe fn(*const ()) -> RawWaker, + + /// This function will be called when `wake` is called on the `Waker`. + /// It must wake up the task associated with this `RawWaker`. + pub wake: unsafe fn(*const ()), + + /// This function gets called when a `RawWaker` gets dropped. + /// + /// The implementation of this function must make sure to release any + /// resources that are associated with this instance of a `RawWaker` and + /// associated task. + pub drop_fn: unsafe fn(*const ()), +} + +impl fmt::Debug for RawWakerVTable { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.debug_struct("RawWakerVTable") + .finish() + } +} /// A `Waker` is a handle for waking up a task by notifying its executor that it /// is ready to be run. /// -/// This handle contains a trait object pointing to an instance of the `UnsafeWake` -/// trait, allowing notifications to get routed through it. +/// This handle encapsulates a `RawWaker` instance, which defines the +/// executor-specific wakeup behavior. +/// +/// Implements `Clone`, `Send`, and `Sync`. #[repr(transparent)] pub struct Waker { - inner: NonNull, + waker: RawWaker, } impl Unpin for Waker {} @@ -21,264 +81,52 @@ unsafe impl Send for Waker {} unsafe impl Sync for Waker {} impl Waker { - /// Constructs a new `Waker` directly. - /// - /// Note that most code will not need to call this. Implementers of the - /// `UnsafeWake` trait will typically provide a wrapper that calls this - /// but you otherwise shouldn't call it directly. - /// - /// If you're working with the standard library then it's recommended to - /// use the `Waker::from` function instead which works with the safe - /// `Arc` type and the safe `Wake` trait. - #[inline] - pub unsafe fn new(inner: NonNull) -> Self { - Waker { inner } - } - /// Wake up the task associated with this `Waker`. - #[inline] pub fn wake(&self) { - unsafe { self.inner.as_ref().wake() } + // The actual wakeup call is delegated through a virtual function call + // to the implementation which is defined by the executor. + unsafe { (self.waker.vtable.wake)(self.waker.data) } } - /// Returns whether or not this `Waker` and `other` awaken the same task. + /// Returns whether or not this `Waker` and other `Waker` have awaken the same task. /// /// This function works on a best-effort basis, and may return false even /// when the `Waker`s would awaken the same task. However, if this function - /// returns true, it is guaranteed that the `Waker`s will awaken the same - /// task. + /// returns `true`, it is guaranteed that the `Waker`s will awaken the same task. /// /// This function is primarily used for optimization purposes. - #[inline] pub fn will_wake(&self, other: &Waker) -> bool { - self.inner == other.inner + self.waker == other.waker } - /// Returns whether or not this `Waker` and `other` `LocalWaker` awaken - /// the same task. - /// - /// This function works on a best-effort basis, and may return false even - /// when the `Waker`s would awaken the same task. However, if this function - /// returns true, it is guaranteed that the `Waker`s will awaken the same - /// task. + /// Creates a new `Waker` from `RawWaker`. /// - /// This function is primarily used for optimization purposes. - #[inline] - pub fn will_wake_local(&self, other: &LocalWaker) -> bool { - self.will_wake(&other.0) + /// The method cannot check whether `RawWaker` fulfills the required API + /// contract to make it usable for `Waker` and is therefore unsafe. + pub unsafe fn new_unchecked(waker: RawWaker) -> Waker { + Waker { + waker: waker, + } } } impl Clone for Waker { - #[inline] fn clone(&self) -> Self { - unsafe { - self.inner.as_ref().clone_raw() + Waker { + waker: unsafe { (self.waker.vtable.clone)(self.waker.data) }, } } } -impl fmt::Debug for Waker { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_struct("Waker") - .finish() - } -} - impl Drop for Waker { - #[inline] fn drop(&mut self) { - unsafe { - self.inner.as_ref().drop_raw() - } + unsafe { (self.waker.vtable.drop_fn)(self.waker.data) } } } -/// A `LocalWaker` is a handle for waking up a task by notifying its executor that it -/// is ready to be run. -/// -/// This is similar to the `Waker` type, but cannot be sent across threads. -/// Task executors can use this type to implement more optimized single-threaded wakeup -/// behavior. -#[repr(transparent)] -#[derive(Clone)] -pub struct LocalWaker(Waker); - -impl Unpin for LocalWaker {} -impl !Send for LocalWaker {} -impl !Sync for LocalWaker {} - -impl LocalWaker { - /// Constructs a new `LocalWaker` directly. - /// - /// Note that most code will not need to call this. Implementers of the - /// `UnsafeWake` trait will typically provide a wrapper that calls this - /// but you otherwise shouldn't call it directly. - /// - /// If you're working with the standard library then it's recommended to - /// use the `local_waker_from_nonlocal` or `local_waker` to convert a `Waker` - /// into a `LocalWaker`. - /// - /// For this function to be used safely, it must be sound to call `inner.wake_local()` - /// on the current thread. - #[inline] - pub unsafe fn new(inner: NonNull) -> Self { - LocalWaker(Waker::new(inner)) - } - - /// Borrows this `LocalWaker` as a `Waker`. - /// - /// `Waker` is nearly identical to `LocalWaker`, but is threadsafe - /// (implements `Send` and `Sync`). - #[inline] - pub fn as_waker(&self) -> &Waker { - &self.0 - } - - /// Converts this `LocalWaker` into a `Waker`. - /// - /// `Waker` is nearly identical to `LocalWaker`, but is threadsafe - /// (implements `Send` and `Sync`). - #[inline] - pub fn into_waker(self) -> Waker { - self.0 - } - - /// Wake up the task associated with this `LocalWaker`. - #[inline] - pub fn wake(&self) { - unsafe { self.0.inner.as_ref().wake_local() } - } - - /// Returns whether or not this `LocalWaker` and `other` `LocalWaker` awaken the same task. - /// - /// This function works on a best-effort basis, and may return false even - /// when the `LocalWaker`s would awaken the same task. However, if this function - /// returns true, it is guaranteed that the `LocalWaker`s will awaken the same - /// task. - /// - /// This function is primarily used for optimization purposes. - #[inline] - pub fn will_wake(&self, other: &LocalWaker) -> bool { - self.0.will_wake(&other.0) - } - - /// Returns whether or not this `LocalWaker` and `other` `Waker` awaken the same task. - /// - /// This function works on a best-effort basis, and may return false even - /// when the `Waker`s would awaken the same task. However, if this function - /// returns true, it is guaranteed that the `LocalWaker`s will awaken the same - /// task. - /// - /// This function is primarily used for optimization purposes. - #[inline] - pub fn will_wake_nonlocal(&self, other: &Waker) -> bool { - self.0.will_wake(other) - } -} - -impl From for Waker { - /// Converts a `LocalWaker` into a `Waker`. - /// - /// This conversion turns a `!Sync` `LocalWaker` into a `Sync` `Waker`, allowing a wakeup - /// object to be sent to another thread, but giving up its ability to do specialized - /// thread-local wakeup behavior. - #[inline] - fn from(local_waker: LocalWaker) -> Self { - local_waker.0 - } -} - -impl fmt::Debug for LocalWaker { +impl fmt::Debug for Waker { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_struct("LocalWaker") + f.debug_struct("Waker") .finish() } } - -/// An unsafe trait for implementing custom memory management for a `Waker` or `LocalWaker`. -/// -/// A `Waker` conceptually is a cloneable trait object for `Wake`, and is -/// most often essentially just `Arc`. However, in some contexts -/// (particularly `no_std`), it's desirable to avoid `Arc` in favor of some -/// custom memory management strategy. This trait is designed to allow for such -/// customization. -/// -/// When using `std`, a default implementation of the `UnsafeWake` trait is provided for -/// `Arc` where `T: Wake`. -pub unsafe trait UnsafeWake: Send + Sync { - /// Creates a clone of this `UnsafeWake` and stores it behind a `Waker`. - /// - /// This function will create a new uniquely owned handle that under the - /// hood references the same notification instance. In other words calls - /// to `wake` on the returned handle should be equivalent to calls to - /// `wake` on this handle. - /// - /// # Unsafety - /// - /// This function is unsafe to call because it's asserting the `UnsafeWake` - /// value is in a consistent state, i.e., hasn't been dropped. - unsafe fn clone_raw(&self) -> Waker; - - /// Drops this instance of `UnsafeWake`, deallocating resources - /// associated with it. - /// - /// FIXME(cramertj) - /// This method is intended to have a signature such as: - /// - /// ```ignore (not-a-doctest) - /// fn drop_raw(self: *mut Self); - /// ``` - /// - /// Unfortunately in Rust today that signature is not object safe. - /// Nevertheless it's recommended to implement this function *as if* that - /// were its signature. As such it is not safe to call on an invalid - /// pointer, nor is the validity of the pointer guaranteed after this - /// function returns. - /// - /// # Unsafety - /// - /// This function is unsafe to call because it's asserting the `UnsafeWake` - /// value is in a consistent state, i.e., hasn't been dropped. - unsafe fn drop_raw(&self); - - /// Indicates that the associated task is ready to make progress and should - /// be `poll`ed. - /// - /// Executors generally maintain a queue of "ready" tasks; `wake` should place - /// the associated task onto this queue. - /// - /// # Panics - /// - /// Implementations should avoid panicking, but clients should also be prepared - /// for panics. - /// - /// # Unsafety - /// - /// This function is unsafe to call because it's asserting the `UnsafeWake` - /// value is in a consistent state, i.e., hasn't been dropped. - unsafe fn wake(&self); - - /// Indicates that the associated task is ready to make progress and should - /// be `poll`ed. This function is the same as `wake`, but can only be called - /// from the thread that this `UnsafeWake` is "local" to. This allows for - /// implementors to provide specialized wakeup behavior specific to the current - /// thread. This function is called by `LocalWaker::wake`. - /// - /// Executors generally maintain a queue of "ready" tasks; `wake_local` should place - /// the associated task onto this queue. - /// - /// # Panics - /// - /// Implementations should avoid panicking, but clients should also be prepared - /// for panics. - /// - /// # Unsafety - /// - /// This function is unsafe to call because it's asserting the `UnsafeWake` - /// value is in a consistent state, i.e., hasn't been dropped, and that the - /// `UnsafeWake` hasn't moved from the thread on which it was created. - unsafe fn wake_local(&self) { - self.wake() - } -} diff --git a/src/libstd/future.rs b/src/libstd/future.rs index d1203be3cf011..aa784746122db 100644 --- a/src/libstd/future.rs +++ b/src/libstd/future.rs @@ -5,7 +5,7 @@ use core::marker::Unpin; use core::pin::Pin; use core::option::Option; use core::ptr::NonNull; -use core::task::{LocalWaker, Poll}; +use core::task::{Waker, Poll}; use core::ops::{Drop, Generator, GeneratorState}; #[doc(inline)] @@ -32,10 +32,10 @@ impl> !Unpin for GenFuture {} #[unstable(feature = "gen_future", issue = "50547")] impl> Future for GenFuture { type Output = T::Return; - fn poll(self: Pin<&mut Self>, lw: &LocalWaker) -> Poll { + fn poll(self: Pin<&mut Self>, waker: &Waker) -> Poll { // Safe because we're !Unpin + !Drop mapping to a ?Unpin value let gen = unsafe { Pin::map_unchecked_mut(self, |s| &mut s.0) }; - set_task_waker(lw, || match gen.resume() { + set_task_waker(waker, || match gen.resume() { GeneratorState::Yielded(()) => Poll::Pending, GeneratorState::Complete(x) => Poll::Ready(x), }) @@ -43,10 +43,10 @@ impl> Future for GenFuture { } thread_local! { - static TLS_WAKER: Cell>> = Cell::new(None); + static TLS_WAKER: Cell>> = Cell::new(None); } -struct SetOnDrop(Option>); +struct SetOnDrop(Option>); impl Drop for SetOnDrop { fn drop(&mut self) { @@ -58,12 +58,12 @@ impl Drop for SetOnDrop { #[unstable(feature = "gen_future", issue = "50547")] /// Sets the thread-local task context used by async/await futures. -pub fn set_task_waker(lw: &LocalWaker, f: F) -> R +pub fn set_task_waker(waker: &Waker, f: F) -> R where F: FnOnce() -> R { let old_waker = TLS_WAKER.with(|tls_waker| { - tls_waker.replace(Some(NonNull::from(lw))) + tls_waker.replace(Some(NonNull::from(waker))) }); let _reset_waker = SetOnDrop(old_waker); f() @@ -78,7 +78,7 @@ where /// retrieved by a surrounding call to get_task_waker. pub fn get_task_waker(f: F) -> R where - F: FnOnce(&LocalWaker) -> R + F: FnOnce(&Waker) -> R { let waker_ptr = TLS_WAKER.with(|tls_waker| { // Clear the entry so that nested `get_task_waker` calls @@ -88,7 +88,7 @@ where let _reset_waker = SetOnDrop(waker_ptr); let waker_ptr = waker_ptr.expect( - "TLS LocalWaker not set. This is a rustc bug. \ + "TLS Waker not set. This is a rustc bug. \ Please file an issue on https://github.com/rust-lang/rust."); unsafe { f(waker_ptr.as_ref()) } } @@ -99,5 +99,5 @@ pub fn poll_with_tls_waker(f: Pin<&mut F>) -> Poll where F: Future { - get_task_waker(|lw| F::poll(f, lw)) + get_task_waker(|waker| F::poll(f, waker)) } diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index 244caf28ec7cd..207ba9ae02f05 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -468,8 +468,6 @@ pub mod task { //! Types and Traits for working with asynchronous tasks. #[doc(inline)] pub use core::task::*; - #[doc(inline)] - pub use alloc_crate::task::*; } #[unstable(feature = "futures_api", diff --git a/src/libstd/panic.rs b/src/libstd/panic.rs index d27f6ca88c2e9..862fdf051ccd1 100644 --- a/src/libstd/panic.rs +++ b/src/libstd/panic.rs @@ -12,7 +12,7 @@ use panicking; use ptr::{Unique, NonNull}; use rc::Rc; use sync::{Arc, Mutex, RwLock, atomic}; -use task::{LocalWaker, Poll}; +use task::{Waker, Poll}; use thread::Result; #[stable(feature = "panic_hooks", since = "1.10.0")] @@ -323,9 +323,9 @@ impl fmt::Debug for AssertUnwindSafe { impl<'a, F: Future> Future for AssertUnwindSafe { type Output = F::Output; - fn poll(self: Pin<&mut Self>, lw: &LocalWaker) -> Poll { + fn poll(self: Pin<&mut Self>, waker: &Waker) -> Poll { let pinned_field = unsafe { Pin::map_unchecked_mut(self, |x| &mut x.0) }; - F::poll(pinned_field, lw) + F::poll(pinned_field, waker) } } From 01a704cf3650710a3e3db221759207539de61613 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 3 Feb 2019 12:30:34 -0800 Subject: [PATCH 2/9] Apply suggestions from code review Co-Authored-By: Matthias247 --- src/libcore/task/wake.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libcore/task/wake.rs b/src/libcore/task/wake.rs index bb91bf2ecfb44..adaca50434557 100644 --- a/src/libcore/task/wake.rs +++ b/src/libcore/task/wake.rs @@ -8,7 +8,9 @@ use marker::Unpin; /// A `RawWaker` allows the implementor of a task executor to create a `Waker` /// which provides customized wakeup behavior. /// -/// It consists of a data pointer and a virtual function pointer table (vtable) that +/// [vtable]: https://en.wikipedia.org/wiki/Virtual_method_table +/// +/// It consists of a data pointer and a [virtual function pointer table (vtable)][vtable] that /// customizes the behavior of the `RawWaker`. #[derive(PartialEq)] pub struct RawWaker { @@ -16,7 +18,7 @@ pub struct RawWaker { /// by the executor. This could be e.g. a type-erased pointer to an `Arc` /// that is associated with the task. /// The value of this field gets passed to all functions that are part of - /// the vtable as first parameter. + /// the vtable as the first parameter. pub data: *const (), /// Virtual function pointer table that customizes the behavior of this waker. pub vtable: &'static RawWakerVTable, @@ -30,7 +32,7 @@ impl fmt::Debug for RawWaker { } /// A virtual function pointer table (vtable) that specifies the behavior -/// of a `RawWaker`. +/// of a [`RawWaker`]. /// /// The pointer passed to all functions inside the vtable is the `data` pointer /// from the enclosing `RawWaker` object. @@ -105,7 +107,7 @@ impl Waker { /// contract to make it usable for `Waker` and is therefore unsafe. pub unsafe fn new_unchecked(waker: RawWaker) -> Waker { Waker { - waker: waker, + waker, } } } From 9e6bc3c4386bf5f7f1885fdaab4ef01fdc93007e Mon Sep 17 00:00:00 2001 From: Matthias Einwag Date: Sun, 3 Feb 2019 12:59:51 -0800 Subject: [PATCH 3/9] Apply review suggestions and fix tests --- src/libcore/task/wake.rs | 65 ++++++----- .../compile-fail/must_use-in-stdlib-traits.rs | 4 +- src/test/run-pass/async-await.rs | 69 ++++++++++-- src/test/run-pass/futures-api.rs | 103 ++++++++++++------ 4 files changed, 163 insertions(+), 78 deletions(-) diff --git a/src/libcore/task/wake.rs b/src/libcore/task/wake.rs index adaca50434557..fe2de61c59446 100644 --- a/src/libcore/task/wake.rs +++ b/src/libcore/task/wake.rs @@ -5,14 +5,14 @@ use fmt; use marker::Unpin; -/// A `RawWaker` allows the implementor of a task executor to create a `Waker` +/// A `RawWaker` allows the implementor of a task executor to create a [`Waker`] /// which provides customized wakeup behavior. /// /// [vtable]: https://en.wikipedia.org/wiki/Virtual_method_table /// /// It consists of a data pointer and a [virtual function pointer table (vtable)][vtable] that /// customizes the behavior of the `RawWaker`. -#[derive(PartialEq)] +#[derive(PartialEq, Debug)] pub struct RawWaker { /// A data pointer, which can be used to store arbitrary data as required /// by the executor. This could be e.g. a type-erased pointer to an `Arc` @@ -24,55 +24,41 @@ pub struct RawWaker { pub vtable: &'static RawWakerVTable, } -impl fmt::Debug for RawWaker { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_struct("RawWaker") - .finish() - } -} - /// A virtual function pointer table (vtable) that specifies the behavior /// of a [`RawWaker`]. /// /// The pointer passed to all functions inside the vtable is the `data` pointer -/// from the enclosing `RawWaker` object. -#[derive(PartialEq, Copy, Clone)] +/// from the enclosing [`RawWaker`] object. +#[derive(PartialEq, Copy, Clone, Debug)] pub struct RawWakerVTable { - /// This function will be called when the `RawWaker` gets cloned, e.g. when - /// the `Waker` in which the `RawWaker` is stored gets cloned. + /// This function will be called when the [`RawWaker`] gets cloned, e.g. when + /// the [`Waker`] in which the [`RawWaker`] is stored gets cloned. /// /// The implementation of this function must retain all resources that are - /// required for this additional instance of a `RawWaker` and associated - /// task. Calling `wake` on the resulting `RawWaker` should result in a wakeup - /// of the same task that would have been awoken by the original `RawWaker`. + /// required for this additional instance of a [`RawWaker`] and associated + /// task. Calling `wake` on the resulting [`RawWaker`] should result in a wakeup + /// of the same task that would have been awoken by the original [`RawWaker`]. pub clone: unsafe fn(*const ()) -> RawWaker, - /// This function will be called when `wake` is called on the `Waker`. - /// It must wake up the task associated with this `RawWaker`. + /// This function will be called when `wake` is called on the [`Waker`]. + /// It must wake up the task associated with this [`RawWaker`]. pub wake: unsafe fn(*const ()), - /// This function gets called when a `RawWaker` gets dropped. + /// This function gets called when a [`RawWaker`] gets dropped. /// /// The implementation of this function must make sure to release any - /// resources that are associated with this instance of a `RawWaker` and + /// resources that are associated with this instance of a [`RawWaker`] and /// associated task. - pub drop_fn: unsafe fn(*const ()), -} - -impl fmt::Debug for RawWakerVTable { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_struct("RawWakerVTable") - .finish() - } + pub drop: unsafe fn(*const ()), } /// A `Waker` is a handle for waking up a task by notifying its executor that it /// is ready to be run. /// -/// This handle encapsulates a `RawWaker` instance, which defines the +/// This handle encapsulates a [`RawWaker`] instance, which defines the /// executor-specific wakeup behavior. /// -/// Implements `Clone`, `Send`, and `Sync`. +/// Implements [`Clone`], [`Send`], and [`Sync`]. #[repr(transparent)] pub struct Waker { waker: RawWaker, @@ -87,6 +73,10 @@ impl Waker { pub fn wake(&self) { // The actual wakeup call is delegated through a virtual function call // to the implementation which is defined by the executor. + + // SAFETY: This is safe because `Waker::new_unchecked` is the only way + // to initialize `wake` and `data` requiring the user to acknowledge + // that the contract of `RawWaker` is upheld. unsafe { (self.waker.vtable.wake)(self.waker.data) } } @@ -101,10 +91,11 @@ impl Waker { self.waker == other.waker } - /// Creates a new `Waker` from `RawWaker`. + /// Creates a new `Waker` from [`RawWaker`]. /// - /// The method cannot check whether `RawWaker` fulfills the required API - /// contract to make it usable for `Waker` and is therefore unsafe. + /// The behavior of the returned `Waker` is undefined if the contract defined + /// in [RawWaker]'s documentation is not upheld. Therefore this method is + /// unsafe. pub unsafe fn new_unchecked(waker: RawWaker) -> Waker { Waker { waker, @@ -115,6 +106,9 @@ impl Waker { impl Clone for Waker { fn clone(&self) -> Self { Waker { + // SAFETY: This is safe because `Waker::new_unchecked` is the only way + // to initialize `clone` and `data` requiring the user to acknowledge + // that the contract of [`RawWaker`] is upheld. waker: unsafe { (self.waker.vtable.clone)(self.waker.data) }, } } @@ -122,7 +116,10 @@ impl Clone for Waker { impl Drop for Waker { fn drop(&mut self) { - unsafe { (self.waker.vtable.drop_fn)(self.waker.data) } + // SAFETY: This is safe because `Waker::new_unchecked` is the only way + // to initialize `drop` and `data` requiring the user to acknowledge + // that the contract of `RawWaker` is upheld. + unsafe { (self.waker.vtable.drop)(self.waker.data) } } } diff --git a/src/test/compile-fail/must_use-in-stdlib-traits.rs b/src/test/compile-fail/must_use-in-stdlib-traits.rs index 7e446fdaeaf41..b4f07ab33214c 100644 --- a/src/test/compile-fail/must_use-in-stdlib-traits.rs +++ b/src/test/compile-fail/must_use-in-stdlib-traits.rs @@ -4,7 +4,7 @@ use std::iter::Iterator; use std::future::Future; -use std::task::{Poll, LocalWaker}; +use std::task::{Poll, Waker}; use std::pin::Pin; use std::unimplemented; @@ -13,7 +13,7 @@ struct MyFuture; impl Future for MyFuture { type Output = u32; - fn poll(self: Pin<&mut Self>, lw: &LocalWaker) -> Poll { + fn poll(self: Pin<&mut Self>, waker: &Waker) -> Poll { Poll::Pending } } diff --git a/src/test/run-pass/async-await.rs b/src/test/run-pass/async-await.rs index 552f38215738b..2def62a6ba60c 100644 --- a/src/test/run-pass/async-await.rs +++ b/src/test/run-pass/async-await.rs @@ -9,17 +9,70 @@ use std::sync::{ atomic::{self, AtomicUsize}, }; use std::task::{ - LocalWaker, Poll, Wake, - local_waker_from_nonlocal, + Poll, Waker, RawWaker, RawWakerVTable, }; +macro_rules! waker_vtable { + ($ty:ident) => { + &RawWakerVTable { + clone: clone_arc_raw::<$ty>, + drop: drop_arc_raw::<$ty>, + wake: wake_arc_raw::<$ty>, + } + }; +} + +pub trait ArcWake { + fn wake(arc_self: &Arc); + + fn into_waker(wake: Arc) -> Waker where Self: Sized + { + let ptr = Arc::into_raw(wake) as *const(); + + unsafe { + Waker::new_unchecked(RawWaker{ + data: ptr, + vtable: waker_vtable!(Self), + }) + } + } +} + +unsafe fn increase_refcount(data: *const()) { + // Retain Arc by creating a copy + let arc: Arc = Arc::from_raw(data as *const T); + let arc_clone = arc.clone(); + // Forget the Arcs again, so that the refcount isn't decrased + let _ = Arc::into_raw(arc); + let _ = Arc::into_raw(arc_clone); +} + +unsafe fn clone_arc_raw(data: *const()) -> RawWaker { + increase_refcount::(data); + RawWaker { + data: data, + vtable: waker_vtable!(T), + } +} + +unsafe fn drop_arc_raw(data: *const()) { + // Drop Arc + let _: Arc = Arc::from_raw(data as *const T); +} + +unsafe fn wake_arc_raw(data: *const()) { + let arc: Arc = Arc::from_raw(data as *const T); + ArcWake::wake(&arc); + let _ = Arc::into_raw(arc); +} + struct Counter { wakes: AtomicUsize, } -impl Wake for Counter { - fn wake(this: &Arc) { - this.wakes.fetch_add(1, atomic::Ordering::SeqCst); +impl ArcWake for Counter { + fn wake(arc_self: &Arc) { + arc_self.wakes.fetch_add(1, atomic::Ordering::SeqCst); } } @@ -29,11 +82,11 @@ fn wake_and_yield_once() -> WakeOnceThenComplete { WakeOnceThenComplete(false) } impl Future for WakeOnceThenComplete { type Output = (); - fn poll(mut self: Pin<&mut Self>, lw: &LocalWaker) -> Poll<()> { + fn poll(mut self: Pin<&mut Self>, waker: &Waker) -> Poll<()> { if self.0 { Poll::Ready(()) } else { - lw.wake(); + waker.wake(); self.0 = true; Poll::Pending } @@ -130,7 +183,7 @@ where { let mut fut = Box::pin(f(9)); let counter = Arc::new(Counter { wakes: AtomicUsize::new(0) }); - let waker = local_waker_from_nonlocal(counter.clone()); + let waker = ArcWake::into_waker(counter.clone()); assert_eq!(0, counter.wakes.load(atomic::Ordering::SeqCst)); assert_eq!(Poll::Pending, fut.as_mut().poll(&waker)); assert_eq!(1, counter.wakes.load(atomic::Ordering::SeqCst)); diff --git a/src/test/run-pass/futures-api.rs b/src/test/run-pass/futures-api.rs index e3023521100c4..058d09e2420ae 100644 --- a/src/test/run-pass/futures-api.rs +++ b/src/test/run-pass/futures-api.rs @@ -3,28 +3,75 @@ use std::future::Future; use std::pin::Pin; -use std::rc::Rc; use std::sync::{ Arc, atomic::{self, AtomicUsize}, }; use std::task::{ - Poll, Wake, Waker, LocalWaker, - local_waker, local_waker_from_nonlocal, + Poll, Waker, RawWaker, RawWakerVTable, }; -struct Counter { - local_wakes: AtomicUsize, - nonlocal_wakes: AtomicUsize, +macro_rules! waker_vtable { + ($ty:ident) => { + &RawWakerVTable { + clone: clone_arc_raw::<$ty>, + drop: drop_arc_raw::<$ty>, + wake: wake_arc_raw::<$ty>, + } + }; } -impl Wake for Counter { - fn wake(this: &Arc) { - this.nonlocal_wakes.fetch_add(1, atomic::Ordering::SeqCst); +pub trait ArcWake { + fn wake(arc_self: &Arc); + + fn into_waker(wake: Arc) -> Waker where Self: Sized + { + let ptr = Arc::into_raw(wake) as *const(); + + unsafe { + Waker::new_unchecked(RawWaker{ + data: ptr, + vtable: waker_vtable!(Self), + }) + } } +} + +unsafe fn increase_refcount(data: *const()) { + // Retain Arc by creating a copy + let arc: Arc = Arc::from_raw(data as *const T); + let arc_clone = arc.clone(); + // Forget the Arcs again, so that the refcount isn't decrased + let _ = Arc::into_raw(arc); + let _ = Arc::into_raw(arc_clone); +} - unsafe fn wake_local(this: &Arc) { - this.local_wakes.fetch_add(1, atomic::Ordering::SeqCst); +unsafe fn clone_arc_raw(data: *const()) -> RawWaker { + increase_refcount::(data); + RawWaker { + data: data, + vtable: waker_vtable!(T), + } +} + +unsafe fn drop_arc_raw(data: *const()) { + // Drop Arc + let _: Arc = Arc::from_raw(data as *const T); +} + +unsafe fn wake_arc_raw(data: *const()) { + let arc: Arc = Arc::from_raw(data as *const T); + ArcWake::wake(&arc); + let _ = Arc::into_raw(arc); +} + +struct Counter { + wakes: AtomicUsize, +} + +impl Wake for Counter { + fn wake(arc_self: &Arc) { + arc_self.wakes.fetch_add(1, atomic::Ordering::SeqCst); } } @@ -32,40 +79,28 @@ struct MyFuture; impl Future for MyFuture { type Output = (); - fn poll(self: Pin<&mut Self>, lw: &LocalWaker) -> Poll { - // Wake once locally - lw.wake(); - // Wake twice non-locally - let waker = lw.clone().into_waker(); + fn poll(self: Pin<&mut Self>, waker: &Waker) -> Poll { + // Wake twice waker.wake(); waker.wake(); Poll::Ready(()) } } -fn test_local_waker() { +fn test_waker() { let counter = Arc::new(Counter { - local_wakes: AtomicUsize::new(0), - nonlocal_wakes: AtomicUsize::new(0), + wakes: AtomicUsize::new(0), }); - let waker = unsafe { local_waker(counter.clone()) }; - assert_eq!(Poll::Ready(()), Pin::new(&mut MyFuture).poll(&waker)); - assert_eq!(1, counter.local_wakes.load(atomic::Ordering::SeqCst)); - assert_eq!(2, counter.nonlocal_wakes.load(atomic::Ordering::SeqCst)); -} + let waker = ArcWake::into_waker(counter.clone()); + assert_eq!(2, Arc::strong_count(&counter)); -fn test_local_as_nonlocal_waker() { - let counter = Arc::new(Counter { - local_wakes: AtomicUsize::new(0), - nonlocal_wakes: AtomicUsize::new(0), - }); - let waker: LocalWaker = local_waker_from_nonlocal(counter.clone()); assert_eq!(Poll::Ready(()), Pin::new(&mut MyFuture).poll(&waker)); - assert_eq!(0, counter.local_wakes.load(atomic::Ordering::SeqCst)); - assert_eq!(3, counter.nonlocal_wakes.load(atomic::Ordering::SeqCst)); + assert_eq!(2, counter.wakes.load(atomic::Ordering::SeqCst)); + + drop(waker); + assert_eq!(1, Arc::strong_count(&counter)); } fn main() { - test_local_waker(); - test_local_as_nonlocal_waker(); + test_waker(); } From f005e1c5d72c775bbb4370e9d8031fbc74efe4eb Mon Sep 17 00:00:00 2001 From: Matthias Einwag Date: Sun, 3 Feb 2019 14:59:22 -0800 Subject: [PATCH 4/9] Fix test --- src/test/run-pass/futures-api.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/run-pass/futures-api.rs b/src/test/run-pass/futures-api.rs index 058d09e2420ae..8ada7d4fa7416 100644 --- a/src/test/run-pass/futures-api.rs +++ b/src/test/run-pass/futures-api.rs @@ -69,7 +69,7 @@ struct Counter { wakes: AtomicUsize, } -impl Wake for Counter { +impl ArcWake for Counter { fn wake(arc_self: &Arc) { arc_self.wakes.fetch_add(1, atomic::Ordering::SeqCst); } From e1ec81459da4ba8e0633d90ddf440522a1587f35 Mon Sep 17 00:00:00 2001 From: Matthias Einwag Date: Tue, 5 Feb 2019 01:14:09 -0800 Subject: [PATCH 5/9] Apply more review suggestions --- src/libcore/future/future.rs | 8 +++++--- src/libcore/task/wake.rs | 6 ++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/libcore/future/future.rs b/src/libcore/future/future.rs index c40045245425b..470143d797afc 100644 --- a/src/libcore/future/future.rs +++ b/src/libcore/future/future.rs @@ -19,7 +19,8 @@ use task::{Poll, Waker}; /// final value. This method does not block if the value is not ready. Instead, /// the current task is scheduled to be woken up when it's possible to make /// further progress by `poll`ing again. The wake up is performed using -/// `cx.waker()`, a handle for waking up the current task. +/// the `waker` argument of the `poll()` method, which is a handle for waking +/// up the current task. /// /// When using a future, you generally won't call `poll` directly, but instead /// `await!` the value. @@ -78,8 +79,9 @@ pub trait Future { /// /// Once a future has completed (returned `Ready` from `poll`), /// then any future calls to `poll` may panic, block forever, or otherwise - /// cause bad behavior. The `Future` trait itself provides no guarantees - /// about the behavior of `poll` after a future has completed. + /// cause any kind of bad behavior expect causing memory unsafety. + /// The `Future` trait itself provides no guarantees about the behavior + /// of `poll` after a future has completed. /// /// [`Poll::Pending`]: ../task/enum.Poll.html#variant.Pending /// [`Poll::Ready(val)`]: ../task/enum.Poll.html#variant.Ready diff --git a/src/libcore/task/wake.rs b/src/libcore/task/wake.rs index fe2de61c59446..1f42d3e2690f6 100644 --- a/src/libcore/task/wake.rs +++ b/src/libcore/task/wake.rs @@ -42,6 +42,9 @@ pub struct RawWakerVTable { /// This function will be called when `wake` is called on the [`Waker`]. /// It must wake up the task associated with this [`RawWaker`]. + /// + /// The implemention of this function must not consume the provided data + /// pointer. pub wake: unsafe fn(*const ()), /// This function gets called when a [`RawWaker`] gets dropped. @@ -125,7 +128,10 @@ impl Drop for Waker { impl fmt::Debug for Waker { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let vtable_ptr = self.waker.vtable as *const RawWakerVTable; f.debug_struct("Waker") + .field("data", &self.waker.data) + .field("vtable", &vtable_ptr) .finish() } } From 363e992b9885e2ce55b389ab274f9cd88598104d Mon Sep 17 00:00:00 2001 From: Matthias Einwag Date: Tue, 5 Feb 2019 01:30:00 -0800 Subject: [PATCH 6/9] review suggestions --- src/libcore/future/future.rs | 6 ++++-- src/libcore/task/wake.rs | 5 +++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/libcore/future/future.rs b/src/libcore/future/future.rs index 470143d797afc..459e8a927e757 100644 --- a/src/libcore/future/future.rs +++ b/src/libcore/future/future.rs @@ -68,13 +68,15 @@ pub trait Future { /// typically do *not* suffer the same problems of "all wakeups must poll /// all events"; they are more like `epoll(4)`. /// - /// An implementation of `poll` should strive to return quickly, and must - /// *never* block. Returning quickly prevents unnecessarily clogging up + /// An implementation of `poll` should strive to return quickly, and should + /// not block. Returning quickly prevents unnecessarily clogging up /// threads or event loops. If it is known ahead of time that a call to /// `poll` may end up taking awhile, the work should be offloaded to a /// thread pool (or something similar) to ensure that `poll` can return /// quickly. /// + /// An implementation of `poll` may also never cause memory unsafety. + /// /// # Panics /// /// Once a future has completed (returned `Ready` from `poll`), diff --git a/src/libcore/task/wake.rs b/src/libcore/task/wake.rs index 1f42d3e2690f6..a877e033bc63b 100644 --- a/src/libcore/task/wake.rs +++ b/src/libcore/task/wake.rs @@ -29,6 +29,11 @@ pub struct RawWaker { /// /// The pointer passed to all functions inside the vtable is the `data` pointer /// from the enclosing [`RawWaker`] object. +/// +/// The functions inside this struct are only intended be called on the `data` +/// pointer of a properly constructed [`RawWaker`] object from inside the +/// [`RawWaker`] implementation. Calling one of the contained functions using +/// any other `data` pointer will cause undefined behavior. #[derive(PartialEq, Copy, Clone, Debug)] pub struct RawWakerVTable { /// This function will be called when the [`RawWaker`] gets cloned, e.g. when From 8e7ef03141c40e34bd740bfe521b656387af9d56 Mon Sep 17 00:00:00 2001 From: Matthias Einwag Date: Tue, 5 Feb 2019 20:32:35 -0800 Subject: [PATCH 7/9] Move ArcWake in common test file. --- src/test/run-pass/async-await.rs | 60 ++---------------------- src/test/run-pass/auxiliary/arc_wake.rs | 62 +++++++++++++++++++++++++ src/test/run-pass/futures-api.rs | 61 +++--------------------- 3 files changed, 73 insertions(+), 110 deletions(-) create mode 100644 src/test/run-pass/auxiliary/arc_wake.rs diff --git a/src/test/run-pass/async-await.rs b/src/test/run-pass/async-await.rs index 2def62a6ba60c..1843feed927a2 100644 --- a/src/test/run-pass/async-await.rs +++ b/src/test/run-pass/async-await.rs @@ -1,7 +1,10 @@ // edition:2018 +// aux-build:arc_wake.rs #![feature(arbitrary_self_types, async_await, await_macro, futures_api)] +extern crate arc_wake; + use std::pin::Pin; use std::future::Future; use std::sync::{ @@ -9,62 +12,9 @@ use std::sync::{ atomic::{self, AtomicUsize}, }; use std::task::{ - Poll, Waker, RawWaker, RawWakerVTable, + Poll, Waker, }; - -macro_rules! waker_vtable { - ($ty:ident) => { - &RawWakerVTable { - clone: clone_arc_raw::<$ty>, - drop: drop_arc_raw::<$ty>, - wake: wake_arc_raw::<$ty>, - } - }; -} - -pub trait ArcWake { - fn wake(arc_self: &Arc); - - fn into_waker(wake: Arc) -> Waker where Self: Sized - { - let ptr = Arc::into_raw(wake) as *const(); - - unsafe { - Waker::new_unchecked(RawWaker{ - data: ptr, - vtable: waker_vtable!(Self), - }) - } - } -} - -unsafe fn increase_refcount(data: *const()) { - // Retain Arc by creating a copy - let arc: Arc = Arc::from_raw(data as *const T); - let arc_clone = arc.clone(); - // Forget the Arcs again, so that the refcount isn't decrased - let _ = Arc::into_raw(arc); - let _ = Arc::into_raw(arc_clone); -} - -unsafe fn clone_arc_raw(data: *const()) -> RawWaker { - increase_refcount::(data); - RawWaker { - data: data, - vtable: waker_vtable!(T), - } -} - -unsafe fn drop_arc_raw(data: *const()) { - // Drop Arc - let _: Arc = Arc::from_raw(data as *const T); -} - -unsafe fn wake_arc_raw(data: *const()) { - let arc: Arc = Arc::from_raw(data as *const T); - ArcWake::wake(&arc); - let _ = Arc::into_raw(arc); -} +use arc_wake::ArcWake; struct Counter { wakes: AtomicUsize, diff --git a/src/test/run-pass/auxiliary/arc_wake.rs b/src/test/run-pass/auxiliary/arc_wake.rs new file mode 100644 index 0000000000000..0baaa378046f3 --- /dev/null +++ b/src/test/run-pass/auxiliary/arc_wake.rs @@ -0,0 +1,62 @@ +// edition:2018 + +#![feature(arbitrary_self_types, futures_api)] + +use std::sync::Arc; +use std::task::{ + Poll, Waker, RawWaker, RawWakerVTable, +}; + +macro_rules! waker_vtable { + ($ty:ident) => { + &RawWakerVTable { + clone: clone_arc_raw::<$ty>, + drop: drop_arc_raw::<$ty>, + wake: wake_arc_raw::<$ty>, + } + }; +} + +pub trait ArcWake { + fn wake(arc_self: &Arc); + + fn into_waker(wake: Arc) -> Waker where Self: Sized + { + let ptr = Arc::into_raw(wake) as *const(); + + unsafe { + Waker::new_unchecked(RawWaker{ + data: ptr, + vtable: waker_vtable!(Self), + }) + } + } +} + +unsafe fn increase_refcount(data: *const()) { + // Retain Arc by creating a copy + let arc: Arc = Arc::from_raw(data as *const T); + let arc_clone = arc.clone(); + // Forget the Arcs again, so that the refcount isn't decrased + let _ = Arc::into_raw(arc); + let _ = Arc::into_raw(arc_clone); +} + +unsafe fn clone_arc_raw(data: *const()) -> RawWaker { + increase_refcount::(data); + RawWaker { + data: data, + vtable: waker_vtable!(T), + } +} + +unsafe fn drop_arc_raw(data: *const()) { + // Drop Arc + let _: Arc = Arc::from_raw(data as *const T); +} + +unsafe fn wake_arc_raw(data: *const()) { + let arc: Arc = Arc::from_raw(data as *const T); + ArcWake::wake(&arc); + let _ = Arc::into_raw(arc); +} diff --git a/src/test/run-pass/futures-api.rs b/src/test/run-pass/futures-api.rs index 8ada7d4fa7416..fd4b585d34572 100644 --- a/src/test/run-pass/futures-api.rs +++ b/src/test/run-pass/futures-api.rs @@ -1,6 +1,10 @@ +// aux-build:arc_wake.rs + #![feature(arbitrary_self_types, futures_api)] #![allow(unused)] +extern crate arc_wake; + use std::future::Future; use std::pin::Pin; use std::sync::{ @@ -8,62 +12,9 @@ use std::sync::{ atomic::{self, AtomicUsize}, }; use std::task::{ - Poll, Waker, RawWaker, RawWakerVTable, + Poll, Waker, }; - -macro_rules! waker_vtable { - ($ty:ident) => { - &RawWakerVTable { - clone: clone_arc_raw::<$ty>, - drop: drop_arc_raw::<$ty>, - wake: wake_arc_raw::<$ty>, - } - }; -} - -pub trait ArcWake { - fn wake(arc_self: &Arc); - - fn into_waker(wake: Arc) -> Waker where Self: Sized - { - let ptr = Arc::into_raw(wake) as *const(); - - unsafe { - Waker::new_unchecked(RawWaker{ - data: ptr, - vtable: waker_vtable!(Self), - }) - } - } -} - -unsafe fn increase_refcount(data: *const()) { - // Retain Arc by creating a copy - let arc: Arc = Arc::from_raw(data as *const T); - let arc_clone = arc.clone(); - // Forget the Arcs again, so that the refcount isn't decrased - let _ = Arc::into_raw(arc); - let _ = Arc::into_raw(arc_clone); -} - -unsafe fn clone_arc_raw(data: *const()) -> RawWaker { - increase_refcount::(data); - RawWaker { - data: data, - vtable: waker_vtable!(T), - } -} - -unsafe fn drop_arc_raw(data: *const()) { - // Drop Arc - let _: Arc = Arc::from_raw(data as *const T); -} - -unsafe fn wake_arc_raw(data: *const()) { - let arc: Arc = Arc::from_raw(data as *const T); - ArcWake::wake(&arc); - let _ = Arc::into_raw(arc); -} +use arc_wake::ArcWake; struct Counter { wakes: AtomicUsize, From a1c4cf6889f69dc335a3f0b42b29e4d9a081005d Mon Sep 17 00:00:00 2001 From: Matthias Einwag Date: Wed, 6 Feb 2019 22:56:33 -0800 Subject: [PATCH 8/9] Change RawWaker constructor to const fn --- src/libcore/task/wake.rs | 28 +++++++++++++++++++++---- src/test/run-pass/auxiliary/arc_wake.rs | 10 ++------- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/libcore/task/wake.rs b/src/libcore/task/wake.rs index a877e033bc63b..21f0a8cea4168 100644 --- a/src/libcore/task/wake.rs +++ b/src/libcore/task/wake.rs @@ -19,9 +19,29 @@ pub struct RawWaker { /// that is associated with the task. /// The value of this field gets passed to all functions that are part of /// the vtable as the first parameter. - pub data: *const (), + data: *const (), /// Virtual function pointer table that customizes the behavior of this waker. - pub vtable: &'static RawWakerVTable, + vtable: &'static RawWakerVTable, +} + +impl RawWaker { + /// Creates a new `RawWaker` from the provided `data` pointer and `vtable`. + /// + /// The `data` pointer can be used to store arbitrary data as required + /// by the executor. This could be e.g. a type-erased pointer to an `Arc` + /// that is associated with the task. + /// The value of this poiner will get passed to all functions that are part + /// of the `vtable` as the first parameter. + /// + /// The `vtable` customizes the behavior of a `Waker` which gets created + /// from a `RawWaker`. For each operation on the `Waker`, the associated + /// function in the `vtable` of the underlying `RawWaker` will be called. + pub const fn new(data: *const (), vtable: &'static RawWakerVTable) -> RawWaker { + RawWaker { + data, + vtable, + } + } } /// A virtual function pointer table (vtable) that specifies the behavior @@ -102,8 +122,8 @@ impl Waker { /// Creates a new `Waker` from [`RawWaker`]. /// /// The behavior of the returned `Waker` is undefined if the contract defined - /// in [RawWaker]'s documentation is not upheld. Therefore this method is - /// unsafe. + /// in [`RawWaker`]'s and [`RawWakerVTable`]'s documentation is not upheld. + /// Therefore this method is unsafe. pub unsafe fn new_unchecked(waker: RawWaker) -> Waker { Waker { waker, diff --git a/src/test/run-pass/auxiliary/arc_wake.rs b/src/test/run-pass/auxiliary/arc_wake.rs index 0baaa378046f3..034e378af7f19 100644 --- a/src/test/run-pass/auxiliary/arc_wake.rs +++ b/src/test/run-pass/auxiliary/arc_wake.rs @@ -25,10 +25,7 @@ pub trait ArcWake { let ptr = Arc::into_raw(wake) as *const(); unsafe { - Waker::new_unchecked(RawWaker{ - data: ptr, - vtable: waker_vtable!(Self), - }) + Waker::new_unchecked(RawWaker::new(ptr, waker_vtable!(Self))) } } } @@ -44,10 +41,7 @@ unsafe fn increase_refcount(data: *const()) { unsafe fn clone_arc_raw(data: *const()) -> RawWaker { increase_refcount::(data); - RawWaker { - data: data, - vtable: waker_vtable!(T), - } + RawWaker::new(data, waker_vtable!(T)) } unsafe fn drop_arc_raw(data: *const()) { From 1ef34a5a39641846e824b6450a705d6031002beb Mon Sep 17 00:00:00 2001 From: Matthias Einwag Date: Fri, 8 Feb 2019 18:47:19 -0800 Subject: [PATCH 9/9] Remove rustdoc test which referenced unstable API --- src/test/rustdoc-js/substring.js | 10 ---------- 1 file changed, 10 deletions(-) delete mode 100644 src/test/rustdoc-js/substring.js diff --git a/src/test/rustdoc-js/substring.js b/src/test/rustdoc-js/substring.js deleted file mode 100644 index 3a6750151f7d8..0000000000000 --- a/src/test/rustdoc-js/substring.js +++ /dev/null @@ -1,10 +0,0 @@ -// exact-check - -const QUERY = 'waker_from'; - -const EXPECTED = { - 'others': [ - { 'path': 'std::task', 'name': 'local_waker_from_nonlocal' }, - { 'path': 'alloc::task', 'name': 'local_waker_from_nonlocal' }, - ], -};