Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Provide getters of data and vtable pointer for RawWaker? #87021

Closed
oxalica opened this issue Jul 10, 2021 · 9 comments
Closed

Feature: Provide getters of data and vtable pointer for RawWaker? #87021

oxalica opened this issue Jul 10, 2021 · 9 comments
Labels
A-async-await Area: Async & Await AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@oxalica
Copy link
Contributor

oxalica commented Jul 10, 2021

Docs of std::task::RawWaker:

It consists of a data pointer and a virtual function pointer table (vtable) that customizes the behavior of the RawWaker.

We said it consists of two pointer, but doesn't enforce its layout by repr, nor provide methods to get data and vtable pointer from a constructed RawWaker. So it impossible to destruct a RawWaker even with transmute, since the layout is unknown, which makdes it difficult to pass RawWaker or Waker across FFI boundary.

Note that Waker has repr(transparent) on it so it's able to transmute into a RawWaker. But it barely does anything since we can do nothing with a RawWaker. I'm confusing about why there is a repr(transparent) struct to an blackbox struct without any methods.

If it's intended to NOT stabilizing the layout of std::task::RawWaker. I think we should at least provide methods like fn data(&self) -> *mut () and fn vtable(&self) -> &'static RawWakerVTable to do the inverse of RawWaker::new, which can be used to destruct it to cross FFI boundary.

@GuillaumeGomez GuillaumeGomez added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Jul 10, 2021
@crlf0710 crlf0710 added the A-async-await Area: Async & Await label Nov 28, 2021
@yoshuawuyts
Copy link
Member

We discussed this during today's triage meeting, and the async foundations WG is generally +1 on this idea.

cc/ @rust-lang/libs what do you think the right steps here should be?

@cuviper
Copy link
Member

cuviper commented Nov 29, 2021

It makes sense to me, but it's more precisely a question for @rust-lang/libs-api.

@cuviper cuviper added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Nov 29, 2021
@inquisitivecrystal inquisitivecrystal added the I-needs-decision Issue: In need of a decision. label Nov 29, 2021
@tmandry
Copy link
Member

tmandry commented Nov 29, 2021

wg-async-foundations discussed and we're generally supportive of the idea, but agreed it's the libs API team who owns this.

@yaahc yaahc added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Nov 30, 2021
@joshtriplett
Copy link
Member

joshtriplett commented Dec 1, 2021

We discussed this in today's @rust-lang/libs-api meeting. We're in favor of adding these methods, as inverses of RawWaker::new. Please feel free to send a PR adding these as unstable APIs and we'll r+ it.

@eholk
Copy link
Contributor

eholk commented Dec 6, 2021

@rustbot label +AsyncAwait-Triaged

@rustbot rustbot added the AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. label Dec 6, 2021
@m-ou-se m-ou-se removed I-libs-api-nominated Nominated for discussion during a libs-api team meeting. I-needs-decision Issue: In need of a decision. labels Dec 8, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 31, 2022
Implement `RawWaker` and `Waker` getters for underlying pointers

implement rust-lang#87021

New APIs:
- `RawWaker::data(&self) -> *const ()`
- `RawWaker::vtable(&self) -> &'static RawWakerVTable`
- ~`Waker::as_raw_waker(&self) -> &RawWaker`~ `Waker::as_raw(&self) -> &RawWaker`

This third one is an auxiliary function to make the two APIs above more useful. Since we can only get `&Waker` in `Future::poll`, without this, we need to `transmute` it into `&RawWaker` (relying on `repr(transparent)`) in order to access its data/vtable pointers.

~Not sure if it should be named `as_raw` or `as_raw_waker`. Seems we always use `as_<something-raw>` instead of just `as_raw`. But `as_raw_waker` seems not quite consistent with `Waker::from_raw`.~ As suggested in rust-lang#91828 (comment), use `as_raw`.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 31, 2022
Implement `RawWaker` and `Waker` getters for underlying pointers

implement rust-lang#87021

New APIs:
- `RawWaker::data(&self) -> *const ()`
- `RawWaker::vtable(&self) -> &'static RawWakerVTable`
- ~`Waker::as_raw_waker(&self) -> &RawWaker`~ `Waker::as_raw(&self) -> &RawWaker`

This third one is an auxiliary function to make the two APIs above more useful. Since we can only get `&Waker` in `Future::poll`, without this, we need to `transmute` it into `&RawWaker` (relying on `repr(transparent)`) in order to access its data/vtable pointers.

~Not sure if it should be named `as_raw` or `as_raw_waker`. Seems we always use `as_<something-raw>` instead of just `as_raw`. But `as_raw_waker` seems not quite consistent with `Waker::from_raw`.~ As suggested in rust-lang#91828 (comment), use `as_raw`.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 1, 2022
Implement `RawWaker` and `Waker` getters for underlying pointers

implement rust-lang#87021

New APIs:
- `RawWaker::data(&self) -> *const ()`
- `RawWaker::vtable(&self) -> &'static RawWakerVTable`
- ~`Waker::as_raw_waker(&self) -> &RawWaker`~ `Waker::as_raw(&self) -> &RawWaker`

This third one is an auxiliary function to make the two APIs above more useful. Since we can only get `&Waker` in `Future::poll`, without this, we need to `transmute` it into `&RawWaker` (relying on `repr(transparent)`) in order to access its data/vtable pointers.

~Not sure if it should be named `as_raw` or `as_raw_waker`. Seems we always use `as_<something-raw>` instead of just `as_raw`. But `as_raw_waker` seems not quite consistent with `Waker::from_raw`.~ As suggested in rust-lang#91828 (comment), use `as_raw`.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 1, 2022
Implement `RawWaker` and `Waker` getters for underlying pointers

implement rust-lang#87021

New APIs:
- `RawWaker::data(&self) -> *const ()`
- `RawWaker::vtable(&self) -> &'static RawWakerVTable`
- ~`Waker::as_raw_waker(&self) -> &RawWaker`~ `Waker::as_raw(&self) -> &RawWaker`

This third one is an auxiliary function to make the two APIs above more useful. Since we can only get `&Waker` in `Future::poll`, without this, we need to `transmute` it into `&RawWaker` (relying on `repr(transparent)`) in order to access its data/vtable pointers.

~Not sure if it should be named `as_raw` or `as_raw_waker`. Seems we always use `as_<something-raw>` instead of just `as_raw`. But `as_raw_waker` seems not quite consistent with `Waker::from_raw`.~ As suggested in rust-lang#91828 (comment), use `as_raw`.
@jstarks
Copy link

jstarks commented Apr 7, 2022

I was surprised to see that as_raw(&self) was added to Waker, but not into_raw(self). This can be worked around via transmute or as_raw + forget, but it's probably still good to round out the API (from_raw, as_raw, into_raw).

@oxalica
Copy link
Contributor Author

oxalica commented May 5, 2022

I was surprised to see that as_raw(&self) was added to Waker, but not into_raw(self). This can be worked around via transmute or as_raw + forget, but it's probably still good to round out the API (from_raw, as_raw, into_raw).

Typically, we can only get &Waker from Context without cloning. I'd think into_raw would be less useful.

@ultimaweapon
Copy link

I was tried to implement a single threaded async executor on the stable branch and just got bitten by this issue, which forced me to use a global variable to store the future-specific data when it got suspended.

@Amanieu
Copy link
Member

Amanieu commented Dec 8, 2022

Closing this in favor of the tracking issue #96992.

@Amanieu Amanieu closed this as completed Dec 8, 2022
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Dec 13, 2023
…ng_issue_number, r=workingjubilee

fix `waker_getters` tracking issue number

The feature currently links to the closed issue rust-lang#87021. Make it link to the tracking issue rust-lang#96992 instead.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Dec 13, 2023
Rollup merge of rust-lang#118873 - lukas-code:fix_waker_getter_tracking_issue_number, r=workingjubilee

fix `waker_getters` tracking issue number

The feature currently links to the closed issue rust-lang#87021. Make it link to the tracking issue rust-lang#96992 instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.