Skip to content

futures::channel::oneshot severe bug affecting cleanup

futures::channel::oneshot::Receiver has a broken implementation of FusedFuture which makes it not work in select!. If the Sender is dropped, the select! functions as if the Receiver is never Ready (even though when you poll it it is Ready in this case). https://github.com/rust-lang/futures-rs/issues/2455

One big problem with this that on shutdown of part of Arti, it can cause tasks to hang rather than noticing everything has gone away and terminating.

Options:

  1. Do something (what?) to try to spot when a oneshot::Receiver is used within select!, and call .fuse() on it (and store that somewhere) to get a properly working impl of FusedFuture. I don't think this is very viable unless we have a more concrete idea. (Hrm, can we make <Receiver as FusedFuture>::is_terminated a deprecated method and spot its uses by select!?)

  2. Use Tokio's oneshot. Inspection of the implementation (thanks @trinity-1686a) reveals that it doesn't depend on the Tokio executor. Downside: every crate now ends up with a dep on Tokio. This is bad because it would make it easy to use other bits of tokio, despite our intent to be runtime-agnostic. It's also bad for our code size.

  3. Use Tokio's oneshot but via a re-export it from tor-async-utils. That helps prevent the accidental use of Tokio but it doesn't help with code size.

  4. Copy Tokio's oneshot.rs into tor-async-utils.

  5. Use postage::oneshot. But it has a different (and rather less good) API.

  6. Use postage::oneshot but wrap it in a newtype. I'm not sure the implementation of the good API in terms of the less good one will be much fun or very principled.

  7. Write our own oneshot.

  8. Use FusedFuture<oneshot::Receiver<T>> everywhere and enforce this by (i) making all the constructors like oneshot::channel deprecated and (ii) providing wrappers that wrap the Receiver. (This is an extra byte (probably word) of state for each Receiver and therefore will have a perf impact, so on super hot paths we might want something else. But we don't have many oneshots on super hot paths.)

CC @gabi-250 @nickm since this'll probably end up being a widespread thing.