Tracking Issue for `waker_getters`

[oxalica]

Feature gate: #![feature(waker_getters)]

This is a tracking issue for getters of data and vtable pointers for RawWaker, and the method to get RawWaker from Waker.

Public API

// core::task
impl RawWaker {
    pub fn data(&self) -> *const ();
    pub fn vtable(&self) -> &'static RawWakerVTable;
}
impl Waker {
    pub fn as_raw(&self) -> &RawWaker;
}

Steps / History

• [x] Implementation: #91828
• [ ] Final comment period (FCP)
• [ ] Stabilization PR

Unresolved Questions

• Do we also need fn into_raw(self) -> RawWaker for Waker? https://github.com/rust-lang/rust/issues/87021#issuecomment-1091899591

[oxalica]

Maybe we could stabilize this? Note sure what I could do next. @rust-lang/libs-api

[Amanieu]

I am concerned about as_raw, it's not clear to me how it is intended to be used. It takes Waker by reference and returns a RawWaker, which you can then decompose into a vtable and data pointer. However I don't see how you can then re-construct a reference to the original Waker: doing so with from_raw will create a second Waker which will call the drop function in the vtable when dropped. This can lead to a double-drop once the original Waker is dropped as well.

[oxalica]

@Amanieu

I am concerned about as_raw, it's not clear to me how it is intended to be used. It takes Waker by reference and returns a RawWaker

It returns &RawWaker, not RawWaker, thus it's a getter rather than an ownership transfer. Its purpose is to allow getting or checking the underlying data. Reconstructing a [Raw]Waker from the result pointers is indeed incorrect, and unnecessary due to the existence of Waker::clone. If we do want reconstruction, I'd consider adding another fn into_raw(Waker) -> RawWaker, which also consists with Waker::from_raw. But I think it's less useful, because we can only access &mut Context and get a &Waker. We don't own Waker without explicitly cloning it somewhere.

[Amanieu]

Then I'm afraid that I don't understand how as_raw is intended to be used. What can you actually do with the returned vtable and data? I can understand passing it through FFI, but then you eventually have to pass it back to Rust code to use it as a Waker.

[oxalica]

@Amanieu

Then I'm afraid that I don't understand how as_raw is intended to be used.

It's effectively downcast_ref for Waker. It allows validating Waker VTables, ad-hoc optimization, or retrieving data from known Wakers, where the async runtime only supports Wakers from itself. I found an example in, https://github.com/embassy-rs/embassy/blob/be55d2a39eb399d693d11f37dfa4b87cd2bd3c7a/embassy-executor/src/raw/waker.rs#L42

I can understand passing it through FFI, but then you eventually have to pass it back to Rust code to use it as a Waker.

In case of passing through FFI, we need do &Waker -> [*const (); 2] -> &Waker [^1] without ownership transfer. The last step can be done via reconstructing ManuallyDrop<Waker> and using the reference to inner. If we want to transfer the ownership via, to say fn into_raw(Waker) -> RawWaker, we need an extra Waker::clone during every poll.

Note that the ManuallyDrop<Waker> - &Waker pattern above can be found in, https://github.com/tokio-rs/tokio/blob/c693ccd210c7a318957b0701f4a9632b2d545d6a/tokio/src/util/wake.rs#L18-L22

[^1]: Since the VTable representation is repr(Rust). Here we actually need to create a new repr(C) VTable for forwarding method calls, rather than to pass the pointer through FFI directly. This is doable currently.

[Amanieu]

I feel that a better story is needed for getting a &Waker from a &RawWaker. Perhaps something like this?

impl Waker {
    unsafe fn with_raw<T>(raw: &RawWaker, f: impl FnOnce(&Waker) -> T) -> T;
}

Also I do think that we should add into_raw for consistency with from_raw.

[oxalica]

I feel that a better story is needed for getting a &Waker from a &RawWaker. Perhaps something like this?

impl Waker {
    unsafe fn with_raw<T>(raw: &RawWaker, f: impl FnOnce(&Waker) -> T) -> T;
}

This API seems to be the only choice since we cannot let temporary Waker instance escape, but it would be less flexible for the tokio case. It's not possible to return a impl Deref<Target = Waker> via this API. For the FFI case, it seems to work though.

Also this API is more like a helper, which is able to be implemented outside std (RawWaker::data cannot, due to repr(Rust)). Should it also be a part of waker_getters?

[Amanieu]

I think it should be inlcuded as the counterpart to Waker::as_raw, otherwise it isn't clear how the RawWaker returned by as_raw can be safely used. This is important for the doc examples for these functions.

For tokio, it seems to only be used here. It should be easy to refactor that to use with_raw.

