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 non-blocking files #1500

Open
codefromthecrypt opened this issue Jun 2, 2023 · 9 comments
Open

support non-blocking files #1500

codefromthecrypt opened this issue Jun 2, 2023 · 9 comments
Labels
enhancement New feature or request

Comments

@codefromthecrypt
Copy link
Collaborator

Is your feature request related to a problem? Please describe.

Currently, we support non-blocking stdio and sockets, but not files. This leads to an awkward and unnecessary skip in go's tests.

Describe the solution you'd like

Go supports non-blocking access and we support native access to files, so we should be able to implement this. If there are any caveats about windows, support can begin with a limited set of platforms.

Describe alternatives you've considered

continue to not do this

Additional context

our file type already has a SetNonblock setting, so that's a start. We also have a separate file implementation for raw os-backed files. So, we should be able to figure out why we aren't implementing non-blocking access for os files, then figure out how to implement it.

@codefromthecrypt codefromthecrypt added the enhancement New feature or request label Jun 2, 2023
@evacchi
Copy link
Contributor

evacchi commented Jun 2, 2023

a quick summary of what's required to make this works.

In theory, we can easily enable SetNonblock() for regular files, but effectively it would only work on *NIX. In fact, the syscall.SetNonblock() fun on Windows is a no-op.

Now, even if we did support that only on *NIX, we should still fix poll_oneoff; because as much as it would still work without it (read() returns EAGAIN and you could just retry after a little), existing code may still assume that select works.

so:

  • we can enable syscall.Setnonblock() on regular files
  • it won't work on Windows unless we explicitly implement something
  • read() now does not block and may return EAGAIN
  • we should fix poll_oneoff because it is currently special-cased for stdin.

now the good news about poll_oneoff is that there actually is a select() implementation that works on Windows
... the bad news it only works on sockets. Better than nothing.

So in short:

  • regular files:
  • easy to set the flag for nonblocking
  • requires reworking poll_oneoff (but this can be done in a separate PR)
    • won't work for free on Windows
    • would work almost for free on Windows+Sockets (via WinSock select)

EDIT: on a second thought. Apparently regular files always return immediately from poll_oneoff. So technically our implementation is correct also on Windows 😅

Now I am tempted to take a look at sockets 🤔

@codefromthecrypt
Copy link
Collaborator Author

some trouble in HTTP land. I began troubleshooting with @chriso but I think we should all be able to enjoy a weekend and I don't think this will quickly resolve. #1503

@codefromthecrypt
Copy link
Collaborator Author

so next step is to really look carefully at windows and sockets, and find some real-world wasm (very hard to find, like this and this) which can verify assumptions.

@evacchi
Copy link
Contributor

evacchi commented Jun 4, 2023

as for PR #1502, this only enables non-blocking I/O for files on *NIX.

Interesting notes, really on regular files poll always returns OK, except that then read() may still return EAGAIN l https://www.remlab.net/op/nonblock.shtml. (In the tests we are using pipes). On Windows Pipes do exist but they require quite a bit of boilerplate to initialize, to the point there are literally go libraries that only do that https://github.com/gesellix/go-npipe https://github.com/natefinch/npipe

Summary for further work on Windows.

Long story short, it should be feasible to provide select, and thus poll_oneoff for sockets on Windows too, but it will require some rework of the current poll_oneoff impl, which is really ad-hoc for stdin :^)

@codefromthecrypt
Copy link
Collaborator Author

Thanks for the update. Really good info!

Aside: I think one thing we need before closing this also, is a gotip version of the zig non-blocking test, as it isn't intuitive how to set it up. the question came up on slack by @tegk.

@evacchi
Copy link
Contributor

evacchi commented Jun 15, 2023

some further work in #1517 on unix. This should address most issues on this platform (with some love probably still needed for poll_oneoff), while work on Windows needs the most attention

@chriso
Copy link
Contributor

chriso commented Jun 27, 2023

Related: #1538

evacchi added a commit to evacchi/wazero that referenced this issue Jul 9, 2023
Improve tetratelabs#1500 with a simple observation, i.e. we really
need non-blocking I/O only for pipes and sockets.

Now, for sockets we can use WinSock select, as for pipes,
select_windows.go already imports PeekNamedPipe.

We combine PeekNamedPipe with a blocking Read only for
those cases when PeekNamedPipe returns n > 0, which
means a Read of n bytes won't block.

We special case n == 0 to return EAGAIN and
ERROR_BROKEN_PIPE as a non-failure (EOF reached).

We also introduce `isNonblock(f)` function, specialized
on Windows to also check if the file is ModeNamedPipe;
otherwise we would default to blocking anyway.

Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
@evacchi
Copy link
Contributor

evacchi commented Jul 10, 2023

#1570 improved support to pipes on Windows. I think that if we fix sockets (just use WinSock select) and we tie everything together in poll_oneoff, we are getting closer to resolving this!

@shynome
Copy link

shynome commented May 16, 2024

hi, is there any api to set nonblock

this make me fall down again. :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants