Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions crates/test-programs/src/bin/p2_clocks_zero_wait.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
fn main() {
let pollable = wasip2::clocks::monotonic_clock::subscribe_duration(0);
for _ in 0..20 {
if pollable.ready() {
return;
}
wasip2::clocks::monotonic_clock::subscribe_duration(0).block();
}

panic!("pollable should eventually be ready");
}
3 changes: 1 addition & 2 deletions crates/test-programs/src/bin/p2_tcp_busy_poll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ use test_programs::wasi::sockets::tcp::TcpSocket;
// prevent e.g. socket readiness from being delivered. Here we verify that such
// starvation does not happen.
fn test_tcp_busy_poll(family: IpAddressFamily, address: IpSocketAddress) {
let zero_wait = monotonic_clock::subscribe_duration(0);

let net = Network::default();

let listener = TcpSocket::new(family).unwrap();
Expand All @@ -33,6 +31,7 @@ fn test_tcp_busy_poll(family: IpAddressFamily, address: IpSocketAddress) {
let rx_ready = rx.subscribe();
let mut counter = 0;
loop {
let zero_wait = monotonic_clock::subscribe_duration(0);
if counter > 1_000_000 {
panic!("socket still not ready!");
}
Expand Down
8 changes: 5 additions & 3 deletions crates/wasi/src/p2/host/clocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ fn subscribe_to_duration(
duration: tokio::time::Duration,
) -> wasmtime::Result<Resource<DynPollable>> {
let sleep = if duration.is_zero() {
table.push(Deadline::Past)?
table.push(Deadline::Past { yielded: false })?
} else if let Some(deadline) = tokio::time::Instant::now().checked_add(duration) {
// NB: this resource created here is not actually exposed to wasm, it's
// only an internal implementation detail used to match the signature
Expand Down Expand Up @@ -115,7 +115,7 @@ impl monotonic_clock::Host for WasiClocksCtxView<'_> {
}

enum Deadline {
Past,
Past { yielded: bool },
Instant(tokio::time::Instant),
Never,
}
Expand All @@ -124,7 +124,8 @@ enum Deadline {
impl Pollable for Deadline {
async fn ready(&mut self) {
match self {
Deadline::Past => {
Deadline::Past { yielded: true } => {}
Deadline::Past { yielded } => {
// It is important we yield to Tokio here; otherwise we risk
// starving `mio` such that it is unable to signal readiness for
// other pollables (e.g. TCP sockets) when the guest is polling
Expand All @@ -142,6 +143,7 @@ impl Pollable for Deadline {
// are hypothetically other ways to generate a pollable that's
// always immediately ready, which this hack doesn't cover, but
// we consider this sufficient for now.
Copy link
Copy Markdown

@ignatz ignatz Jun 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm primarily commenting to say: "Thank you" 🙏 and secondarily in a selfish attempt to better understand what's going on, i.e. from a place of ignorance.

I understood from your text that Pollable::ready is called multiple time and I would have absolutely written the code in the same way it was, i.e. it's surprising to me that the status quote cannot make progress. Wouldn't all futures returned by yield_now get resolved eventually even if some of them get dropped?

Since I don't even have the context why Pollable::ready gets called multiple times, I'm wondering if this can lead to some re-ordering of tasks? First call will yield, second won't which means a second continuation may get scheduled earlier than the first. This is probably not an issue since tokio doesn't give any ordering guarantees but may still be surprising 🤷.

Maybe there's some way in the downstream consuming code to address this issue rather than the conditional yielding? Then again, I've no clue and you should do what makes the most sense.

From a mere user's perspective, I would expect repeated yields from WASM to lead to repeated yields to the tokio runtime ... as long as that's the case 🤷‍♀️

EDIT: specifically await Timer(0) is a common approach in many languages (JS, Dart, ...) to yield to the runtime/event-loop.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation of pollable.ready(), which is what wstd is calling, creates this Pollable::ready future defined here, polls it, then drops it. With the previous implementation it would never make progress since the yield was attempted to happen every time. It looks like wstd never actually blocked on the pollable, it only kept asking if it was ready. This patched implementation still preserves yielding behavior, but the yield is only attempted once.

*yielded = true;
tokio::task::yield_now().await
}
Deadline::Instant(instant) => tokio::time::sleep_until(*instant).await,
Expand Down
5 changes: 5 additions & 0 deletions crates/wasi/tests/all/p2/async_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,3 +435,8 @@ async fn file_truncation_readonly(component_path: &str) {
let contents = std::fs::read(&file).expect("read truncation test file");
assert_eq!(EXPECTED_CONTENTS, contents);
}

#[test_log::test(tokio::test(flavor = "multi_thread"))]
async fn p2_clocks_zero_wait() {
run(P2_CLOCKS_ZERO_WAIT_COMPONENT, |_| {}).await.unwrap()
}
5 changes: 5 additions & 0 deletions crates/wasi/tests/all/p2/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,3 +403,8 @@ fn file_truncation_readonly(component_path: &str) {
let contents = std::fs::read(&file).expect("read truncation test file");
assert_eq!(EXPECTED_CONTENTS, contents);
}

#[test_log::test]
fn p2_clocks_zero_wait() {
run(P2_CLOCKS_ZERO_WAIT_COMPONENT, |_| {}).unwrap()
}
Loading