Skip to content

Commit d60f178

Browse files
dicejdicej
authored
return ERRNO_AGAIN from fd_{read,write} when appropriate (#13111)
* return `ERRNO_AGAIN` from `fd_{read,write}` when appropriate The WASIp1 `fd_read` and `fd_write` functions should return `Err(ERRNO_AGAIN)` rather Ok(0) if a non-blocking operation fails to transfer any bytes. [The docs](https://github.com/WebAssembly/WASI/blob/wasi-0.1/preview1/docs.md) aren't particularly clear about this other than saying these functions are "similar to [readv,writev] in POSIX", but if any functions ought to ever return `ERRNO_AGAIN`, it's these. I'm guessing this hasn't been noticed (or at least not reported) until now because most guests use blocking I/O for files, but Go uses non-blocking I/O to support concurrent I/O across multiple goroutines, and it only knows to call `poll_oneoff` (rather than e.g. assume EOF) if it gets an `EAGAIN`. * make `BlockingMode::read` mirror `BlockingMode::write` * update p1 file I/O tests to cover blocking and non-blocking cases * fix `wasi-common`'s `path_open` with `APPEND | NONBLOCK`
1 parent 6f7399d commit d60f178

File tree

13 files changed

+408
-205
lines changed

13 files changed

+408
-205
lines changed

‎crates/test-programs/src/bin/p1_cli_hostcall_fuel.rs‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
use std::ptr;
2+
use test_programs::preview1::BlockingMode;
23

34
fn main() {
45
big_poll();
56
big_string();
6-
big_iovecs();
7+
big_iovecs(BlockingMode::Blocking);
8+
big_iovecs(BlockingMode::NonBlocking);
79
}
810

911
fn big_string() {
@@ -18,7 +20,7 @@ fn big_string() {
1820
);
1921
}
2022

21-
fn big_iovecs() {
23+
fn big_iovecs(blocking_mode: BlockingMode) {
2224
let mut iovs = Vec::new();
2325
let mut ciovs = Vec::new();
2426
for _ in 0..10_000 {
@@ -40,14 +42,14 @@ fn big_iovecs() {
4042
wasip1::OFLAGS_CREAT,
4143
wasip1::RIGHTS_FD_WRITE | wasip1::RIGHTS_FD_READ,
4244
0,
43-
0,
45+
blocking_mode.fd_flags(),
4446
)
4547
.unwrap()
4648
};
4749

4850
unsafe {
49-
assert_eq!(wasip1::fd_write(fd, &ciovs), Err(wasip1::ERRNO_NOMEM));
50-
assert_eq!(wasip1::fd_read(fd, &iovs), Err(wasip1::ERRNO_NOMEM));
51+
assert_eq!(blocking_mode.write(fd, &ciovs), Err(wasip1::ERRNO_NOMEM));
52+
assert_eq!(blocking_mode.read(fd, &iovs), Err(wasip1::ERRNO_NOMEM));
5153
assert_eq!(wasip1::fd_pwrite(fd, &ciovs, 0), Err(wasip1::ERRNO_NOMEM));
5254
assert_eq!(wasip1::fd_pread(fd, &iovs, 0), Err(wasip1::ERRNO_NOMEM));
5355
}
@@ -63,8 +65,8 @@ fn big_iovecs() {
6365
buf_len: 10_000,
6466
});
6567
unsafe {
66-
assert_eq!(wasip1::fd_write(fd, &ciovs), Err(wasip1::ERRNO_NOMEM));
67-
assert_eq!(wasip1::fd_read(fd, &iovs), Err(wasip1::ERRNO_NOMEM));
68+
assert_eq!(blocking_mode.write(fd, &ciovs), Err(wasip1::ERRNO_NOMEM));
69+
assert_eq!(blocking_mode.read(fd, &iovs), Err(wasip1::ERRNO_NOMEM));
6870
assert_eq!(wasip1::fd_pwrite(fd, &ciovs, 0), Err(wasip1::ERRNO_NOMEM));
6971
assert_eq!(wasip1::fd_pread(fd, &iovs, 0), Err(wasip1::ERRNO_NOMEM));
7072
}

‎crates/test-programs/src/bin/p1_fd_flags_set.rs‎

Lines changed: 60 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
#![expect(unsafe_op_in_unsafe_fn, reason = "old code, not worth updating yet")]
22

33
use std::{env, process};
4-
use test_programs::preview1::open_scratch_directory;
4+
use test_programs::preview1::{BlockingMode, open_scratch_directory};
55

6-
unsafe fn test_fd_fdstat_set_flags(dir_fd: wasip1::Fd) {
6+
unsafe fn test_fd_fdstat_set_flags(dir_fd: wasip1::Fd, blocking_mode: BlockingMode) {
77
const FILE_NAME: &str = "file";
88
let data = &[0u8; 100];
99

@@ -14,20 +14,21 @@ unsafe fn test_fd_fdstat_set_flags(dir_fd: wasip1::Fd) {
1414
wasip1::OFLAGS_CREAT,
1515
wasip1::RIGHTS_FD_READ | wasip1::RIGHTS_FD_WRITE,
1616
0,
17-
wasip1::FDFLAGS_APPEND,
17+
blocking_mode.fd_flags() | wasip1::FDFLAGS_APPEND,
1818
)
1919
.expect("opening a file");
2020

2121
// Write some data and then verify the written data
2222
assert_eq!(
23-
wasip1::fd_write(
24-
file_fd,
25-
&[wasip1::Ciovec {
26-
buf: data.as_ptr(),
27-
buf_len: data.len(),
28-
}],
29-
)
30-
.expect("writing to a file"),
23+
blocking_mode
24+
.write(
25+
file_fd,
26+
&[wasip1::Ciovec {
27+
buf: data.as_ptr(),
28+
buf_len: data.len(),
29+
}],
30+
)
31+
.expect("writing to a file"),
3132
data.len(),
3233
"should write {} bytes",
3334
data.len(),
@@ -38,14 +39,15 @@ unsafe fn test_fd_fdstat_set_flags(dir_fd: wasip1::Fd) {
3839
let buffer = &mut [0u8; 100];
3940

4041
assert_eq!(
41-
wasip1::fd_read(
42-
file_fd,
43-
&[wasip1::Iovec {
44-
buf: buffer.as_mut_ptr(),
45-
buf_len: buffer.len(),
46-
}]
47-
)
48-
.expect("reading file"),
42+
blocking_mode
43+
.read(
44+
file_fd,
45+
&[wasip1::Iovec {
46+
buf: buffer.as_mut_ptr(),
47+
buf_len: buffer.len(),
48+
}]
49+
)
50+
.expect("reading file"),
4951
buffer.len(),
5052
"should read {} bytes",
5153
buffer.len()
@@ -59,14 +61,15 @@ unsafe fn test_fd_fdstat_set_flags(dir_fd: wasip1::Fd) {
5961
wasip1::fd_seek(file_fd, 0, wasip1::WHENCE_SET).expect("seeking file");
6062

6163
assert_eq!(
62-
wasip1::fd_write(
63-
file_fd,
64-
&[wasip1::Ciovec {
65-
buf: data.as_ptr(),
66-
buf_len: data.len(),
67-
}],
68-
)
69-
.expect("writing to a file"),
64+
blocking_mode
65+
.write(
66+
file_fd,
67+
&[wasip1::Ciovec {
68+
buf: data.as_ptr(),
69+
buf_len: data.len(),
70+
}],
71+
)
72+
.expect("writing to a file"),
7073
data.len(),
7174
"should write {} bytes",
7275
data.len(),
@@ -75,37 +78,39 @@ unsafe fn test_fd_fdstat_set_flags(dir_fd: wasip1::Fd) {
7578
wasip1::fd_seek(file_fd, 100, wasip1::WHENCE_SET).expect("seeking file");
7679

7780
assert_eq!(
78-
wasip1::fd_read(
79-
file_fd,
80-
&[wasip1::Iovec {
81-
buf: buffer.as_mut_ptr(),
82-
buf_len: buffer.len(),
83-
}]
84-
)
85-
.expect("reading file"),
81+
blocking_mode
82+
.read(
83+
file_fd,
84+
&[wasip1::Iovec {
85+
buf: buffer.as_mut_ptr(),
86+
buf_len: buffer.len(),
87+
}]
88+
)
89+
.expect("reading file"),
8690
buffer.len(),
8791
"should read {} bytes",
8892
buffer.len()
8993
);
9094

9195
assert_eq!(&data[..], &buffer[..]);
9296

93-
wasip1::fd_fdstat_set_flags(file_fd, 0).expect("disabling flags");
97+
wasip1::fd_fdstat_set_flags(file_fd, blocking_mode.fd_flags()).expect("disabling flags");
9498

9599
// Overwrite some existing data to ensure the append mode is now off
96100
wasip1::fd_seek(file_fd, 0, wasip1::WHENCE_SET).expect("seeking file");
97101

98102
let data = &[2u8; 100];
99103

100104
assert_eq!(
101-
wasip1::fd_write(
102-
file_fd,
103-
&[wasip1::Ciovec {
104-
buf: data.as_ptr(),
105-
buf_len: data.len(),
106-
}],
107-
)
108-
.expect("writing to a file"),
105+
blocking_mode
106+
.write(
107+
file_fd,
108+
&[wasip1::Ciovec {
109+
buf: data.as_ptr(),
110+
buf_len: data.len(),
111+
}],
112+
)
113+
.expect("writing to a file"),
109114
data.len(),
110115
"should write {} bytes",
111116
data.len(),
@@ -114,14 +119,15 @@ unsafe fn test_fd_fdstat_set_flags(dir_fd: wasip1::Fd) {
114119
wasip1::fd_seek(file_fd, 0, wasip1::WHENCE_SET).expect("seeking file");
115120

116121
assert_eq!(
117-
wasip1::fd_read(
118-
file_fd,
119-
&[wasip1::Iovec {
120-
buf: buffer.as_mut_ptr(),
121-
buf_len: buffer.len(),
122-
}]
123-
)
124-
.expect("reading file"),
122+
blocking_mode
123+
.read(
124+
file_fd,
125+
&[wasip1::Iovec {
126+
buf: buffer.as_mut_ptr(),
127+
buf_len: buffer.len(),
128+
}]
129+
)
130+
.expect("reading file"),
125131
buffer.len(),
126132
"should read {} bytes",
127133
buffer.len()
@@ -157,6 +163,7 @@ fn main() {
157163
};
158164

159165
unsafe {
160-
test_fd_fdstat_set_flags(dir_fd);
166+
test_fd_fdstat_set_flags(dir_fd, BlockingMode::Blocking);
167+
test_fd_fdstat_set_flags(dir_fd, BlockingMode::NonBlocking);
161168
}
162169
}

‎crates/test-programs/src/bin/p1_file_read_write.rs‎

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
#![expect(unsafe_op_in_unsafe_fn, reason = "old code, not worth updating yet")]
22

33
use std::{env, process};
4-
use test_programs::preview1::open_scratch_directory;
4+
use test_programs::preview1::{BlockingMode, open_scratch_directory};
55

6-
unsafe fn test_file_read_write(dir_fd: wasip1::Fd) {
6+
unsafe fn test_file_read_write(dir_fd: wasip1::Fd, blocking_mode: BlockingMode) {
77
// Create a file in the scratch directory.
88
let file_fd = wasip1::path_open(
99
dir_fd,
@@ -12,7 +12,7 @@ unsafe fn test_file_read_write(dir_fd: wasip1::Fd) {
1212
wasip1::OFLAGS_CREAT,
1313
wasip1::RIGHTS_FD_READ | wasip1::RIGHTS_FD_WRITE,
1414
0,
15-
0,
15+
blocking_mode.fd_flags(),
1616
)
1717
.expect("opening a file");
1818
assert!(
@@ -25,7 +25,9 @@ unsafe fn test_file_read_write(dir_fd: wasip1::Fd) {
2525
buf: contents.as_ptr() as *const _,
2626
buf_len: contents.len(),
2727
};
28-
let mut nwritten = wasip1::fd_write(file_fd, &[ciovec]).expect("writing bytes at offset 0");
28+
let mut nwritten = blocking_mode
29+
.write(file_fd, &[ciovec])
30+
.expect("writing bytes at offset 0");
2931
assert_eq!(nwritten, 4, "nwritten bytes check");
3032

3133
let contents = &mut [0u8; 4];
@@ -34,7 +36,9 @@ unsafe fn test_file_read_write(dir_fd: wasip1::Fd) {
3436
buf_len: contents.len(),
3537
};
3638
wasip1::fd_seek(file_fd, 0, wasip1::WHENCE_SET).expect("seeking to offset 0");
37-
let mut nread = wasip1::fd_read(file_fd, &[iovec]).expect("reading bytes at offset 0");
39+
let mut nread = blocking_mode
40+
.read(file_fd, &[iovec])
41+
.expect("reading bytes at offset 0");
3842
assert_eq!(nread, 4, "nread bytes check");
3943
assert_eq!(contents, &[0u8, 1, 2, 3], "written bytes equal read bytes");
4044

@@ -61,8 +65,9 @@ unsafe fn test_file_read_write(dir_fd: wasip1::Fd) {
6165
buf_len: remaining,
6266
});
6367

64-
nwritten =
65-
wasip1::fd_write(file_fd, ciovecs.as_slice()).expect("writing bytes at offset 0");
68+
nwritten = blocking_mode
69+
.write(file_fd, ciovecs.as_slice())
70+
.expect("writing bytes at offset 0");
6671

6772
offset += nwritten;
6873
if offset == contents.len() {
@@ -91,7 +96,9 @@ unsafe fn test_file_read_write(dir_fd: wasip1::Fd) {
9196
buf_len: 2,
9297
},
9398
];
94-
nread = wasip1::fd_read(file_fd, iovecs).expect("reading bytes at offset 0");
99+
nread = blocking_mode
100+
.read(file_fd, iovecs)
101+
.expect("reading bytes at offset 0");
95102
if nread == 0 {
96103
break;
97104
}
@@ -107,7 +114,9 @@ unsafe fn test_file_read_write(dir_fd: wasip1::Fd) {
107114
buf_len: contents.len(),
108115
};
109116
wasip1::fd_seek(file_fd, 2, wasip1::WHENCE_SET).expect("seeking to offset 2");
110-
nread = wasip1::fd_read(file_fd, &[iovec]).expect("reading bytes at offset 2");
117+
nread = blocking_mode
118+
.read(file_fd, &[iovec])
119+
.expect("reading bytes at offset 2");
111120
assert_eq!(nread, 2, "nread bytes check");
112121
assert_eq!(contents, &[2u8, 3, 0, 0], "file cursor was overwritten");
113122

@@ -117,7 +126,9 @@ unsafe fn test_file_read_write(dir_fd: wasip1::Fd) {
117126
buf_len: contents.len(),
118127
};
119128
wasip1::fd_seek(file_fd, 2, wasip1::WHENCE_SET).expect("seeking to offset 2");
120-
nwritten = wasip1::fd_write(file_fd, &[ciovec]).expect("writing bytes at offset 2");
129+
nwritten = blocking_mode
130+
.write(file_fd, &[ciovec])
131+
.expect("writing bytes at offset 2");
121132
assert_eq!(nwritten, 2, "nwritten bytes check");
122133

123134
let contents = &mut [0u8; 4];
@@ -126,15 +137,17 @@ unsafe fn test_file_read_write(dir_fd: wasip1::Fd) {
126137
buf_len: contents.len(),
127138
};
128139
wasip1::fd_seek(file_fd, 0, wasip1::WHENCE_SET).expect("seeking to offset 0");
129-
nread = wasip1::fd_read(file_fd, &[iovec]).expect("reading bytes at offset 0");
140+
nread = blocking_mode
141+
.read(file_fd, &[iovec])
142+
.expect("reading bytes at offset 0");
130143
assert_eq!(nread, 4, "nread bytes check");
131144
assert_eq!(contents, &[0u8, 1, 1, 0], "file cursor was overwritten");
132145

133146
wasip1::fd_close(file_fd).expect("closing a file");
134147
wasip1::path_unlink_file(dir_fd, "file").expect("removing a file");
135148
}
136149

137-
unsafe fn test_file_write_and_file_pos(dir_fd: wasip1::Fd) {
150+
unsafe fn test_file_write_and_file_pos(dir_fd: wasip1::Fd, blocking_mode: BlockingMode) {
138151
let path = "file2";
139152
let file_fd = wasip1::path_open(
140153
dir_fd,
@@ -160,7 +173,9 @@ unsafe fn test_file_write_and_file_pos(dir_fd: wasip1::Fd) {
160173
buf_len: 0,
161174
};
162175
wasip1::fd_seek(file_fd, 2, wasip1::WHENCE_SET).expect("seeking to offset 2");
163-
let n = wasip1::fd_write(file_fd, &[ciovec]).expect("writing bytes at offset 2");
176+
let n = blocking_mode
177+
.write(file_fd, &[ciovec])
178+
.expect("writing bytes at offset 2");
164179
assert_eq!(n, 0);
165180

166181
assert_eq!(wasip1::fd_tell(file_fd).unwrap(), 2);
@@ -174,7 +189,9 @@ unsafe fn test_file_write_and_file_pos(dir_fd: wasip1::Fd) {
174189
buf_len: buf.len(),
175190
};
176191
wasip1::fd_seek(file_fd, 50, wasip1::WHENCE_SET).expect("seeking to offset 50");
177-
let n = wasip1::fd_write(file_fd, &[ciovec]).expect("writing bytes at offset 50");
192+
let n = blocking_mode
193+
.write(file_fd, &[ciovec])
194+
.expect("writing bytes at offset 50");
178195
assert_eq!(n, 1);
179196

180197
assert_eq!(wasip1::fd_tell(file_fd).unwrap(), 51);
@@ -206,7 +223,9 @@ fn main() {
206223

207224
// Run the tests.
208225
unsafe {
209-
test_file_read_write(dir_fd);
210-
test_file_write_and_file_pos(dir_fd);
226+
test_file_read_write(dir_fd, BlockingMode::Blocking);
227+
test_file_read_write(dir_fd, BlockingMode::NonBlocking);
228+
test_file_write_and_file_pos(dir_fd, BlockingMode::Blocking);
229+
test_file_write_and_file_pos(dir_fd, BlockingMode::NonBlocking);
211230
}
212231
}

0 commit comments

Comments
 (0)