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

task: use pin-project for TaskLocalFuture #5758

Merged
merged 4 commits into from Jun 10, 2023

Conversation

BugenZhao
Copy link
Contributor

Motivation

As pin-project-lite supported custom Drop implementation in taiki-e/pin-project-lite#25, I suppose it's worthwhile switching back to the pin-project implementation to eliminate unsafe code for TaskLocalFuture.

Solution

Implement TaskLocalFuture with pin_project_lite::pin_project! with PinnedDrop.

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Comment on lines +333 to +334
#[pin]
_pinned: PhantomPinned,
Copy link
Contributor Author

@BugenZhao BugenZhao Jun 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really clear about whether we need this Future to be !Unpin, as making it Unpin seems to make life easier in some situations.

For example, if the feature in #5743 is added, it'll be common that we poll this with &mut fut to avoid the future being consumed, then we're able to take the value back after getting Ready. If the future is always !Unpin, we are not able to take back the ownership of the future. :(

Currently, I leave it untouched according to #3943 (comment). IIUC, this is considered to be a breaking change in the compatibility?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the PhantomPinned field would not be breaking, but we could never undo it because that would be breaking.

As for into_value, you make a good point. That method would not be able to take ownership of the future.

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@Darksonn Darksonn added A-tokio Area: The main tokio crate M-task Module: tokio/task labels Jun 4, 2023
@Darksonn Darksonn requested a review from taiki-e June 4, 2023 11:13
Copy link
Member

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Darksonn Darksonn merged commit e63d0f1 into tokio-rs:master Jun 10, 2023
58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-task Module: tokio/task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants