Skip to content

Commit d826d8a

Browse files
authored
Opt out of tokio's per-task budget in wasip2 noop polls (#14265)
This commit updates various calls of `noop_waker` and `poll_noop` throughout wasip2 to ensure they're all using `tokio::task::unconstrained` to specifically opt-out of Tokio's default task-budgeting mechanism. This is necessary to get some various tests I'm working on wasi-libc working on wasip2 because otherwise the test exhausts the task budget pretty quickly and then results in pretty surprising I/O behavior. One example is that a TCP socket ready for writing with `check-write` is subsequently not actually ready for writing because the task budget ran out. To the best of my ability to understand what I'm running into is the same as what's run into in #14174. I'm not entirely sure though, but I'm going to conservatively say... Closes #14174
1 parent bf33049 commit d826d8a

8 files changed

Lines changed: 111 additions & 45 deletions

File tree

crates/test-programs/src/bin/p2_tcp_streams.rs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,19 +110,62 @@ fn test_tcp_shutdown_should_not_lose_data(net: &Network, family: IpAddressFamily
110110
});
111111
}
112112

113+
// Once a stream is writable it should in theory always be writable...
114+
fn test_tcp_check_write_should_not_be_rate_limited(net: &Network, family: IpAddressFamily) {
115+
setup(net, family, |_server, client| {
116+
client.output.subscribe().block();
117+
assert!(client.output.check_write().unwrap() > 0);
118+
119+
for i in 0..1000 {
120+
let permit = client.output.check_write().unwrap();
121+
assert!(
122+
permit > 0,
123+
"check-write reported 0 bytes on an idle stream after {i} calls"
124+
);
125+
}
126+
});
127+
}
128+
129+
// Assert that repeated small writes are sent to the kernel's buffer, and the
130+
// 300 writes here should surely be much larger than any kernel buffer...
131+
fn test_tcp_nonblocking_write_loop(net: &Network, family: IpAddressFamily) {
132+
setup(net, family, |server, client| {
133+
client.output.subscribe().block();
134+
135+
const COUNT: usize = 300;
136+
for i in 0..COUNT {
137+
let permit = client.output.check_write().unwrap();
138+
assert!(
139+
permit > 0,
140+
"check-write reported 0 bytes after {i} nonblocking writes"
141+
);
142+
client.output.write(b"x").unwrap();
143+
}
144+
145+
// Everything written above must arrive at the peer.
146+
client.socket.shutdown(ShutdownType::Send).unwrap();
147+
let received = server.input.blocking_read_to_end().unwrap();
148+
assert_eq!(received.len(), COUNT);
149+
});
150+
}
151+
113152
fn main() {
114153
let net = Network::default();
115154

116155
test_tcp_input_stream_should_be_closed_by_remote_shutdown(&net, IpAddressFamily::Ipv4);
117156
test_tcp_input_stream_should_be_closed_by_local_shutdown(&net, IpAddressFamily::Ipv4);
118157
test_tcp_output_stream_should_be_closed_by_local_shutdown(&net, IpAddressFamily::Ipv4);
119158
test_tcp_shutdown_should_not_lose_data(&net, IpAddressFamily::Ipv4);
159+
test_tcp_check_write_should_not_be_rate_limited(&net, IpAddressFamily::Ipv4);
160+
test_tcp_nonblocking_write_loop(&net, IpAddressFamily::Ipv4);
120161

121162
if supports_ipv6() {
122163
test_tcp_input_stream_should_be_closed_by_remote_shutdown(&net, IpAddressFamily::Ipv6);
123164
test_tcp_input_stream_should_be_closed_by_local_shutdown(&net, IpAddressFamily::Ipv6);
124165
test_tcp_output_stream_should_be_closed_by_local_shutdown(&net, IpAddressFamily::Ipv6);
125166
test_tcp_shutdown_should_not_lose_data(&net, IpAddressFamily::Ipv6);
167+
test_tcp_check_write_should_not_be_rate_limited(&net, IpAddressFamily::Ipv6);
168+
test_tcp_nonblocking_write_loop(&net, IpAddressFamily::Ipv6);
126169
}
127170
}
128171

