Skip to content

Commit

Permalink
FuturesUnordered: fix partial iteration (#2574)
Browse files Browse the repository at this point in the history
The IntoIter impl advances the head pointer every iteration, but this breaks the linked list invariant that the head's prev should be null.

If the iteration is not done to completion, on subsequent drop, FuturesUnordered::unlink relies on this broken invariant and ends up panicking.

The fix is to maintain the `head->prev == null` invariant while iterating. Also added a test for this bug.
  • Loading branch information
wfraser authored Feb 23, 2022
1 parent 9ec7707 commit 391d89e
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 0 deletions.
4 changes: 4 additions & 0 deletions futures-util/src/stream/futures_unordered/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use super::task::Task;
use super::FuturesUnordered;
use core::marker::PhantomData;
use core::pin::Pin;
use core::ptr;
use core::sync::atomic::Ordering::Relaxed;

/// Mutable iterator over all futures in the unordered set.
Expand Down Expand Up @@ -58,6 +59,9 @@ impl<Fut: Unpin> Iterator for IntoIter<Fut> {
// valid `next_all` checks can be skipped.
let next = (**task).next_all.load(Relaxed);
*task = next;
if !task.is_null() {
*(**task).prev_all.get() = ptr::null_mut();
}
self.len -= 1;
Some(future)
}
Expand Down
14 changes: 14 additions & 0 deletions futures/tests/stream_futures_unordered.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,20 @@ fn into_iter_len() {
assert!(into_iter.next().is_none());
}

#[test]
fn into_iter_partial() {
let stream = vec![future::ready(1), future::ready(2), future::ready(3), future::ready(4)]
.into_iter()
.collect::<FuturesUnordered<_>>();

let mut into_iter = stream.into_iter();
assert!(into_iter.next().is_some());
assert!(into_iter.next().is_some());
assert!(into_iter.next().is_some());
assert_eq!(into_iter.len(), 1);
// don't panic when iterator is dropped before completing
}

#[test]
fn futures_not_moved_after_poll() {
// Future that will be ready after being polled twice,
Expand Down

0 comments on commit 391d89e

Please sign in to comment.