[oxalica]

@Amanieu Noticing that Waker is repr(transparent), it should be safe to transmute from &RawWaker to &Waker I think? Then the with_raw you suggested can be replaced by,

impl Waker {
    pub unsafe fn from_raw_ref(raw: &RawWaker) -> &Waker { transmute(raw) }
}

[wishawa]

~~I have a pretty simple use case for this: checking if two Wakers are the same.~~

~~The Future contract requires wake to be called on the Waker from the latest poll, so on every poll a Future that keeps its Waker stored have to replace its stored Waker with one newly cloned from the Context. Waker::as_raw would allow us to first compare the two Wakers, avoiding replacement (and the atomic operations involved) if they are the same.~~

Didn't realize will_wake does this already! Thanks @Thomasdezeeuw

[Amanieu]

Because Waker has private fields, the #[repr(transparent)] is an implementation detail and not an API guarantee that users can rely on. It isn't shown in rustdoc for this reason.

The problem with from_raw_ref is that it effectively turns this into a public guarantee: it might be something we can commit to but it would have a higher barrier for acceptance.

[Thomasdezeeuw]

To add another use case is storing the waker atomically. You can split the data and vtable into two AtomicU64 and store those atomically separately (with some careful programming) or in a single AtomicU128 (on 64 bit archs at least).

I have a pretty simple use case for this: checking if two Wakers are the same.

The Future contract requires wake to be called on the Waker from the latest poll, so on every poll a Future that keeps its Waker stored have to replace its stored Waker with one newly cloned from the Context. Waker::as_raw would allow us to first compare the two Wakers, avoiding replacement (and the atomic operations involved) if they are the same.

@wishawa wote that task::Waker::will_wake exists for this.

[conradludgate]

I think it might be natural to think that a Waker could do more than just wake(). Like how a Box<dyn Error> might be a specific kind of error with some fields we would care about.

I can imagine as situation where you might want to check if the current Waker is a FooWaker in order to extract a field from it, eg a TaskID. (I am exploring an idea at the moment to hack in a go like context.Context typemap into core::task::Context through this mechanism.)

In order to do this, I could verify that waker.raw_waker().vtable() == &FooWakerVTable.

(in a similar respect to Error and source, it would be interesting to have a waker chain, especially with the recent push to structured concurrency with many nested sub-executors, but that is for a separate issue and another time)

[p-avital]

Hi, now that stabby is out, I'm eagerly awaiting accessors for the contents of the vtable and the date pointer, so that they may be passed over the ffi frontier in an ffi safe manner without having to wrap the waker in a stable trait object at every poll, which is what stabby is currently forced to do to safely support futures being passed across the ffi frontier.

Note that since the vtable isn't repr(c), and the functions use the default calling convention, access to the vtable but not the functions in contains is not sufficient: I'd need accessors to at least one of the wakes, clone and drop.

As a second point, is there a good reason why the function pointers use the (unstable) rust calling convention? This means the function pointers couldn't be passed across ffi even if they were accessible...

Anything I could do to help with the process?

[p-avital]

Hi again,

I've started a new RFC (https://github.com/rust-lang/rfcs/pull/3404) which, while orthogonal code-wise, has the same end-purpose of allowing the safe passage of wakers across the FFI.

A key issue that waker_getters don't address is that the vtable is composed of extern "rust" function pointers, which can't safely cross the FFI bounday. My proposed solution involves modifying the current layout of RawWakerVTable, which would make accessing the vtable's inner function pointers fallible.

I just wanted this stated somewhere to avoid stabilizing accessors that would prevent the vtable from being made truly FFI-safe without breaks in the accessors API. For example, an eventual RawWakerVTable::get_wake_fn(&self) -> unsafe fn(*const ()) would prevent proposing layouts for the vtable that may not contain said function pointer. Should such accessors be proposed, I strongly suggest considering making them of the form RawWakerVTable::get_wake_fn(&self) -> Option<unsafe fn(*const ())>

[dvdhrm]

I think it might be natural to think that a Waker could do more than just wake(). Like how a Box<dyn Error> might be a specific kind of error with some fields we would care about.

I can imagine as situation where you might want to check if the current Waker is a FooWaker in order to extract a field from it, eg a TaskID. (I am exploring an idea at the moment to hack in a go like context.Context typemap into core::task::Context through this mechanism.)

In order to do this, I could verify that waker.raw_waker().vtable() == &FooWakerVTable.

We ended up doing the exact same thing. These getters allow us to provide executor-specific extensions on std::task::Context, by verifying that a specific context is indeed one of our contexts (kind of RTTI).

Most async-runtimes have global executors that you use to spawn tasks. But with feature(waker_getters) you can actually implement features like Executor::spawn(f: impl Future, cx: std::task::Context) -> Executor::Task which would use the passed std::task::Context to access state of the calling future and allow spawning another future on the same executor / scheduling-class / etc. You can also use it to modify scheduling priorities / properties of the calling future, and much more.

Without this you need a lookup-tree to get your internal state from a public Context, even though the data is already stored in the context.

[jkarneges]

One way to construct a value within a function and return its reference is to have its memory location passed in. It's a little gross, but for example:

impl Waker {
    unsafe fn from_raw<'a, 'b: 'a>(
        raw: &'b RawWaker,
        output_mem: &'a mut MaybeUninit<Waker>,
    ) -> &'a Waker {
        // MaybeUninit doesn't drop
        output_mem.write(Waker::from_raw(RawWaker::new(raw.data(), raw.vtable())));
        output_mem.assume_init_ref()
    }
}

EDIT: fixed the code

[AldaronLau]

Would it make sense to have the API look like this?

// core::task
impl RawWaker {
    pub fn data(&self) -> *const ();
    pub fn vtable(&self) -> &'static RawWakerVTable;
}

impl Waker {
    pub unsafe fn clone_from_raw(waker: &RawWaker) -> Waker;
    pub fn as_raw(&self) -> &RawWaker;
    pub fn into_raw(self) -> RawWaker;
}

Or a slightly different take:

// core::task
impl RawWaker {
    pub fn data(&self) -> *const ();
    pub fn vtable(&self) -> &'static RawWakerVTable;
}

impl Waker {
    // Maybe named something different?
    pub fn raw_with<T>(&self, f: impl FnOnce(&RawWaker) -> T) -> T;
    pub fn into_raw(self) -> RawWaker;
}

[wyfo]

My issue with this feature is that it's not really usable with alloc::task::Wake. Could we add the following TryFrom implementations?

impl<W: Wake + Send + Sync + 'static> TryFrom<Waker> for Arc<W> {/*...*/}

impl<'a, W: Wake + Send + Sync + 'static> TryFrom<&'a, Waker> for &'a Arc<W> {/*...*/}

(for the symmetry with the already existing From implementation) Should it be moved in another feature?

[jkarneges]

What's left to move forward with this? It would enable Context extension experimentation outside of std, which could help accelerate async runtime interop.

[withoutboats]

(NOT A CONTRIBUTION)

This would be a useful API & I'd like to see it stabilized. I don't think any of the discussion on this issue should be blocking on stabilizing the APIs already under this feature.

My motivation is for a tightly bound executor/reactor: I want to check in the reactor if a registered waker has the vtable for "my" executor, so that I can avoid making the virtual wake call and directly re-enqueue the task. I don't need to pass the Waker through FFI, I don't need to get a waker back from a RawWaker, these are not necessary features for this API to be useful.

Because Waker has private fields, the #[repr(transparent)] is an implementation detail and not an API guarantee that users can rely on. It isn't shown in rustdoc for this reason.

I don't think the conversion from &RawWaker to &Waker is needed for this API to be useful, but I also find it extremely hard to imagine Waker ever gaining any other fields. We added Context specifically to have a place to add fields that wasn't Waker. I think T-libs should guarantee the layout of Waker.

[dtolnay]

@rust-lang/libs-api: @rfcbot fcp merge

I propose that we stabilize the following 3 accessors, which have been available since Rust 1.60-nightly (2 years).

// core::task
impl RawWaker {
    pub fn data(&self) -> *const ();
    pub fn vtable(&self) -> &'static RawWakerVTable;
}
impl Waker {
    pub fn as_raw(&self) -> &RawWaker;
}

The use case is described succinctly and convincingly in https://github.com/rust-lang/rust/issues/96992#issuecomment-1343487771.

It's effectively downcast_ref for Waker. It allows validating Waker VTables, ad-hoc optimization, or retrieving data from known Wakers, where the async runtime only supports Wakers from itself.

Here is code in embassy-executor that needs downcasting, and does it in a nasty way today to work around the lack of stable accessors. The workaround involves assuming implementation details of the standard library's data structures and compiler's struct layout.

fn task_from_waker(waker: &Waker) -> TaskRef {
    ///🤞
    struct WakerHack {
        data: *const (),
        vtable: &'static RawWakerVTable,
    }

    let hack: &WakerHack = unsafe { mem::transmute(waker) };
    if hack.vtable != &VTABLE {
        panic!("Found waker not created by the Embassy executor. `embassy_time::Timer` only works with the Embassy executor.");
    }

    unsafe { TaskRef::from_ptr(hack.data as *const TaskHeader) }
}

By making stable accessors available, this would be:

fn task_from_waker(waker: &Waker) -> TaskRef {
    let raw_waker = waker.as_raw();     // <--
    if raw_waker.vtable() != &VTABLE {  // <--
        panic!("...");
    }
    let ptr = raw_waker.data();         // <--
    unsafe { TaskRef::from_ptr(ptr as *const TaskHeader) }
}

The importance of this use case is reinforced by https://github.com/rust-lang/rust/issues/96992#issuecomment-1900106571, and https://github.com/rust-lang/rust/issues/96992#issuecomment-1880141049.

Alternatives

I would be open to considering making public fields on RawWaker, instead of accessors. This is in fact what is shown in the core::task stabilization RFC. As far as I can tell, https://github.com/rust-lang/rfcs/pull/2592#issuecomment-458944053 is what precipitated the change to private fields under the possibility that adding a 3rd field Option<TypeId> would turn out to be useful, which seems obsolete as a rationale.

// core::task
pub struct RawWaker {
    pub data: *const (),
    pub vtable: &'static RawWakerVTable,
}
impl RawWaker {
    pub const fn new(data: *const (), vtable: &'static RawWakerVTable) -> Self;  // (already stable)
}

Or, move the accessors to Waker and phase out the use of RawWaker. I tried looking into why Waker and RawWaker are separate types and failed to find a good reason. My guess is this pre-dates the separation of Waker from Context, and might no longer be well motivated now that Context serves as our extension point going forward (for things like LocalWaker support) and Waker will remain permanently just a RawWaker as articulated in https://github.com/rust-lang/rust/issues/96992#issuecomment-1900106571.

// core::task
pub struct Waker {
    data: *const (),
    vtable: &'static RawWakerVTable,
}
impl Waker {
    pub const unsafe fn new(data: *const (), vtable: &'static RawWakerVTable) -> Self;
    pub fn data(&self) -> *const ();
    pub fn vtable(&self) -> &'static RawWakerVTable;
    #[deprecated]
    pub unsafe fn from_raw(waker: RawWaker) -> Self;
}

[rfcbot]

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

• [x] @Amanieu
• [x] @BurntSushi
• [x] @dtolnay
• [ ] @joshtriplett
• [ ] @m-ou-se

Concerns:

• Alternative 1: public RawWaker fields (https://github.com/rust-lang/rust/issues/96992#issuecomment-1902780569)
• Alternative 2: phase out RawWaker (https://github.com/rust-lang/rust/issues/96992#issuecomment-1902780569)

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[dtolnay]

@rfcbot concern Alternative 1: public RawWaker fields @rfcbot concern Alternative 2: phase out RawWaker

[joshtriplett]

:+1: for going with alternative 1. (Please feel free to check my box for an FCP that proposes alternative 1.)

[dtolnay]

@rfcbot fcp cancel

We talked about this in today's libs-api meeting and think we're on board with giving RawWaker public data and vtable fields, rather than a .data() and .vtable() accessor method

Let me go ahead and FCP that (Alternative 1 from above).

[rfcbot]

@dtolnay proposal cancelled.

[dtolnay]

@rust-lang/libs-api: @rfcbot fcp merge

I propose stabilizing the following counterproposal:

// core::task
impl Waker {
    pub fn as_raw(&self) -> &RawWaker;  // unstable since 1.60-nightly
}
pub struct RawWaker {
    pub data: *const (),                  // change from private to public
    pub vtable: &'static RawWakerVTable,  // change from private to public
}

and deleting these 2 unstable accessors currently tracked by this tracking issue:

impl RawWaker {
    pub fn data(&self) -> *const ();
    pub fn vtable(&self) -> &'static RawWakerVTable;
}

RawWaker was originally stabilized with private fields related to a plan to maybe add a 3rd field Option<TypeId> (https://github.com/rust-lang/rfcs/pull/2592#issuecomment-458944053).

An Option<TypeId> field was envisioned to support downcasting Waker based on the type of object pointed to by data.

Instead, modern implementations such as embassy have done downcasting based on the value of vtable, not the type of the object pointed by data. This is just as good and no separate TypeId is needed. See https://github.com/rust-lang/rust/issues/96992#issuecomment-1902780274 for links to the relevant code.

[rfcbot]

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

• [x] @Amanieu
• [ ] @BurntSushi
• [x] @dtolnay
• [x] @joshtriplett
• [ ] @m-ou-se

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[BurntSushi]

Is there a downside to using public accessors? It seems like the more conservative route. And are there compelling use cases specifically for exposing the fields?

(I don't have any use case in mind for adding another field to RawWaker. I am only coming at this from the angle of preserving for the future unless there's a good reason not to.)