RFR 8245194: Unix domain socket channel implementation

[Alan Bateman]

On 18/06/2020 15:31, Michael McMahon wrote:
>
> Hi,
>
> I'd like to start the review for JEP-380 (Unix Domain Socket Channels) 
> [1]. The first and smaller part of this JEP (8241305: Add protocol 
> specific factory creation methods to SocketChannel and 
> ServerSocketChannel) has already been integrated in JDK 15. This main 
> part of the JEP will hopefully be targeted to 16 under this bugid 
> (8245194 Unix Domain socket channel implementation [3])
>
> The full webrev [4] touches a lot of files, so I have put the public 
> API change in a separate webrev at [5] and there is a specdiff at [6].
>
> The implementation, while it touches a lot of files, is mostly about 
> re-factoring the existing SocketImpl and ServerSocketImpl 
> implementation classes into separate Inet and Unix variants, along 
> with the new implementation code for Unix domain.
>
> Comments welcome on either the implementation or the API, although I 
> would like to concentrate on the API to start with as I expect the 
> review will have several iterations.
>
I've read through the API and overall I think this looks very good and 
fits well with the existing API. Splitting this into API + 
implementation for review purposes is good as I agree the implementation 
changes will go through several review cycles.

The javadoc updates to SocketChannel/ServerSocketChannel are good except 
connect(SocketAddress) where it creates an inconsistency with "For 
Internet Protocol channels ..." and "For Unix Domain channels". The 
existing API tries to talk about the channel's socket so I think connect 
should follow this (you've got it right in the other additions, or 
rather than I didn't spot other inconsistencies). Another place with the 
same issue is the wording in in java.net.NetworkPermission.

SelectorProvider::inheritedChannel. "... where the inherited channel 
represents a Unix domain socket" is consistent with the existing javadoc 
but it inconsistent with the javadoc in the channel classes where they 
create a socket channel "for an Internet protocol sockets". So if you 
are okay with it, I think we should adjust the existing inheritedChannel 
javadoc as part of this.

UnixDomainPrincipal and the ExtendedSocketOption.SO_PEERCRED socket 
option looks good. UnixDomainPrincipal will probably need to link to the 
description for Unix domain sockets.

I'm in two minds on adding UnixDomainSocketAddress to java.nio.channels 
as it could be argued that it should be in java.net with SocketAddress 
and InetSocketAddress. I need to think a bit more about this one.

I have a few comments on the updates to the package description but 
overall I think the API additions are in good shape. Okay with me if you 
want get the CSR submitted (propose rather and finalize) and then update 
it later when it is final.

-Alan











-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20200629/6086c614/attachment.htm>