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

#[derive(Clone)] for Pending. #1918

Merged
merged 2 commits into from
Oct 22, 2019
Merged

Conversation

EkardNT
Copy link
Contributor

@EkardNT EkardNT commented Oct 21, 2019

Description

Make futures::future::Pending<T> implement Clone.

Motivation

I am writing a program that uses tokio-timer's Delay in a loop to asynchronously sleep for a long time, e.g. 5 minutes. If my program receives a shutdown signal I want to interrupt the sleep early and exit the loop. My plan for doing so was to use the Abortable future support from this crate to propagate the shutdown signal.

My loop would basically look like this:

let (shutdown_future, shutdown_handle) = abortable(pending::<()>());
while let Either::Right(_) = select(shutdown_future, tokio::timer::delay_for(FIVE_MINUTES)).await {
    // Do work
}
// Then in some signal handler I could trigger graceful shutdown.
when_shutdown_signal_received(move || {
    shutdown_handle.abort();
});

The only problem is that I am running this loop on multiple threads, so Abortable<Pending<()>> needs to be Send, Sync, and Clone. Abortable is all three already and Pending is Send + Sync, but not Clone. I worked around this by writing my own Pending struct that is Clone, but I'd prefer to use the one from futures-util.

Alternatives

An alternative to this PR would be if there is a simpler preexisting way of accomplishing my goal above that I don't know about. I tried to use the oneshot channel, but the Receiver is not Clone, which I think makes sense given that its a single-consumer channel.

@Nemo157
Copy link
Member

Nemo157 commented Oct 21, 2019

derive(Clone) will only impl Clone where T: Clone, since Pending doesn't actually need to clone a value it should have a custom impl of Clone that is always available.

@EkardNT
Copy link
Contributor Author

EkardNT commented Oct 21, 2019

Good point. I've now made Clone unconditionally implemented for all T.

@EkardNT
Copy link
Contributor Author

EkardNT commented Oct 22, 2019

I checked the build failure and all the errors are either timeouts to download from static.rust-lang.org or "--all-features is not allowed in the root of a virtual workspace", neither of which seem to be related to my change.

@taiki-e
Copy link
Member

taiki-e commented Oct 22, 2019

"--all-features is not allowed in the root of a virtual workspace"

This is a cargo bug. (fixed in rust-lang/cargo#7525, but unreleased)

@cramertj cramertj merged commit 99d5ebe into rust-lang:master Oct 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants