If you think it's worth including a non-sync version as well, let me know and I'll add that. IMO the tiny perf difference is probably not worth the additional complexity.

This seems pretty slick to me! This form of cancellation feels like it's common enough in other libraries that it seems worthwhile to include here as well. I do think though yeah if we have a sync version it'd be wortwhile to have an unsync version too. Before that though I'd be curious to hear others' thoughts on a combinator like this, @aturon perhaps?

At a high level.

• I'm +1 on the feature.
• I don't love the name... maybe ShortCircuit or Abort. Generally, it seems that '*able' is not a common naming idiom and "kill" sounds violent! (but I'm fine being overridden on this point).
• Why does it require the inner future to be of ()? Also, the fact that the inner future was aborted is not exposed.
• I'm not a fan of the proliferation of of unsync duplication, but that ship has sailed, so ¯\_(ツ)_/¯.

Going in more detail, I'm not sure about either the implementation or the API. The implication of Killable taking &KillHandle is that the same KillHandle can be used to back multiple Killable instances. However, the implementation does not support this.

I would say either the implementation should change or the API should not allow constructing a Killhandle standalone.

@carllerche I need to be able to construct a KillHandle standalone so that I can cancel an outer future from deep inside-- think a request handler wanting to kill a service. If you only provide a KillHandle after constructing the top-level Killable, there's no way to do this.

I do agree, though, that it's misleading and user might think they could use one KillHandle for multiple futures. Perhaps make a constructor that returns both a KillHandle and a KillRegistration, and the KillRegistration must be moved into Killable::new?

R.E. Naming: bikeshed away. I think I still personally prefer Cancellable and CancelHandle, but that does have downsides.

@carllerche

Why does it require the inner future to be of ()? Also, the fact that the inner future was aborted is not exposed.

How would you support other types? The cancellation path only knows how to return (). I guess you could also allow the user to supply a default if-cancelled value, but then the API quickly gets more complex. Most of the use-cases I have for this are things that are being spawned or are otherwise top-level side-effect-y futures.

How would you support other types?

An enum.

Most of the use-cases I have for this are things that are being spawned or are otherwise top-level side-effect-y futures.

Should this API be tied to the executor then? It could be an additional fn on the Executor trait that returns the cancel handle.

I don't have strong opinions either way. I'm mostly just brainstorming here.

An enum.

Cancellable<T> impls Future<Item=Option<T::Item>>? Seems fine to me. Just means I'll be adding one more .map(|_| ()).

To be clear, I don't necessarily think my idea to support more than Item=() inner futures is a good idea, but I am proposing it for discussion to see what others think of the pros / cons trade off.

Yeah it seems like a reasonable idea.

Another question is whether future has to be canceled when KillHandle/CancelHandle is dropped.

This is useful if you keep a handle in some structure just to cancel the future when not needed anymore. And that structure may occasionally drop unexpectedly (e.g. on panic). Currently, I use often use oneshot with select and it's handy that it cancels future on dropping oneshot::Sender too. So this use case is quite real.

@tailhook I'd personally be opposed to cancelling on on drop of the CancelHandle-- it seems like surprising behavior to me and I'd have to be careful to avoid it.

@tailhook I'd personally be opposed to cancelling on on drop of the CancelHandle-- it seems like surprising behavior to me and I'd have to be careful to avoid it.

Well, you have to be careful both ways. You're likely to have a memory leak if CancelHandle is dropped without cancelation. Can you describe your real life use case?

You're likely to have a memory leak if CancelHandle is dropped without cancelation.

I just use this to kill futures early or to kill a future from another future deep inside without plumbing through some kind of shared flag. In the first case, the future would still be expected to complete normally, and in the second case, it doesn't make any sense why an inner future dropping should kill the outer one.

@cramertj I wonder if all this is evidence that we may want to let this bake externally first? That way we could iterate on it elsewhere and bring it in once it's stabilized?

