jobserver: accept socket descriptors in addition to fifo/char-device - #2803
jobserver: accept socket descriptors in addition to fifo/char-device#2803xim wants to merge 1 commit into
Conversation
A jobserver pool may be implemented over a Unix domain socket pair rather than a named pipe. The token semantics are identical (one byte read = acquire a slot, one byte written = release a slot), and read/write/poll behave the same on a socket as on a pipe, so the descriptor-type sanity check should accept S_IFSOCK as well. This follows the precedent of accepting S_IFCHR for CUSE/FUSE-emulated FIFOs; Many other clients, including make itself, performs no such descriptor type check at all.
|
I have a branch https://github.com/xim/ninja/tree/jobserver-pool-with-fd-socket-support on top of v1.13.2 and this is a part of those changes.. In it, I've cherry-picked changes needed for connecting to a socket-based global jobserver. Once more of the cherry-picked changes are merged, I'd be very interesting in revisiting the decision to not support |
| return (ret == 0) && (((info.st_mode & S_IFMT) == S_IFIFO) || | ||
| ((info.st_mode & S_IFMT) == S_IFCHR)); | ||
| ((info.st_mode & S_IFMT) == S_IFCHR) || | ||
| ((info.st_mode & S_IFMT) == S_IFSOCK)); |
There was a problem hiding this comment.
thank you for this change. Can you add a comment here explaining which cases each value corresponds to for future maintenance? We may want to completely remove this check as well, it's here to catch simple user errors like typos, and is not critical to operations. Wdyt?
There was a problem hiding this comment.
S_IFIFOworks as specified by GNU Make 4.4+.S_IFCHRlooks like it would be https://wiki.gentoo.org/wiki/Steve - as close at it gets to FIFO behavior without breaking any promises, no over-reservation or over-commitment.- The use of
S_IFSOCKappears to be entirely undocumented in the wild - but still sensible to SOME extent, with the huge catch that you need a server which will ALWAYS lease out +1 extra token as a reserve ahead of time as there is no "request" mechanism possible despite the need to fork a new connection for each client. So you always leak/reserve one extra token per connected client, which is why this was never adopted.
But neither of the usage examples matter in this case. The only requirement for you as a CLIENT is that the file type needs to guarantee that tokens can't be read more than once - and all 3 of those now supported file types uphold that guarantee. The other remaining file types S_IFBLK, S_IFDIR, S_IFLNK and S_IFREG don't, which means the enumeration of supported file types is now complete.
There was a problem hiding this comment.
Maybe, but a comment in the source code that gives examples where these three cases are useful would be nice. @xim can you do so?
4 participants
A jobserver pool may be implemented over a Unix domain socket pair rather than a named pipe. The token semantics are identical (one byte read = acquire a slot, one byte written = release a slot), and read/write/poll behave the same on a socket as on a pipe, so the descriptor-type sanity check should accept S_IFSOCK as well.
This follows the precedent of accepting S_IFCHR for CUSE/FUSE-emulated FIFOs; Many other clients, including make itself, performs no such descriptor type check at all.