poll, pselect: handle POLLPRI / errorfds gracefully

[paralin]

pselect() returned ENOSYS when errorfds had any bits set. This breaks programs that use the standard POSIX pattern of passing exceptfds to select() for EOF/error monitoring (e.g., vim, ncurses applications).

Since WASI has no out-of-band data mechanism, errorfds can never have any exceptional conditions to report. Clear it instead of rejecting the call, matching the behavior callers expect from a POSIX-compatible select().

[paralin]

vim and other programs hit this ENOSYS and break/exit; so returning all-zero is correct.

in practice there are only two cases I found where these "exceptional cases" would ever occur:

1. TCP out-of-band (OOB) data: TCP has a "urgent" flag: sender can mark a single byte as out-of-band. select() reports the socket as having an exceptional condition.
2. Pseudoterminal packet mode: (via TIOCPKT)

exceptfds doesn't report general errors like broken pipe or connection reset, these would show up as errors via getsockopt(SO_ERROR).

[alexcrichton]

Reading over the pselect man page, it seems like the "most correct" thing to do here would be to turn anything in exceptfds into a POLLPRI request sent to poll. If that were done, however, I believe poll would fail in the same way and return ENOSYS as well.

Personally it feels incorrect to me to ignore parameters and proceed since if those were the only fds set, for example, then there's no way for the function to wake up. Overall I feel that returning an error for "you requested to do something that isn't implementable" is the right behavior here as it's an accurate reflection of the state of the world. Would it be possible to patch the application you're working with to avoid using exceptfds on WASI platforms?

[paralin]

It's obviously possible to patch the programs, but I have a feeling it would end up becoming a nightmare patching every single program that calls this API. Fwiw, I tested it and it works correctly with the PR as is. If there's a more correct way to do it, I can look into it. Emulating the condition never happening seems to make sense to me, for compatibility, as opposed to ENOSYS, particularly with it being such a rare condition anyway. But I understand if you want to reject.

I guess I could send a PR to vim to handle ENOSYS for this call properly.

[alexcrichton]

How about a variation of this:

• Update the implementation of pselect to process exceptfds by threading POLLPRI to poll. This'd resolve the usage of pselect and move the "problem child" to poll itself.
• Within poll if anything is listening for something other than POLLPRI allow the operation to happen (e.g. pretending it never happens since it's not implemented in wasi-libc). If, however, POLLPRI is the exclusive set of things being listened on an error is returned to indicate that it's not supported.

That should get pselect working I believe for these use cases where there's something else to listen with, and it'll also preserve a bit of the original intent of "this isn't actually implemented" too. Does that sound reasonable?

If so would you be up for including a test here as well to exercise this new behavior?

[paralin]

I'll work on it! Thanks!

[paralin]

@alexcrichton POLLPRI - usually 0x002 - is not defined in wasi-libc and it's already taken actually -

POLLWRNORM 0x2 <-- 0x002 conflicts here

I defined it in my PR as 0x0200 since that was unused. It's only used internally and never passed to the WASI API, so I think this is ok.

This still makes me a bit nervous as I worry that some programs will only sometimes call this with exclusively POLLPRI fds, get an unexpected ENOSYS, and crash, and now it'll be a bit harder to predict if this will happen as it's only sometimes that it will return ENOSYS.

I added the test as requested - ready for your review.

[paralin]

CI green 👍🏽

[alexcrichton]

Reading over this it all looks pretty good, thanks! W.r.t POLLPRI that's currently setup to be a public symbol exposed to wasi-libc users, which is also where the CI failures originally came from (thanks for bearing through the failures...)

You have a good point though, and it also raises another question for me. You say that vim and other programs were the impetus here, but do know of more details about the use case here? For example do you know if these applications just blanket insert POLLPRI to all file descriptors? Or are they specifically looking for a particular event on a specific file descriptor (e.g. some tty event or some out-of-band TCP data). Additionally, with POLLPRI missing from wasi-libc, how were these applications compiled against wasi-libc in the first place?

[paralin]

@alexcrichton I patched wasi-libc! Check here: skiffos/buildroot@f6cb880

Check this out:

To compile:

git clone --recursive https://github.com/skiffos/skiffos.git skiffos
cd ./skiffos
export SKIFF_CONFIG=virt/wasm
export SKIFF_WORKSPACE=wasm
make configure compile

Then workspaces/wasm/target has the rootfs with all the wasm binaries.

To answer your question:

I cross-compiled vim to wasm32-wasi using the buildroot/wasi-sdk toolchain. vim (and programs like it) use select() with errorfds, not poll() with POLLPRI directly.

Vim's RealWaitForChar in src/os_unix.c (inside #ifdef HAVE_SELECT, line ~6564):

FD_SET(fd, &rfds);
#ifndef __QNX__
FD_SET(fd, &efds);   // monitor for exceptional conditions (EOF)
#endif
// ...
ret = select(maxfd + 1, &rfds, &wfds, &efds, tvp);

vim adds the specific terminal fd (and optionally GPM mouse fd, XSMP ICE fd) to both rfds and efds. The efds usage is the standard POSIX pattern for EOF/error detection on those specific fds. The comment in vim source says "Select on ready for reading and exceptional condition (end of file)."

vim does have an ENOTSUP handler for select() returning -1, but it's guarded behind #ifdef __TANDEM (HP NonStop), so it wasn't compiled for WASI.

To your second question: these applications compiled fine against wasi-libc because they never reference POLLPRI. They use select()/pselect() with fd_set and FD_SET - all are defined in wasi-libc's headers. The compilation path is select() -> pselect() (both defined in wasi-libc). Applications calling poll() with POLLPRI directly would not have compiled before this PR (since the constant wasn't defined), but that's a much less common pattern than select() with errorfds.

[alexcrichton]

Oh right yes sorry of course you're not working with POLLPRI! Apologies I'm getting my wires crossed... Also, thanks for the context, that's quite helpful!

I think let's do this given all this information:

• I'm going to land this PR as-is. My rationale for this is that it's a clear compatibility win with existing applications. I'm also rationalizing the addition of POLLPRI insofar as the functionality is already exposed through select and pselect through exceptfds arguments, so there's no need to hide the implementation guts of poll.
• I continue to not be certain about the handling of returning an error with POLLPRI, so I'm going to file a follow-up issue to discuss this and see if we can't cc some more folks on that as well.

Thanks again for the PR, discussing this with me, and answering my questions. Much appreciated!

[paralin]

Thank you for your quick response on this PR and feedback + working with me to find a solution!