Repository navigation
Merge sock_send/sock_recv with fd_write/fd_read #4
Description
WASI currently has two pairs of functions which are similar to each other: sock_send/sock_recv and fd_write/fd_read. This PR describes a plan for merging them, in favor of fd_write/fd_read.
Background
The reason why send and recv are separate in POSIX is that they add flags arguments. POSIX says that send and recv are equivalent to write and read when no flags are set.
WASI's sock_send doesn't currently support any flags. WASI's sock_recv supports __WASI_SOCK_RECV_PEEK and __WASI_SOCK_RECV_WAITALL which correspond to MSG_PEEK and MSG_WAITALL in POSIX. Both of these operations conceptually could work on files, however typical operating systems only support them on sockets.
On Linux, there is a subtle difference between recv and read: "If a zero-length datagram is pending, read(2) and recv() with a flags argument of zero provide different behavior. In this circumstance, read(2) has no effect (the datagram remains pending), while recv() consumes the pending datagram." It is possible that applications could depend on this subtle difference, but the only reference to it I've been able to find is the git commit which added this line to the man page, which describes a bug where "[...] we would end up in a busy loop when we were using read(2). Changing to recv(2) fixed the issue [...]". The recv behavior, is what the code in that bug wanted, and is the more intuitive behavior.
The cause of this subtlety is that read special-cases a 0 return value to mean the end-of-file/stream has been reached. That creates an ambiguity when reading a zero-length datagram.
Proposal
- Remove
sock_sendandsock_recv. - Add
__wasi_siflags_tand__wasi_riflags_targuments tofd_writeandfd_read, respectively. - Make
fd_readreturn__WASI_EMSGSIZEwhen receiving a datagram which is larger than the provided buffer. And remove__WASI_SOCK_RECV_DATA_TRUNCATED, which is whatsock_recvused in that case. WASI libc will check for this and to continue to implement the POSIX API (MSG_TRUNC). - Make
fd_readreturn__WASI_EEOS, a new errno code, when the end-of-file/stream is reached. This eliminates the ambiguity of the special case for 0. WASI libc will check for this and continue to implement the POSIX API with 0 being a special case. - Add rights for
__WASI_RIGHT_FD_READ_PEEKand__WASI_RIGHT_FD_READ_WAITALL, which are required to use the__WASI_SOCK_RECV_PEEKand__WASI_SOCK_RECV_WAITALLflags, respectively. These rights would not be granted for file-based file descriptors on OS's that don't support these features on files. - Remove the
fs_filetypefield from thefdstat_tstruct. This further hides unnecessary differences between sockets and files.fd_fdstat_getis an otherwise ambient authority, meaning anyone can do it on any open file descriptor. The file type is still accessible, viafd_filestat_get, but that requires (__WASI_RIGHT_FD_FILESTAT_GET). - That happens to leave us with no easy way to implement
isatty, so add a__WASI_RIGHT_FD_ISATTYright, to indicate whether a file descriptor is known to be a terminal. This is a little unusual as it's not a typical right, as it's not associated with an operation. However, this right makes it simple to implementisatty, which is used by libc to do line buffering for stdout when it's on a tty.
And some minor tidying:
- Rename
sock_shutdowntofd_shutdown, and make it a file descriptor operation that happens to depend on the__WASI_RIGHT_SOCK_SHUTDOWNright, which on typical implementations will only get granted for sockets. This is the last remainingsock_*function. - Rename
__WASI_RIGHT_SOCK_SHUTDOWNto__WASI_RIGHT_FD_SHUTDOWN. - Rename
__WASI_SOCK_RECV_PEEKand__WASI_SOCK_RECV_WAITALLto sayFD_READinstead ofSOCK_RECV.
Miscellaneous notes
The change to make fd_read return __WASI_EEOS on end-of-file/stream also fixes an oddity in POSIX in which many applications do an extra read call after the EOF is encountered, in order to get a 0 return from read to confirm they've actually reached the end. That said, implementations on POSIX hosts won't be able to report __WASI_EEOS until they get a 0 from read themselves, so in practice there will still be an extra read on such systems.
Activity
alexcrichton commentedon Apr 9, 2019
This all sounds like a great idea to me, funneling these through the same syscall seems pretty natural fits well with the existing capabilities and such. I'm curious about a few parts though:
I was initially wondering how this affects
isattyas it has to do with sockets/files, but reading up on the implementation it looks like__wasi_fd_fdstat_getis used to see whether it's a character device you can't seek/tell on. Would it make sense to getfd_filestat_getworking with character devices or should that only work with literal files on the filesystem?Otherwise, is this something that we'd have to add more rights for in the future? In that would new kinds of file descriptors get their own rights? (just wondering if this is a "slippery slope" or pretty contained)
I was curious about this and wondering how we'd manage that. Is it the intention that
__WASI_EEOSwould be returned on a successful read instead of__WASI_ESUCCESS? For example if I read a 15 byte file with a 30 byte buffer, would I read 15 bytes and get a__WASI_EEOSreturn code?sunfishcode commentedon Apr 9, 2019
My theory is that this isn't a slippery slope because
isattyis special by virtue of being one of the things implicitly called by libc forstdoutsupport in C (and similar in several other languages).fd_filestat_getdoes work on character devices, but I was trying to avoid importing it, since this affects hello world in these languages. However, you may be right that that's the better solution here, rather than the special__WASI_RIGHT_FD_ISATTYright.Yes, that's right. Another option is to add a bool output parameter to
fd_readto indicate end-of-file/stream explicitly. My rough sense is that an output parameter is partially redundant because it'll never be true when the return value isn't__WASI_ESUCCESS, however either way would work.alexcrichton commentedon Apr 10, 2019
Ah ok, if
fd_filestat_getworks then I might personally advocate for that rather than adding a new right, but of course leaving the option open to adding a new right later if it becomes necessary.For
__WASI_EEOSsounds good to me! I'd personally consider that the ergonomics of these APIs (e.g. how usable they are to call directly) isn't too important as they'll almost always want to be wrapped with something else. In that sense it's easy enough for libraries deep in the stack to check forEEOSorESUCCESSand everyone else wouldn't have to worry about it.npmccallum commentedon Apr 12, 2019
When you use these rights for double duty, how does the caller determine the difference between, expressed in POSIX errno terms,
ENOPERMandENOTSOCK?I would expect that using the flags on
read()andwrite()would tell me that the flags don't apply to my type of file descriptor not that I'm missing some right.npmccallum commentedon Apr 12, 2019
I haven't actually looked at the error handling part of WASI yet. However, I presume the libc implementation would return 15 in this case and expect a second
read()call. On the secondread()call we would return 0. Is this correct?taralx commentedon Apr 15, 2019
For what it's worth, we should encourage implementations to support the full set of operations even on files - no reason you couldn't support peek on seekable data, for example.
sunfishcode commentedon Apr 15, 2019
One option is to rewrite the errno value to
ENOTSOCKin libc; that's what CloudABI does to solve some related problems, for example.POSIX read() and write() work on both files and sockets. POSIX defines recv() and send() with no flags to be equivalent to read() and write(), except that they fail with
ENOTSOCKif used on files. This restriction is arbitrary; is it useful and worth keeping?Yes. You can always keep calling read after you reach EOF, and it'll return 0 (as long as the file isn't modified), so programs that expect to read until they get a 0 will work as intended.
npmccallum commentedon Apr 22, 2019
It sucks to consume a (likely) second context switch on
ENOTCAPABLE. But hopefully that isn't common. Hopefully it is also not in a critical code path. I suspect that is not likely.I think that is exactly my question. That behavior is very well known. I definitely imagine there are libraries that are using that behavior instead of a more explicit test. However, I don't have a good sense of the actual cost in choosing not to preserve that behavior.
+1
taralx commentedon Apr 22, 2019
I don't see why there would be a second switch - the socket-ness just has to be tracked by the libc (and probably should).
syrusakbary commentedon Oct 1, 2019
At @wasmerio we are very interested in moving sockets forward in WASI.
Are there any companies already working on this (or interested in working on it in the future), so we can start collaborating? (Mozilla? Fastly?)
I'd like to minimize any surprises when the spec advances, so Wasmer and all its users can use sockets as soon as the spec is ready.
Following that thought. I'd like to offer Wasmer workforce to start the development on the wasi-libc repo.
Merge `recv`/`send` into `read`/`write`.
sunfishcode commentedon Jul 19, 2022
This issue has been subsumed by the component model, which is adding a
streamtype which will provide read and write operations that we can use with all stream-like use cases.wasi-js-bindings: Basic implemention of sock_send and sock_recv
introduce `proc_exec2` (WebAssembly#4)
Port the wasi-clocks API to the new wit format.
Merge pull request WebAssembly#4 from sunfishcode/cli-reactor
Port the wasi-filesystem API to the new wai format. WebAssembly#35
Update to the latest wit-abi tag (WebAssembly#4)