nonblocking check of pollables when reactor is busy #78
Conversation
| let mut future_group = futures_concurrency::future::FutureGroup::< | ||
| Pin<Box<dyn std::future::Future<Output = bool>>>, | ||
| >::new(); | ||
| future_group.insert(Box::pin(cpu_heavy)); | ||
| future_group.insert(Box::pin(timeout)); | ||
| let result = futures_lite::StreamExt::next(&mut future_group).await; |
There was a problem hiding this comment.
I'm surprised this doesn't work here? Afaict this is just race-semantics?
| let mut future_group = futures_concurrency::future::FutureGroup::< | |
| Pin<Box<dyn std::future::Future<Output = bool>>>, | |
| >::new(); | |
| future_group.insert(Box::pin(cpu_heavy)); | |
| future_group.insert(Box::pin(timeout)); | |
| let result = futures_lite::StreamExt::next(&mut future_group).await; | |
| let result = (cpu_heavy, timeout).race().await; |
Assuming futures_concurrency::prelude::*; is imported of course.
There was a problem hiding this comment.
I just tried this change, and it doesn't actually trigger the broken behavior that the existing test case does. I don't actually understand why, though.
There was a problem hiding this comment.
That's really weird; they should both work very similarly. The only real diffence I can think of is perhaps some amount of randomness in the execution order.
@SilverMira do you remember why you chose to use FutureGroup rather than Future::race?
There was a problem hiding this comment.
do you remember why you chose to use
FutureGrouprather thanFuture::race?
I only used it for as an repro, because in my actual project, the issue comes in the form of a deadlock, like how @teohhanhui experienced in #73 (comment).
The difference in between FutureGroup and Future::race lies in how it polls its inner futures, if I recall correctly, Future::race polls its inner futures in a random order whenever any one of its inner futures wake. Which means the wasi future could potentially be randomly chosen to be polled before the other task (even when it's not woken), and Pollable::ready would complete the future, thus winning the future.
Meanwhile, FutureGroup and others like StreamExt will explicitly skip un-woken inner futures when it itself is polled. Since the runtime isn't doing poll::poll while the main task is busy, it never wakes the wasi pollable, hence the FutureGroup never tries to poll them, causing it to lose the race.
as provided by @SilverMira in #70, and tracked in #73 Co-authored-by: SilverMira <[email protected]>
sharing most of the implementation with block_on_pollables
in the nonblocking case, we can skip work. in the blocking case, we need to panic. In the absence of a panic, either the debug assert in block_on_pollables will go off, or the wasi poll() will trap.
Fixes #73.
Add test provided by @SilverMira in #70, tracked in #73.
Add a
nonblock_check_pollablesfunction to the reactor, for use in the "busy case" of the block_on loop. Factor outcheck_pollables, the common part ofblock_on_pollablesandnonblock_check_pollables.