Dropped timer causes infinite wakeups?

[redbaron]

I'd expect this incorrect program to never wake up, never return and underneath stream::poll_fn to be polled once.

When running with smol 0.1.5 it triggers tight infinite polling loop.

use smol::{self, Timer};
use std::task::Poll;

use std::time::Duration;

use futures::future::FutureExt;
use futures::stream::{self, StreamExt};

pub fn main() {
    smol::run(async {
        let mut sd = stream::poll_fn(|cx| {
            println!("I am being polled");
            Timer::after(Duration::from_secs(1)).poll_unpin(cx);
            Poll::<Option<()>>::Pending
        });
        sd.next().await;
    })
}

For comparison, here is Playground with tokio: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=489752594073ad372fee0a65f0aeb7ad

[ComputerDruid]

Slight minimization, it's not unique to Stream:

use smol::{self, Timer};
use std::task::Poll;

use std::time::Duration;

use futures::future::{FutureExt, self};

pub fn main() {
    smol::run(
        future::poll_fn(|cx| {
            println!("I am being polled");
            Timer::after(Duration::from_secs(1)).poll_unpin(cx);
            Poll::<Option<()>>::Pending
        })
    );
}

But I think it's inaccurate to say this triggers an infinite polling loop, just a single spurious poll. This example code creates and drops the timer every time it's polled, which is causing the tight loop.

[ComputerDruid]

I think there's 2 "bugs" this reveals in smol:

• If a new timer is added to the reactor while no one is waiting on the reactor, the next reactor wait will terminate immediately because of the timer_event notification, even though the timer op will have already been handled. This forces the executor to go through the loop again unnecessarily. I wrote Skip notifying the reactor for added timers unnecessarily #116 which I believe addresses that.

• Whenever the event loop runs, smol polls the main future, even if the waker associated with the main future has not been woken. That means that events meant for spawned tasks (or in this bug's case, meant for the management of the timer queue) will also cause the main future to be re-polled, which seems undesirable.

I believe fixing either of these things will make this "bug" disappear. That said, futures getting polled spuriously is fairly normal when multiple futures are combined together with select or join, so I'm not sure that this bug itself represents a real issue. It was fun to dig in to smol to try to figure out why this was happening, regardless.

[ComputerDruid]

Oh, and I found using a variant of the repro case that exits after 20 polls more pleasant to work with, in case that's useful:

use smol::{self, Timer};
use std::task::Poll;

use std::time::Duration;

use futures::future::{self, FutureExt};

pub fn main() {
    let mut poll_count: i32 = 0;
    smol::run(future::poll_fn(|cx| {
        poll_count += 1;
        if poll_count == 20 {
            return Poll::Ready(());
        }
        eprintln!("polled {} times", poll_count);
        Timer::after(Duration::from_secs(1)).poll_unpin(cx);
        Poll::Pending
    }));
}

[ghost]

Thank you for the deep analysis! This will be fixed after I extract the Async+Timer part of smol into a separate crate. We just need to do a bit more diligent around epoll/kqueue/wepoll and check for spurious wakeups...

[ComputerDruid]

I think this is actually fixed by 55f360b

At the very least, I can no longer reproduce it. I think the added timer_event.clear() means that the next reactor wait doesn't immediately wake up because of the new timer.

The other bug I mentioned (main future gets polled regardless of being woken) still exists, but it might be worthwhile to split that out into a separate bug, so I opened #166

[redbaron]

Great, thanks!