Implement Unix domain sockets in libnative #11484
Conversation
| @@ -159,6 +159,8 @@ pub trait IoFactory { | |||
| fn unix_bind(&mut self, path: &CString) -> | |||
| Result<~RtioUnixListener, IoError>; | |||
| fn unix_connect(&mut self, path: &CString) -> Result<~RtioPipe, IoError>; | |||
| fn unix_dgram_connect(&mut self, path: &CString) -> Result<~RtioDatagramPipe, IoError>; | |||
| fn unix_dgram_bind(&mut self, path: &CString) -> Result<~RtioDatagramPipe, IoError>; | |||
There was a problem hiding this comment.
I'm wary to add these new primitives when we don't have a clear timeline to adding this support to librustuv. Do you have any idea if libuv is intending on supporting this soon? If so, then I think this is ok to put in, but if not, I don't want to let the I/O implementations fall too far out of sync. Right now libnative is playing catch-up, and I don't want to play a perpetual game of "libX needs to catch up to libY".
There was a problem hiding this comment.
The only reference I have seen is this: Dropping support for unix datagram sockets
There are two ways here: either we add a parameter to connect and bind to support the socket type (but uv would not support datagram), or those two methods could be exported from native::io::net, but that would fragment more the API.
I could try to add the feature in librustuv (it was my first intention because unix sockets already work there), but I had a hard time getting around the code.
There was a problem hiding this comment.
Interesting! Let's back out this addition from this patch, open an issue with your code, and have discussion continue there. If libuv intentionally removed this functionality, then we'll need to think carefully if we want to add it back.
There was a problem hiding this comment.
Alright, the patch can wait a bit until this is sorted out. I will need it anyway because I want to send messages to syslog, but that leaves me some time to write the syslog library itself :)
|
Do you know how well this patch works on windows? I would imagine that some errors should be raised somewhere, but I'm curious if it compiles and where it dies. |
|
I have not tested it on Windows yet. I'll check on Monday when I get another laptop. Is it possible to cross compile for Win32? I compile on a quad core server, and the build still takes a long time, so it will be even longer on my laptop. |
|
After verification: the socket() function on Windows does not have AF_UNIX, so it would probably compile, but return an error on the libc::socket call. It should be implemented with named pipes. I could take care of that too, but I would need some guidance on how to interact with the WIn32 API from Rust. |
|
I wouldn't try to provide a common API for Unix domain sockets and Windows named pipes. They are conceptually quite different. Unix domain sockets are point to point and bidirectional. You can have server sockets that can accept new point-to-point connections. Named pipes can be point-to-point as well as multipoint-to-point, unidirectional and bidirectional and there is no such thing as dedicated server socket. I would better say that Unix domain sockets are simply not supported in Windows and provide a special named pipe API for Windows. However I don't know if it's worth it, as they are not used that much. |
|
re windows, that behavior sounds fine to me. When this gets merged I'll update the ticket to indicate that windows support is all that remains. We're currently bringing the native I/O implementation up to par with the libuv-backed I/O implementation. For now I think it's a good idea to mirror libuv's api (at least in the stdlib for now). If the two use cases are fundamentally different, then I would like to explore other routes in order to have platform-specific I/O exposed from the standard library. For now, sticking to the existing traits should be "good enough" |
|
Oops, I don't know what happened there. I'll cherry pick and squash my commits in the final pull request. In the mean time: |
|
The diff views have gotten a little conflated from this now, could you rebase away all the already-merged commits? |
|
Alright, I squashed the commit, it should be easier to read now. I updated the code examples to match the API. |
| cast::transmute(storage) | ||
| }; | ||
| unsafe { | ||
| let buf:[libc::c_char, ..libc::sun_len] = storage.sun_path; |
There was a problem hiding this comment.
No need to copy sun_path into a local, you can call as_ptr on it directly.
|
Closing due to inactivity, but feel free to reopen with a rebase! |
|
I got stucked on over stuff in the meantime. I'm fixing the code, rebasing, and I'll push another patch before the end of the week. |
|
No problem, and thanks! |
|
Would love to see this get through. One of the last big steps before turning on libnative by default. |
[`extra_unused_type_parameters`]: Fix edge case FP for parameters in where bounds Generic parameters can end up being used on the left side of where-bounds if they are not directly bound but instead appear nested in some concrete generic type. Therefore, we should walk the left side of where bounds, but only if the bounded type is *not* a generic param, in which case we still need to ignore the bound. Fixes rust-lang#11302 changelog: [`extra_unused_type_parameters`]: Fix edge case false positive for parameters in where bounds
Fix #11201
No Windows named pipes for now and the libuv implementation is missing too
There is a special case for datagram sockets like /dev/log