Skip to content

poll_oneoff() implementation #12

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

Closed
cjihrig opened this issue Oct 2, 2019 · 7 comments · Fixed by #117
Closed

poll_oneoff() implementation #12

cjihrig opened this issue Oct 2, 2019 · 7 comments · Fixed by #117

Comments

@cjihrig
Copy link
Collaborator

cjihrig commented Oct 2, 2019

This and the sockets calls (which might be going away from WASI anyway) are the only completely unimplemented calls.

I'm still trying to figure out the best way forward here. From what I can tell, this is a synchronous call, so I'm not sure that uv_poll_t can be used.

Also, as @saghul pointed out in nodejs/wasi#21 (comment), polling non-sockets on Windows could be an issue.

@saghul
Copy link
Member

saghul commented Oct 2, 2019

I guess you could create a new loop, put all poll handles in there and wait for it to finish, then return. Somewhat akin to what Node does for spawnSync, IIRC.

Now, if socket calls are going away what stuff can one poll on, just timers?

@cjihrig
Copy link
Collaborator Author

cjihrig commented Oct 2, 2019

Now, if socket calls are going away what stuff can one poll on, just timers?

According to the docs, you can poll on __WASI_EVENTTYPE_CLOCK, __WASI_EVENTTYPE_FD_READ, and __WASI_EVENTTYPE_FD_WRITE. Even if the socket API calls go away, I wonder if it would be feasible (on Windows only) to open a socket along with each file descriptor for polling purposes. I'm just kind of thinking out loud though.

Unfortunately, from what I can tell, the WASI API wasn't designed with first class Windows support in mind.

@saghul
Copy link
Member

saghul commented Oct 2, 2019

I see. The fun fact is that in it's current shape we can't use uv_poll_t for non-scket fds, because epoll doesn't support files... You could roll a manual poll() for the fds, because that works with files, but it's broken on macOS and on Windows IIRC.

So that leaves timers. Maybe a good starting point is to just implement timers and claim that any other event type is always ready?

Also thinking out loud here :-)

@cjihrig
Copy link
Collaborator Author

cjihrig commented Oct 2, 2019

@cjihrig
Copy link
Collaborator Author

cjihrig commented May 6, 2020

Hi @saghul 👋

I've been revisiting this lately. I think I have an implementation based on uv_poll_t mostly working. I had two questions/ideas that I wanted to run by you:

  1. Regarding Windows support - uv_poll_t only works with sockets on Windows. Based on these poll() docs, I think it would be OK to always return success for regular files as well. Anything else can just not be supported on Windows for now. Does that make sense to you?

  2. One potential issue that I see is that libuv doesn't support multiple poll handles for the same file. However, there is nothing stopping a WASI user from calling poll_oneoff() with the same descriptor multiple times, potentially polling for different events on it. My thought here was to detect duplicate inputs and dup() the fd for the duration of the poll. Do you think that would work?

@saghul
Copy link
Member

saghul commented May 6, 2020

1. Regarding Windows support - `uv_poll_t` only works with sockets on Windows. Based on [these `poll()` docs](https://docs.oracle.com/cd/E23824_01/html/821-1463/poll-2.html), I think it would be OK to always return success for regular files as well. Anything else can just not be supported on Windows for now. Does that make sense to you?

Yeah!

2\. One potential issue that I see is that libuv doesn't support multiple poll handles for the same file. However, there is nothing stopping a WASI user from calling `poll_oneoff()` with the same descriptor multiple times, potentially polling for different events on it. My thought here was to detect duplicate inputs and `dup()` the fd for the duration of the poll. Do you think that would work?

That might work. But since you also need to keep track of the fds (right?) maybe just remeember them and edit the event mask, then rearm the poll handle?

@cjihrig
Copy link
Collaborator Author

cjihrig commented May 7, 2020

But since you also need to keep track of the fds (right?) maybe just remeember them and edit the event mask, then rearm the poll handle?

Yes, there is a fair amount of state, including the fds, to track. I'm going to try your suggestion. Thanks!

cjihrig added a commit that referenced this issue May 8, 2020
The implementation is currently limited by what
uv_poll_t's support.

Fixes: #12
cjihrig added a commit that referenced this issue May 10, 2020
The implementation is currently limited by what
uv_poll_t's support.

Fixes: #12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants