Support using custom sockets with libuv or libnative #15741
Conversation
|
This would also allow to write network daemons with socket activation support, for instance. |
| /// Convert a from a runtime I/O error to a IoError | ||
| /// | ||
| /// Primarily useful when using `IoFactory::socket_from_raw_fd()`. | ||
| pub fn from_rtio_error(err: rtio::IoError) -> IoError { |
There was a problem hiding this comment.
The details of rtio are largely private, and I would rather not make this a public function as rtio should in general be hidden. How come this was made public?
There was a problem hiding this comment.
So in order to use socket_from_raw_fd, you do the following (as std::io::* does for anything else in IoFactory):
LocalIo::maybe_raise(|io| {
io.socket_from_raw_fd(...)
...
}).map_err(IoError::from_rtio_error)To convert from rt::IoError to std::io::IoError. Since it will be used outside of std, there needs to be a way to convert the error. I'd happily make this private if you can think of a way to avoid this.
There was a problem hiding this comment.
In general our story for extending I/O isn't so great at the moment. As you've discovered it looks like there's going to be lots of duplication between librustuv and libnative if we go down this road, which is a little unfortunate. I think this falls into the same category of where this shouldn't really exist, but it's an unfortunate necessity.
|
I believe I've fixed all the things you brought up @alexcrichton, with the exception of code duplication/other things inherent to the way I/O is currently done - let me know if I missed anything. |
| // See above for what these are | ||
| refcount: Refcount, | ||
| read_access: AccessTimeout, | ||
| write_access: AccessTimeout, |
There was a problem hiding this comment.
These three fields aren't necessary if the handle isn't cloneable.
There was a problem hiding this comment.
Ah I see it is now cloneable
|
I'm not entirely thrilled about all the duplication between librustuv and libnative, this seems like a large indication that a better design is possible here. That being said this is all experimental and hidden from normal users so we can stomach this for now certainly. Also, there are 0 tests for this new functionality, is there any way that we can add tests? |
|
What kind of tests would you like? There aren't many (if any) tests for any of the related code in these crates. I can certainly add some for this. |
|
The vast majority of the tests are all in Some tests you may want to write:
You'll want to be pretty careful with the tests because there's not a whole lot of error handling here so the tests are susceptible to EINTR or possibly EAGAIN spuriously |
|
I have a full suite of tests for these in my own library - these require a lot of additional code (and root access) unfortunately so cannot really be included here. I'll try and make up some lower level tests without the dependencies/root requirement which could be included. |
0833b53 to
bbf24b2
Compare
This is akin to supporting custom file descriptors, but for sockets. This allows for the implementation of raw sockets in an external library, as well as integration with other libraries that use sockets such as zeromq.
This fixes a number of issues discussed in the pull request.
|
@alexcrichton Since it looks like libgreen/librustuv are going away (https://github.com/rust-lang/meeting-minutes/blob/master/workweek-2014-08-18/threading-model.md#summarizing-action-items) is it worth me writing the test cases for this? I assume all the I/O stuff will be getting a major rewriting making most of this irrelevant? Would I be best just using |
|
There are some large architectural changes scheduled for libnative/libgreen, and it may be worth holding off on this until after the dust settles. I would very much like to support use cases such as this, and @aturon will be writing some RFCs soon to detail our thinking. This code will almost certainly have its place in the rearchitecting, it may just sadly take a few moments to get there :( |
|
Ok, no problem. I'll keep an eye out for the RFCs. |
|
@alexcrichton If I made a separate pull request for the |
|
Yeah I think that'd be ok. |
|
Sure - in the meantime I'm going to get my library to use liblibc directly and only support the native backend, avoiding the need for this entirely. I'd appreciate pinging if any discussions of this kind of thing comes up in the RFC/implementation phase, I'll try to keep any eye out anyway though. |
|
Sure thing, will do! Thanks for being so patient @mrmonday! |
…chton These are the additions to liblibc required for raw/custom socket support. I've broken this into a separate pull request due to the upcoming I/O overhaul (was originally part of pull #15741). cc @alexcrichton.
|
cc #15643 |
|
A follow up RFC is now available. |
This is akin to supporting custom file descriptors, but for sockets. This allows for the implementation of raw sockets in an external library, as well as integration with other libraries that use sockets such as zeromq.
It exposes a lot more of liblibc than might be desirable, but the other alternative is to add a lot of additional code which probably shouldn't be in the standard library (it quickly explodes into something monstrous like pull #11410, and that was by no means complete). This should be a minimal subset which will allow other libraries to build upon.