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

• Implementation: Implement RawWaker and Waker getters for underlying pointers #91828
• Final comment period (FCP)
• Stabilize waker_getters #129919

Unresolved Questions

• Do we also need fn into_raw(self) -> RawWaker for Waker? Feature: Provide getters of data and vtable pointer for RawWaker? #87021 (comment)

[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 ¹ 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

Footnotes

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 (rust-lang/rfcs#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 #96992 (comment).

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 #96992 (comment), and #96992 (comment).

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, rust-lang/rfcs#2592 (comment) 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 #96992 (comment).

// 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]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[madsmtm]

I... realize that this stabilization process has gone on for a while, so totally understand if you don't want to bother with re-doing it, but:

Given that RawWaker is clearly still useful, and may even be used in LocalWaker, I believe that Alternative 2 #96992 (comment), stabilizing the fields of RawWaker, feels like the more coherent solution.

(A very hypothetical example: if the RawWakerVTable fields ever become public, if I end up manually calling the clone function, I have to construct a Waker to be able to access the fields of the RawWaker returned from that function, which seems backwards).

---

I'd also like to add, if you don't want to commit to a specific number of fields in RawWaker, you could consider adding #[non_exhaustive].

[jkarneges]

This will be merged soon.

It looks like there was a vote to merge, but this issue isn't a PR and it remains open. What's next here?

[wyfo]

As I have already mentioned, this feature is not really usable with alloc::task::Wake.

Could it be possible to add the following implementation?

impl<W: Wake + Send + Sync + 'static> Arc<W> {
    const fn waker_vtable() -> &'static RawWakerVTable { ... }
}

It would allow to match the vtable returned by RawWaker::vtable.

[conradludgate]

this feature is not really usable with alloc::task::Wake.

Does Waker::from(&arc_wake).as_raw().vtable() not work?

[wyfo]

Does Waker::from(&arc_wake).as_raw().vtable() not work?

Yes it works, but it requires to already have an allocated arc_wake, which makes the thing unusable.

The goal is to compare the vtable of a random waker to know if I can cast its data pointer to my own waker implementation. I don't want to rely for this comparison on a lazily allocated waker to get its vtable (especially as this lazy waker may requires some parameters random to be initialized). In short, this is unusable.
That's too bad, because Wake is a convenient implementation shortcut, but I have to rewrite it myself to use this feature.

[Hawk777]

If I understand correctly, the FCP has happened and ended with a disposition to merge. Is there some reason why the FCP checkbox in the first comment is still unticked? In terms of the stabilization PR, is that just waiting for someone to do it, or is there some missing prerequisite?

[Amanieu]

This is just waiting for someone to submit a stabilization PR.

[kevinmehall]

Submitted a stabilization PR: #129919