crates/wasi/src/p2/host/tcp.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ use crate::p2::{
66
},
77
tcp::AsyncOperation,
88
};
9-
use crate::sockets::{WasiSocketsCtxView, noop_cx};
9+
use crate::runtime::poll_now;
10+
use crate::sockets::WasiSocketsCtxView;
1011
use std::net::SocketAddr;
11-
use std::task::Poll;
1212
use wasmtime::component::Resource;
1313
use wasmtime_wasi_io::{
1414
poll::DynPollable,
@@ -80,7 +80,7 @@ impl crate::p2::host::tcp::tcp::HostTcpSocket for WasiSocketsCtxView<'_> {
8080
return Err(ErrorCode::NotInProgress.into());
8181
};
8282

83-
let Poll::Ready(result) = socket.inner.poll_finish_connect(&mut noop_cx()) else {
83+
let Some(result) = poll_now(|cx| socket.inner.poll_finish_connect(cx)) else {
8484
return Err(ErrorCode::WouldBlock.into());
8585
};
8686
socket.in_progress_operation = None;
@@ -134,9 +134,9 @@ impl crate::p2::host::tcp::tcp::HostTcpSocket for WasiSocketsCtxView<'_> {
134134
return Err(ErrorCode::InvalidState.into());
135135
};
136136

137-
let accepted = match listener.poll_accept(&mut noop_cx()) {
138-
Poll::Pending => return Err(ErrorCode::WouldBlock.into()),
139-
Poll::Ready(accepted) => accepted,
137+
let accepted = match poll_now(|cx| listener.poll_accept(cx)) {
138+
Some(accepted) => accepted,
139+
None => return Err(ErrorCode::WouldBlock.into()),
140140
};
141141
let mut tcp_socket = TcpSocket::new(accepted);
142142
let (input, output) = tcp_socket.take_streams()?;

crates/wasi/src/p2/host/udp.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ use crate::p2::bindings::sockets::network::{ErrorCode, IpAddressFamily, IpSocket
22
use crate::p2::bindings::sockets::udp;
33
use crate::p2::udp::{AsyncOperation, IncomingDatagramStream, OutgoingDatagramStream};
44
use crate::p2::{Pollable, SocketError, SocketResult, UdpSocket};
5+
use crate::runtime::poll_now;
56
use crate::sockets::{SocketAddressFamily, WasiSocketsCtxView};
67
use async_trait::async_trait;
78
use std::future::poll_fn;
89
use std::net::SocketAddr;
9-
use std::task::{Context, Poll, Waker};
1010
use wasmtime::component::Resource;
1111
use wasmtime::format_err;
1212
use wasmtime_wasi_io::poll::DynPollable;
@@ -238,9 +238,7 @@ impl udp::HostOutgoingDatagramStream for WasiSocketsCtxView<'_> {
238238
fn check_send(&mut self, this: Resource<udp::OutgoingDatagramStream>) -> SocketResult<u64> {
239239
let stream = self.table.get_mut(&this)?;
240240

241-
let count = if let Poll::Ready(()) =
242-
stream.poll_send_ready(&mut Context::from_waker(Waker::noop()))
243-
{
241+
let count = if let Some(()) = poll_now(|cx| stream.poll_send_ready(cx)) {
244242
// We don't know how many Tokio will accept, so we make up a
245243
// reasonable number here. If we're wrong and `send` returns
246244
// `Ok(0)`, the guest will just have to deal with that, e.g. by

crates/wasi/src/p2/ip_name_lookup.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
use crate::p2::SocketError;
22
use crate::p2::bindings::sockets::ip_name_lookup::{Host, HostResolveAddressStream};
33
use crate::p2::bindings::sockets::network::{ErrorCode, IpAddress, Network};
4+
use crate::runtime::poll_now;
45
use crate::sockets::ip_name_lookup::resolve_addresses;
5-
use crate::sockets::{MaybeReady, WasiSocketsCtxView, noop_cx};
6+
use crate::sockets::{MaybeReady, WasiSocketsCtxView};
67
use std::net::IpAddr;
7-
use std::task::Poll;
88
use std::vec;
99
use wasmtime::Result;
1010
use wasmtime::component::Resource;
@@ -42,7 +42,7 @@ impl HostResolveAddressStream for WasiSocketsCtxView<'_> {
4242
resource: Resource<ResolveAddressStream>,
4343
) -> Result<Option<IpAddress>, SocketError> {
4444
let stream: &mut ResolveAddressStream = self.table.get_mut(&resource)?;
45-
let Poll::Ready(result) = stream.0.poll_ready(&mut noop_cx()) else {
45+
let Some(result) = poll_now(|cx| stream.0.poll_ready(cx)) else {
4646
return Err(ErrorCode::WouldBlock.into());
4747
};
4848

crates/wasi/src/p2/tcp.rs

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@ use crate::p2::bindings::sockets::network::ErrorCode;
33
use crate::p2::{
44
DynInputStream, DynOutputStream, InputStream, OutputStream, Pollable, SocketResult, StreamError,
55
};
6+
use crate::runtime::poll_now;
67
use crate::sockets::{
7-
MaybeReady, TcpListenStream, TcpReceiveStream, TcpSendStream, TcpSocket as P3Socket, noop_cx,
8+
MaybeReady, TcpListenStream, TcpReceiveStream, TcpSendStream, TcpSocket as P3Socket,
89
};
910
use std::future::poll_fn;
1011
use std::mem;
@@ -79,14 +80,14 @@ impl ReadState {
7980
return Ok(bytes::Bytes::new());
8081
}
8182
let mut buf = bytes::BytesMut::zeroed(size.min(crate::MAX_READ_SIZE_ALLOC));
82-
let n = match stream.poll_read(&mut noop_cx(), &mut buf) {
83-
Poll::Pending => 0,
84-
Poll::Ready(Ok(0)) => {
83+
let n = match poll_now(|cx| stream.poll_read(cx, &mut buf)) {
84+
None => 0,
85+
Some(Ok(0)) => {
8586
*self = ReadState::Closed;
8687
return Err(StreamError::Closed);
8788
}
88-
Poll::Ready(Ok(n)) => n,
89-
Poll::Ready(Err(e)) => {
89+
Some(Ok(n)) => n,
90+
Some(Err(e)) => {
9091
*self = ReadState::Closed;
9192
return Err(StreamError::LastOperationFailed(e.into()));
9293
}
@@ -158,13 +159,13 @@ impl WriteState {
158159
}
159160

160161
fn check_write(&mut self) -> StreamResult<usize> {
161-
match self.poll_ready(&mut noop_cx()) {
162-
Poll::Pending => Ok(0),
163-
Poll::Ready(Ok((_, permit))) => {
162+
match poll_now(|cx| self.poll_ready(cx)) {
163+
None => Ok(0),
164+
Some(Ok((_, permit))) => {
164165
*permit = MAX_READ_SIZE_ALLOC;
165166
Ok(*permit)
166167
}
167-
Poll::Ready(Err(e)) => Err(e),
168+
Some(Err(e)) => Err(e),
168169
}
169170
}
170171

@@ -210,9 +211,9 @@ impl WriteState {
210211
}));
211212

212213
// Attempt to finish the write, surfacing potential errors immediately:
213-
match self.poll_ready(&mut noop_cx()) {
214-
Poll::Pending | Poll::Ready(Ok(_)) => Ok(()),
215-
Poll::Ready(Err(e)) => Err(e),
214+
match poll_now(|cx| self.poll_ready(cx)) {
215+
None | Some(Ok(_)) => Ok(()),
216+
Some(Err(e)) => Err(e),
216217
}
217218
}
218219

crates/wasi/src/p2/udp.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ use futures::TryFutureExt;
22

33
use crate::{
44
p2::bindings::sockets::network::ErrorCode,
5-
sockets::{MaybeReady, UdpSocket as P3Socket, noop_cx},
5+
runtime::poll_now,
6+
sockets::{MaybeReady, UdpSocket as P3Socket},
67
};
78
use std::{
89
net::SocketAddr,
@@ -89,8 +90,7 @@ impl IncomingDatagramStream {
8990
}
9091

9192
pub(crate) fn try_recv(&mut self) -> Result<(Vec<u8>, SocketAddr), ErrorCode> {
92-
let noop_cx = &mut noop_cx();
93-
if self.poll_recv_ready(noop_cx).is_pending() {
93+
if poll_now(|cx| self.poll_recv_ready(cx)).is_none() {
9494
return Err(ErrorCode::WouldBlock);
9595
}
9696

@@ -128,7 +128,7 @@ impl OutgoingDatagramStream {
128128
addr: Option<std::net::SocketAddr>,
129129
) -> Result<(), ErrorCode> {
130130
if let Some(send) = &mut self.prev_send {
131-
if !send.poll_ready(&mut noop_cx()).is_ready() {
131+
if poll_now(|cx| send.poll_ready(cx)).is_none() {
132132
return Err(ErrorCode::WouldBlock);
133133
}
134134

@@ -147,7 +147,7 @@ impl OutgoingDatagramStream {
147147
.send(data, addr)
148148
.map_err(|e| e.into()),
149149
);
150-
if send.poll_ready(&mut noop_cx()).is_ready() {
150+
if poll_now(|cx| send.poll_ready(cx)).is_some() {
151151
send.unwrap_ready()
152152
} else {
153153
self.prev_send = Some(send);

crates/wasi/src/runtime.rs

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919
//! Each of these facilities should be used by dependencies of wasmtime-wasi
2020
//! which when implementing component bindings.
2121
22-
use std::future::Future;
23-
use std::pin::Pin;
22+
use std::future::{Future, poll_fn};
23+
use std::pin::{Pin, pin};
2424
use std::sync::LazyLock;
2525
use std::task::{Context, Poll, Waker};
2626

@@ -170,6 +170,37 @@ pub fn with_ambient_tokio_runtime<R>(f: impl FnOnce() -> R) -> R {
170170
}
171171
}
172172

173+
/// Invokes `f` with a noop waker, intended to be used in a `poll*`-style
174+
/// function, and then returns whether the operation was ready or not.
175+
///
176+
/// This uses a "noop waker" to perform the poll and additionally handles
177+
/// details such as opting-out of Tokio's cooperative task budget.
178+
pub(crate) fn poll_now<T>(f: impl FnOnce(&mut Context<'_>) -> Poll<T>) -> Option<T> {
179+
let mut f = Some(f);
180+
// Note that this specifically opts-out of tokio's task budget. This is the
181+
// implementation primitive for a number of wasip2 operations where the
182+
// "true blocking operation", `poll`, happens at some future time w.r.t. an
183+
// in-guest event loop. If tokio's task budget is enabled then the guest
184+
// will quickly run out of budget and this function (which bottoms out in
185+
// tokio primitives within `f`) will return `Pending`. This can pessimize
186+
// guest loops that expect to be able to perform more work
187+
// per-turn-of-the-event-loop and otherwise provide surprising behavior
188+
// because the "everything is pending now" cliff can happen at seemingly
189+
// arbitrary points.
190+
let fut = pin!(tokio::task::unconstrained(poll_fn(move |cx| {
191+
match f.take() {
192+
Some(f) => f(cx),
193+
None => Poll::Pending,
194+
}
195+
})));
196+
197+
let mut context = Context::from_waker(Waker::noop());
198+
match fut.poll(&mut context) {
199+
Poll::Ready(result) => Some(result),
200+
Poll::Pending => None,
201+
}
202+
}
203+
173204
/// Attempts to get the result of a `future`.
174205
///
175206
/// This function does not block and will poll the provided future once. If the
@@ -181,9 +212,5 @@ pub fn poll_noop<F>(future: Pin<&mut F>) -> Option<F::Output>
181212
where
182213
F: Future,
183214
{
184-
let mut task = Context::from_waker(Waker::noop());
185-
match future.poll(&mut task) {
186-
Poll::Ready(result) => Some(result),
187-
Poll::Pending => None,
188-
}
215+
poll_now(|cx| future.poll(cx))
189216
}

crates/wasi/src/sockets/mod.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::runtime::poll_noop;
12
use core::fmt;
23
use core::future::Future;
34
use core::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr};
@@ -247,9 +248,9 @@ impl<T> MaybeReady<T> {
247248
T: Send + 'static,
248249
{
249250
let mut fut = Box::pin(fut);
250-
match crate::runtime::with_ambient_tokio_runtime(|| fut.as_mut().poll(&mut noop_cx())) {
251-
Poll::Ready(val) => Self::Ready(val),
252-
Poll::Pending => Self::new(crate::runtime::spawn(fut)),
251+
match crate::runtime::with_ambient_tokio_runtime(|| poll_noop(fut.as_mut())) {
252+
Some(val) => Self::Ready(val),
253+
None => Self::new(crate::runtime::spawn(fut)),
253254
}
254255
}
255256
pub(crate) fn unwrap_ready(self) -> T {
@@ -281,10 +282,6 @@ impl<T> MaybeReady<T> {
281282
}
282283
}
283284

284-
pub(crate) fn noop_cx() -> std::task::Context<'static> {
285-
std::task::Context::from_waker(futures::task::noop_waker_ref())
286-
}
287-
288285
#[derive(Clone, Copy, Debug)]
289286
pub enum ErrorCode {
290287
AccessDenied,

0 commit comments

Comments
 (0)