Skip to content

Commit

Permalink
Fix test case since rust-lang/futures-rs#2049
Browse files Browse the repository at this point in the history
  • Loading branch information
udoprog committed Jan 28, 2020
1 parent 64573d1 commit 474749f
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 8 deletions.
5 changes: 4 additions & 1 deletion README.md
Expand Up @@ -65,7 +65,9 @@ and be polled more frequently.

This process can lead to an especially unfortunate cases where a small number of
tasks can can cause the polling loop of [FuturesUnordered] to
[spin abnormally]. This issue was [reported by Jon Gjengset].
[spin abnormally].
This issue was [reported by Jon Gjengset], and improved on by [limiting the
amount FuturesUnordered is allowed to spin].

Unicycle addresses this by limiting how frequently a child task may be polled
per _polling cycle_. This is done by keeping two sets of polling interest and
Expand All @@ -75,6 +77,7 @@ wakeups are only registered in the swapped in set which will be polled the next
cycle. For more details, see the _Architecture_ section below.

[spin abnormally]: https://github.com/udoprog/unicycle/blob/master/tests/spinning_futures_unordered.rs
[limiting the amount FuturesUnordered is allowed to spin]: https://github.com/rust-lang/futures-rs/pull/2049
[reported by Jon Gjengset]: https://github.com/rust-lang/futures-rs/issues/2047

## Architecture
Expand Down
5 changes: 4 additions & 1 deletion src/lib.rs
Expand Up @@ -61,7 +61,9 @@
//!
//! This process can lead to an especially unfortunate cases where a small number of
//! tasks can can cause the polling loop of [FuturesUnordered] to
//! [spin abnormally]. This issue was [reported by Jon Gjengset].
//! [spin abnormally].
//! This issue was [reported by Jon Gjengset], and improved on by [limiting the
//! amount FuturesUnordered is allowed to spin].
//!
//! Unicycle addresses this by limiting how frequently a child task may be polled
//! per _polling cycle_. This is done by keeping two sets of polling interest and
Expand All @@ -71,6 +73,7 @@
//! cycle. For more details, see the _Architecture_ section below.
//!
//! [spin abnormally]: https://github.com/udoprog/unicycle/blob/master/tests/spinning_futures_unordered.rs
//! [limiting the amount FuturesUnordered is allowed to spin]: https://github.com/rust-lang/futures-rs/pull/2049
//! [reported by Jon Gjengset]: https://github.com/rust-lang/futures-rs/issues/2047
//!
//! ## Architecture
Expand Down
43 changes: 37 additions & 6 deletions tests/spinning_futures_unordered.rs
Expand Up @@ -3,33 +3,64 @@ use futures::{
stream::{FuturesUnordered, Stream as _},
};
use std::{
cell::Cell,
future::Future,
pin::Pin,
task::{Context, Poll},
};
use unicycle::Unordered;

struct Spinner;
struct Spinner<'a>(&'a Cell<usize>);

impl Future for Spinner {
impl Future for Spinner<'_> {
type Output = ();

fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
println!("Hello!");
self.0.set(self.0.get() + 1);

// Note: this will not be needed once we have a futures release with:
// https://github.com/rust-lang/futures-rs/pull/2049
if self.0.get() > 16 {
return Poll::Ready(());
}

cx.waker().wake_by_ref();
Poll::Pending
}
}

#[ignore]
#[tokio::test]
async fn test_spinning_futures_unordered() {
let count = Cell::new(0);

let futures = FuturesUnordered::new();
futures.push(Spinner);
futures.push(Spinner(&count));
pin_utils::pin_mut!(futures);

let _ = poll_fn::<(), _>(move |cx| {
let _ = Pin::new(&mut futures).poll_next(cx);
panic!("We never reach this...");
Poll::Ready(())
})
.await;

// Note: FuturesUnordered will spin a bit before yielding.
assert!(count.get() > 1);
}

#[tokio::test]
async fn test_spinning_unordered() {
let count = Cell::new(0);

let mut futures = Unordered::new();
futures.push(Spinner(&count));
pin_utils::pin_mut!(futures);

let _ = poll_fn::<(), _>(move |cx| {
let _ = Pin::new(&mut futures).poll_next(cx);
Poll::Ready(())
})
.await;

// Note: Unicycle guarantees each future is poll at most once.
assert_eq!(1, count.get());
}

0 comments on commit 474749f

Please sign in to comment.