Abortable streams #2410
Abortable streams #2410
Conversation
futures-util/src/abortable.rs
Outdated
| /// # use futures::stream::{self, StreamExt}; | ||
| /// | ||
| /// let (abort_handle, abort_registration) = AbortHandle::new_pair(); | ||
| /// let mut stream = Abortable::stream(stream::iter(vec![1, 2, 3]), abort_registration); |
There was a problem hiding this comment.
This function doesn't seem to exist, but I think it's not actually a bad idea to have both Abortable::new (require T: Future) and Abortable::stream (require T: Stream) because we can prevent the creation of Abortables that cannot be polled. -- However, it may feel redundant when we provide other abortables (e.g., abortable sink) in the future.
There was a problem hiding this comment.
At first I wrote Abortable::new and stream, but I thought that new for abortable futures specifically was a bit arbitrary.
taiki-e
added
0.3-backport: pending
futures-util/src/abortable.rs
Outdated
| /// Checks whether the task has been aborted. See [`AbortHandle::abort`] for details. | ||
| pub fn is_aborted(&self) -> bool { | ||
| self.inner.aborted.load(Ordering::Relaxed) | ||
| } |
There was a problem hiding this comment.
This is actually a method that returns true if it is notified that the task should be aborted (= abort was called), and I think there are some notes that are desirable to explain in the documentation.
- This will return true even if
abortis called after the task has completed. - This will return true even if the task still is running at the time.
There was a problem hiding this comment.
Alternatively, we can omit is_aborted method until someone actually requests it.
There was a problem hiding this comment.
We might as well keep it, because we can use it internally in try_poll instead of loading the flag manually. I'll update the docs.
There was a problem hiding this comment.
Updated the docs, is Ordering::Relaxed enough for the user facing API?
Co-authored-by: Taiki Endo <te316e89@gmail.com>
taiki-e
added
0.3-backport: completed
and removed
0.3-backport: pending
This PR makes
Abortableable to work with both futures and streams.Is there a better place to put
Abortablethanfutures-util/src/abortable.rs? Maybe ashared/folder?Resolves #1424