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

Make poll return errno::inval if there are no subscriptions. #193

Merged
merged 1 commit into from
Jan 16, 2020

Conversation

sunfishcode
Copy link
Member

poll with no subscriptions is effectively an infinite hang. Since wasm
has no signals, use cases which involve waking up a libc pause call, which
is implemented in terms of a poll with no subscriptions, aren't applicable.
So all pause can do is suspend the program until the host environment
terminates it.

One situation where that's useful is in debugging, as it's sometimes useful
to be able to suspend a process in place so that it can be inspected by a
debugger. However, that doesn't require a fully infinite suspend; a suspend
with a long timeout is adequate.

Making the no-subscriptions case an error can help catch cases where
applications unintentionally enter infinite loops.

`poll` with no subscriptions is effectively an infinite hang. Since wasm
has no signals, use cases which involve waking up a libc `pause` call, which
is implemented in terms of a poll with no subscriptions, aren't applicable.
So all `pause` can do is suspend the program until the host environment
terminates it.

One situation where that's useful is in debugging, as it's sometimes useful
to be able to suspend a process in place so that it can be inspected by a
debugger. However, that doesn't require a fully infinite suspend; a suspend
with a long timeout is adequate.

Making the no-subscriptions case an error can help catch cases where
applications unintentionally enter infinite loops.
@marmistrz
Copy link
Contributor

I think that ENOTSUP would be more fortunate. On Linux this is a valid value for poll(2):

EINVAL (ppoll()) The timeout value expressed in *ip is invalid (negative).

But it's not supported by WASI.

@sunfishcode
Copy link
Member Author

That's a good point.

From a pure WASI perspective, this issue feels more like an EINVAL situation; then we'd just say that calling the poll syscall with no subscriptions is a bug, which is what EINVAL is for. But from a POSIX and Linux perspective the poll libc function does support poll with 0 subscriptions, so maybe we could have WASI libc return ENOTSUP in that case. Does that make sense?

@sunfishcode
Copy link
Member Author

We discussed this in the video call today, and there were no objections, so I'm calling it a consensus.

@sunfishcode sunfishcode merged commit 77629f3 into WebAssembly:master Jan 16, 2020
@sunfishcode sunfishcode deleted the pause branch January 16, 2020 22:42
sunfishcode added a commit to WebAssembly/wasi-libc that referenced this pull request Jan 16, 2020
With WebAssembly/WASI#193 merged, WASI is moving
to make `poll_oneoff` with no arguments an error. Even though that's in
ephemeral and not yet in a snapshot, we can start to anticipate it in
libc:
 - Remove the `pause` function, since WASI has no signals and thus no
   way to ever wake it up short of having the host terminate it.
 - Make `poll` and `pselect` return `ENOTSUP` in the case of having no
   events to wait for.
sunfishcode added a commit to WebAssembly/wasi-libc that referenced this pull request Jan 17, 2020
With WebAssembly/WASI#193 merged, WASI is moving
to make `poll_oneoff` with no arguments an error. Even though that's in
ephemeral and not yet in a snapshot, we can start to anticipate it in
libc:
 - Remove the `pause` function, since WASI has no signals and thus no
   way to ever wake it up short of having the host terminate it.
 - Make `poll` and `pselect` return `ENOTSUP` in the case of having no
   events to wait for.
marmistrz added a commit to marmistrz/wasmtime that referenced this pull request Jan 17, 2020
marmistrz added a commit to marmistrz/wasmtime that referenced this pull request Jan 17, 2020
marmistrz added a commit to marmistrz/wasmtime that referenced this pull request Jan 17, 2020
sunfishcode pushed a commit to bytecodealliance/wasmtime that referenced this pull request Jan 17, 2020
* Return EINVAL in poll_oneoff with no events.

We adhere to WebAssembly/WASI#193.

* Add a test for empty poll.
sunfishcode added a commit to WebAssembly/wasi-libc that referenced this pull request Feb 24, 2020
With WebAssembly/WASI#193 merged, WASI is moving
to make `poll_oneoff` with no arguments an error. Even though that's in
ephemeral and not yet in a snapshot, we can start to anticipate it in
libc:
 - Remove the `pause` function, since WASI has no signals and thus no
   way to ever wake it up short of having the host terminate it.
 - Make `poll` and `pselect` return `ENOTSUP` in the case of having no
   events to wait for.
sunfishcode added a commit to WebAssembly/wasi-libc that referenced this pull request Mar 3, 2020
With WebAssembly/WASI#193 merged, WASI is moving
to make `poll_oneoff` with no arguments an error. Even though that's in
ephemeral and not yet in a snapshot, we can start to anticipate it in
libc:
 - Remove the `pause` function, since WASI has no signals and thus no
   way to ever wake it up short of having the host terminate it.
 - Make `poll` and `pselect` return `ENOTSUP` in the case of having no
   events to wait for.
sunfishcode added a commit to WebAssembly/wasi-libc that referenced this pull request Mar 18, 2020
With WebAssembly/WASI#193 merged, WASI is moving
to make `poll_oneoff` with no arguments an error. Even though that's in
ephemeral and not yet in a snapshot, we can start to anticipate it in
libc:
 - Remove the `pause` function, since WASI has no signals and thus no
   way to ever wake it up short of having the host terminate it.
 - Make `poll` and `pselect` return `ENOTSUP` in the case of having no
   events to wait for.
sunfishcode added a commit to WebAssembly/wasi-libc that referenced this pull request Jun 2, 2020
With WebAssembly/WASI#193 merged, WASI is moving
to make `poll_oneoff` with no arguments an error. Even though that's in
ephemeral and not yet in a snapshot, we can start to anticipate it in
libc:
 - Remove the `pause` function, since WASI has no signals and thus no
   way to ever wake it up short of having the host terminate it.
 - Make `poll` and `pselect` return `ENOTSUP` in the case of having no
   events to wait for.
sunfishcode added a commit to WebAssembly/wasi-libc that referenced this pull request Jun 2, 2020
* Avoid calling `poll_oneoff` with zero subscriptions.

With WebAssembly/WASI#193 merged, WASI is moving
to make `poll_oneoff` with no arguments an error. Even though that's in
ephemeral and not yet in a snapshot, we can start to anticipate it in
libc:
 - Remove the `pause` function, since WASI has no signals and thus no
   way to ever wake it up short of having the host terminate it.
 - Make `poll` and `pselect` return `ENOTSUP` in the case of having no
   events to wait for.

* Remove `pause` from the defined-symbols.txt list.

* Fix __wasilibc_unmodified_upstream markers.

* Check for zero subscriptions, rather than zero events.

Make `poll` and `pselect` return `ENOTSUP` when asked to poll on zero
subscriptions, rather than when the systerm returns zero events.

While here, drop the `__wasilibc_unmodified_upstream` markers, which
were already pretty noisy here, and would be significantly worse with
this change.

* Add comments about the subtle relationship between nfds and nsubscriptions.

* Rewrite the comment.

* Fix code quotes.
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 this pull request may close these issues.

3 participants