Skip to content

Commit 080a712

Browse files
authored
wasip2: Close handles in stream-error (WebAssembly#675)
If a stream-related operation fails and it's not "closed" then there's an `own` resource within the error. Previously this leaked in wasi-libc but now it's deallocated on-the-spot. Error handling is also deduplicated across a few locations.
1 parent 9e495da commit 080a712

File tree

4 files changed

+45
-36
lines changed

4 files changed

+45
-36
lines changed

libc-bottom-half/cloudlibc/src/libc/unistd/read.c

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,8 @@ ssize_t read(int fildes, void *buf, size_t nbyte) {
3232
nbyte,
3333
&contents,
3434
&stream_error);
35-
36-
if (!ok) {
37-
if (stream_error.tag == STREAMS_STREAM_ERROR_CLOSED)
38-
return 0;
39-
else {
40-
errno = EIO;
41-
return -1;
42-
}
43-
}
35+
if (!ok)
36+
return wasip2_handle_read_error(stream_error);
4437

4538
// Copy the bytes allocated in the canonical ABI to `buf`
4639
memcpy(buf, contents.ptr, contents.len);

libc-bottom-half/cloudlibc/src/libc/unistd/write.c

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,8 @@ ssize_t write(int fildes, const void *buf, size_t nbyte) {
3434
ok = streams_method_output_stream_check_write(output_stream,
3535
&num_bytes_permitted,
3636
&stream_error);
37-
if (!ok) {
38-
errno = EIO;
39-
return -1;
40-
}
37+
if (!ok)
38+
return wasip2_handle_write_error(stream_error);
4139

4240
if (num_bytes_permitted == 0)
4341
poll_method_pollable_block(pollable);
@@ -52,17 +50,14 @@ ssize_t write(int fildes, const void *buf, size_t nbyte) {
5250
ok = streams_method_output_stream_write(output_stream,
5351
&contents,
5452
&stream_error);
55-
if (!ok) {
56-
errno = EIO;
57-
return -1;
58-
}
53+
if (!ok)
54+
return wasip2_handle_write_error(stream_error);
5955

6056
ok = streams_method_output_stream_blocking_flush(output_stream,
6157
&stream_error);
62-
if (!ok) {
63-
errno = EIO;
64-
return -1;
65-
}
58+
if (!ok)
59+
return wasip2_handle_write_error(stream_error);
60+
6661

6762
if (off)
6863
*off += contents.len;

libc-bottom-half/headers/private/wasi/file_utils.h

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,40 @@
22
#define __wasi_file_utils_h
33

44
#ifdef __wasilibc_use_wasip2
5+
#include <assert.h>
6+
#include <wasi/wasip2.h>
57
#include <wasi/descriptor_table.h>
68
#include <dirent.h>
79
#include <fcntl.h>
810
#include <errno.h>
911

12+
/// Handles a `wasi:io/streams.stream-error` for a `read`-style operation.
13+
///
14+
/// If the error indicates "closed" then 0 is returned to mean EOF. Otherwise
15+
/// the last-operation-failed handle is closed and errno is set to EIO.
16+
static int wasip2_handle_read_error(streams_stream_error_t error) {
17+
if (error.tag == STREAMS_STREAM_ERROR_CLOSED) {
18+
return 0;
19+
}
20+
assert(error.tag == STREAMS_STREAM_ERROR_LAST_OPERATION_FAILED);
21+
io_error_error_drop_own(error.val.last_operation_failed);
22+
errno = EIO;
23+
return -1;
24+
}
25+
26+
/// Same as `wasip2_handle_read_error` except "closed" now returns EPIPE
27+
/// instead of 0.
28+
static int wasip2_handle_write_error(streams_stream_error_t error) {
29+
if (error.tag == STREAMS_STREAM_ERROR_CLOSED) {
30+
errno = EPIPE;
31+
return -1;
32+
}
33+
assert(error.tag == STREAMS_STREAM_ERROR_LAST_OPERATION_FAILED);
34+
io_error_error_drop_own(error.val.last_operation_failed);
35+
errno = EIO;
36+
return -1;
37+
}
38+
1039
// Converts the C string `s` into a WASI string stored in `out`.
1140
//
1241
// The returned `wasip2_string_t` should not be deallocated or free'd, and it

libc-bottom-half/sources/wasip2_tcp.c

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <wasi/sockets_utils.h>
99
#include <wasi/tcp.h>
1010
#include <wasi/wasip2.h>
11+
#include <wasi/file_utils.h>
1112

1213
const uint64_t NS_PER_S = 1000000000;
1314

@@ -535,15 +536,9 @@ static ssize_t tcp_recvfrom(void *data, void *buffer,
535536
while (true) {
536537
wasip2_list_u8_t result;
537538
streams_stream_error_t error;
538-
if (!streams_method_input_stream_read(rx_borrow, length, &result, &error)) {
539-
if (error.tag == STREAMS_STREAM_ERROR_CLOSED) {
540-
return 0;
541-
} else {
542-
// TODO wasi-sockets: wasi-sockets has no way to recover TCP stream errors yet.
543-
errno = EPIPE;
544-
return -1;
545-
}
546-
}
539+
if (!streams_method_input_stream_read(rx_borrow, length, &result, &error))
540+
// TODO wasi-sockets: wasi-sockets has no way to recover TCP stream errors yet.
541+
return wasip2_handle_read_error(error);
547542

548543
if (result.len) {
549544
memcpy(buffer, result.ptr, result.len);
@@ -624,19 +619,16 @@ static ssize_t tcp_sendto(void *data, const void *buffer,
624619
while (true) {
625620
streams_stream_error_t error;
626621
uint64_t count;
627-
if (!streams_method_output_stream_check_write(tx_borrow, &count, &error)) {
622+
if (!streams_method_output_stream_check_write(tx_borrow, &count, &error))
628623
// TODO wasi-sockets: wasi-sockets has no way to recover stream errors yet.
629-
errno = EPIPE;
630-
return -1;
631-
}
624+
return wasip2_handle_write_error(error);
632625

633626
if (count) {
634627
count = count < length ? count : length;
635628
wasip2_list_u8_t list = { .ptr = (uint8_t *)buffer, .len = count };
636629
if (!streams_method_output_stream_write(tx_borrow, &list, &error)) {
637630
// TODO wasi-sockets: wasi-sockets has no way to recover TCP stream errors yet.
638-
errno = EPIPE;
639-
return -1;
631+
return wasip2_handle_write_error(error);
640632
} else {
641633
return count;
642634
}

0 commit comments

Comments
 (0)