ShuP1 opened PR #5210 from c-api-stdio-pipe
to main
:
As discussed in issue #4372 and respective issues about python and go bindings.
Can replace
wasi_config_inherit_stdout
in examples/wasi/main.c:79wasi_config_t *wasi_config = wasi_config_new(); assert(wasi_config); wasi_read_pipe_t *wasi_stdout_reader = NULL; wasi_write_pipe_t *wasi_stdout_writer = NULL; wasi_pipe_new(32, &wasi_stdout_reader, &wasi_stdout_writer); assert(wasi_stdout_reader && wasi_stdout_writer); wasi_config_set_stdout_pipe(wasi_config, wasi_stdout_writer); error = wasmtime_context_set_wasi(context, wasi_config); // run and delete store size_t expected = wasi_read_pipe_len(wasi_stdout_reader); byte_t *buf = malloc(expected); size_t filled = wasi_read_pipe_read(wasi_stdout_reader, buf, expected); fprintf(stderr, "Pipe:\n%.*s\n", (int)filled, buf); wasi_read_pipe_delete(wasi_stdout_reader);
See https://github.com/ShuP1/wasmtime/commit/d818f6b8153c71f94283aa0e9c258e57e795e019
This PR replaces #5189
(cc @alexcrichton)
ShuP1 edited PR #5210 from c-api-stdio-pipe
to main
:
As discussed in issue #4372 and respective issues about python and go bindings.
Can replace
wasi_config_inherit_stdout
in examples/wasi/main.c:79wasi_config_t *wasi_config = wasi_config_new(); assert(wasi_config); wasi_read_pipe_t *wasi_stdout_reader = NULL; wasi_write_pipe_t *wasi_stdout_writer = NULL; wasi_pipe_new(32, &wasi_stdout_reader, &wasi_stdout_writer); assert(wasi_stdout_reader && wasi_stdout_writer); wasi_config_set_stdout_pipe(wasi_config, wasi_stdout_writer); error = wasmtime_context_set_wasi(context, wasi_config); // run... size_t expected = wasi_read_pipe_len(wasi_stdout_reader); byte_t *buf = malloc(expected); size_t filled = wasi_read_pipe_read(wasi_stdout_reader, buf, expected); fprintf(stderr, "Pipe:\n%.*s\n", (int)filled, buf); wasi_read_pipe_delete(wasi_stdout_reader);
See https://github.com/ShuP1/wasmtime/commit/d818f6b8153c71f94283aa0e9c258e57e795e019
This PR replaces #5189
(cc @alexcrichton)
alexcrichton created PR review comment:
I think it's ok to use
void*
instead ofbyte_t*
to be more akin toread
/write
alexcrichton created PR review comment:
I think it's ok to remove this "However ..." clause because pipes leading to deadlock aren't necessarily unique to WASI.
alexcrichton created PR review comment:
I think this can be
&mut *mut wasi_write_pipe_t
which should remove the need for this function to be unsafe.
alexcrichton created PR review comment:
I'm not sure what this is intending to communicate? Could you clarify what this is explaining?
alexcrichton submitted PR review.
alexcrichton created PR review comment:
I'm not sure that this read/write impl are the best for WASI. As modeled today I think it would be best to implement synchronous I/O here but this is sort of a form of nonblocking I/O without the ability to be notified when bytes are ready.
Specifically
Read
will not block when the deque is empty, instead returningOk(0)
which many libraries actually interpret as EOF. AdditionallyWrite
will not block when it's full, instead returningOk(0)
which I think libraries take various viewpoints on.I think it would be better for
Read
to block on writing and for writing to block onRead
.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
I think this can be omitted since it doesn't do anything
ShuP1 submitted PR review.
ShuP1 created PR review comment:
std::io::Write::flush
has not default body. IMO to force implementers to thinks about it.
ShuP1 created PR review comment:
Wasi fd_read (like POSIX one), returns the number of byte read.
Then the queue is empty, the returned value is zero.
Effects on programs may varies.
For example, a c program without checks will just ignore this.
While rust print macro unwrapsstd::io::Read::read_exact
and so panic.If you have a good way to explain that I'm taking it…
ShuP1 submitted PR review.
ShuP1 edited PR review comment.
ShuP1 submitted PR review.
ShuP1 created PR review comment:
Is it possible to "null check" a
&mut
since it's expected to be a valid reference ?If not, want would append if the function is given a null pointer ? UB?
Or, is a function documentation enough to be confident than the function is used properly ?
ShuP1 submitted PR review.
ShuP1 created PR review comment:
UNIX pipe returning 0 bytes read means not more data for now
Synchronous IO seems be more user friendly.
But there if not way to read all of stdin for example.
It would just be a deadlock after the last byte.Furthermore I don't see how we can't contact the runtime to suspend the execution and run the "writer" instance.
ShuP1 edited PR review comment.
ShuP1 edited PR review comment.
ShuP1 updated PR #5210 from c-api-stdio-pipe
to main
.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
Yes that makes sense but
VecDeque::flush
is a noop.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
My point is that the
&mut
layer is part of the contract of the function, it must be valid. What it points to could be invalid, hence the next layer of raw pointers.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
I don't think unix pipes return 0 when there's no more data, when the write pipe is still open then it blocks waiting for more data. I think the same thing should happen here, the reader blocks until the writer has written data or the writer has been dropped.
Last updated: Dec 23 2024 at 12:05 UTC