@alexcrichton I'd be happy to throw it up in its own repo somewhere, but I'm not sure we'll get more useful feedback from having it there than we would by continuing to discuss here (or on IRC or something).

IMO we should fix the "out of crate" discover ability problem.

I would suggest taking the following two steps:

• Add a github wiki page that includes an organized list of (somewhat curated) third party utilities.
• Add a link to the github page in the futures-rs documentation explaining that there are additional utilities provided in third party crates.

@cramertj it's true yeah, seems like a good idea though to have a piece of documentation somewhere listing extensions for anyone interested.

Updated in response to comments.

@cramertj would you still be looking to merge in this repo? Or would you be ok having it out of repo for a bit?

@alexcrichton Up to you-- my preference would be to try and get in good shape to be merged into futures since it seems like a fairly common thing to want to do. If you'd prefer to have it available through a separate crate, I'll need someone else to make the repo (Google reasons). @aturon previously suggested starting a tokio-tools crate as a proving ground for combinators-- that could be a place for this as well.

@cramertj well, it seems like discoverability is a general problem. Do you not think that my suggestion above would aid in that regards?

@carllerche I disagree that discoverability is the main problem here. I think the main problem is that we need a place to incubate standard, well-motivated combinators without tying them to futures/tokio stability guarantees. I think a better solution to this would be to have a dedicated futures/tokio combinator library that can frequently release breaking changes. Breaking releases to combinators won't result in the interoperability problems that frequent breaking changes to the futures library will because they won't break the interface exposed by libraries and applications or cause interoperability problems. Libraries/applications could freely use multiple versions of the combinator libraries.

FWIW, I think this applies as well to methods like map, and_then, etc. on the actual Future and Stream types. Over time there've been a number of problems discovered with various combinator methods that would require breaking changes. If these combinators had been implemented via an extension trait in a separate library, it would be easy to make the appropriate fixes. However, the fact that they're in the main futures repo and even the main Future trait means that we can't make changes without breaking the fundamental interface of the library.

@cramertj I think you hit the nail on the head there with the problem of tying combinators to the stability of the Future trait itself (and how breakage in the combinators is totally ok, it's just Future which can't break.

This sounds a lot like itertools, though? It definitely seems to me like we should start a futuretools (to make clear it's not Tokio-specific) crate and probably consider moving a number of standard combinators to the crate if we ever do a major release of futures itself.

@alexcrichton Yup, +1 for futuretools or similar.

Yes, I'm definitely for creating another crate and moving stuff there. IMO the core traits already have too many fns on them, which causes problems for iteration.

I didn't know moving stuff to another crate was on the table, so this makes me happy.

@cramertj would you be willing to maintain such a crate? I'm not sure we have a great place for it right now...

Note for folks reading: futures-tools has been created.

Is there a reason the crate isn't included in this git repository? The cargo workspace feature would make it easy.

It seems that if the plan is to migrate code between the two repos (move a bunch of stuff from here into -tools in futures 0.2 then incrementally "promote" as APIs stabilize) it would be easiest to do so in one repository.

@carllerche That approach sounds good to me.

Side question: do we ever want to promote? For that matter, we could make futures the "with combinators" library and futures-traits the base library, and reexport futures_traits::{Future, Stream, Sink} from futures. That way the internal compatibility mechanisms would be hidden from end-users and they could still depend on one crate for simplicity's sake.

One downside of this approach would be that it might not be clear to end users when a breaking change to futures corresponded to a breaking change to futures-traits, so they might mistakenly make breaking changes.

Perhaps discussion for a futures-util crate should be promoted to a dedicated issue at this point :)

@carllerche @alexcrichton @aturon I've opened #711 for discussion.

@cramertj want to retarget at 0.2 to merge?

@cramertj what's the status here?

Is AtomicTask used to make it safe to move the future to different tasks? What about the case where I'm just doing tokio::spawn(killable_future)? It can't change tasks then, right? So

Any chance to get this in for 0.3?

Closing, opened #1079 targeting 0.3.