Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for pipes in poll_oneoff on Windows #870

Open
marmistrz opened this issue Jan 28, 2020 · 4 comments
Open

Support for pipes in poll_oneoff on Windows #870

marmistrz opened this issue Jan 28, 2020 · 4 comments
Labels
wasi:impl Issues pertaining to WASI implementation in Wasmtime

Comments

@marmistrz
Copy link
Collaborator

marmistrz commented Jan 28, 2020

#552 introduced a minimal viable implementation of poll_oneoff, which consciously did not properly support the following file types on Windows: (quoting MSDN)

  • FILE_TYPE_CHAR - a character file, typically an LPT device or a console.
  • FILE_TYPE_PIPE - a socket, a named pipe, or an anonymous pipe.
  • FILE_TYPE_UNKNOWN - type of the specified file is unknown

I think it's completely fine not to support character files. The most common use case, the Windows console, is already handle by Descriptor::Stdin and friends. Otherwise, it seems to be mostly used for devices connected using the LPT and COM ports. Support for this in wasmtime looks completely out of scope. There seems to be no way of peeking the character files or polling them in any sensible way.

I'm unsure about the error code to be returned, hesitating between EINVAL and ENOTSUP and am more inclined toward the latter. It's not an invalid argument but an operation that is not supported on the file descriptor. (this the way it was handled in #552)

When the file is FILE_TYPE_UNKNOWN, we can't basically do anything useful with it, so I'd go for ENOTSUP. (i.e. stick with the current implementation)

I'm unsure about pipes. We don't support sockets yet and wasm applications have no way of creating pipes on their own. It looks like it's also impossible for the app to open a named pipe created by another process, because paths of form \\.\pipe\PipeName are not available in path_open.
It seems that the only way an app could possibly have access to a pipe would be through WasiCtx, analogously to what is done here:

let (reader, _writer) = os_pipe::pipe()?;
builder = builder.stdin(reader_to_file(reader));

As for the implementation, we could possibly use PeekNamedPipe to find out whether there's any data available inside the pipe, but this would require a busy looping with sleep, similarly to what was done in the initial implementation of #552

What do you think, should we properly support polling of pipes in wasmtime?

cc @kubkon @sunfishcode @peterhuene

@marmistrz marmistrz added the wasi:impl Issues pertaining to WASI implementation in Wasmtime label Jan 28, 2020
@joshtriplett
Copy link
Member

I don't think we should ever translate a poll call to a busyloop; applications calling poll expect to properly sleep, not busyloop.

It may be possible to implement waiting on pipes / named pipes by using overlapped I/O. It's a different model than the UNIX-style select model (it supplies the data when ready, and not just an indication of data available), and it may have some impedance mismatch, but it should allow waiting on input without spinning.

@marmistrz
Copy link
Collaborator Author

This would require a major overhaul of the whole I/O implementation.

Since overlapped I/O provides the data when it's available, wasmtime would need to buffer all (or some) I/O, so that it's not silently dropped. On the other hand, using overlapped I/O (if viable) would also remove the necessity to spin up a separate thread when polling stdin.

@kubkon
Copy link
Member

kubkon commented Feb 24, 2020

FWIW, nonblocking calls are on the roadmap for wasi-common, we're just not there yet ;-) I'm not sure we've got any precise timeline in mind yet either, but I may be mistaken. @sunfishcode @pchickey would you care to chip in?

@pchickey
Copy link
Collaborator

pchickey commented Feb 25, 2020

We're currently using Lucet's suspend/resume functionality so that we can implement non-WASI(-yet) HTTP interfaces using async Rust code (Tokio). We haven't integrated that with WASI's scheduler yet, in part because we've been busy iterating on the HTTP interface itself, and in part because any async based solution will require suspending the Wasm stack, which Wasmtime doesn't support yet. We have to weigh the tradeoffs, but I think with the continuations proposal Andreas made at the CG meeting, Wasmtime will need to add stack-switching functionality, and so we may be able to use async directly in wasi-common.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasi:impl Issues pertaining to WASI implementation in Wasmtime
Projects
None yet
Development

No branches or pull requests

4 participants