RFR 8245194: Unix domain socket channel implementation

[Michael McMahon]

Hi Alan

I realise I didn't reply to this on the list. Thanks for the review.
I have proposed the CSR as you suggested with the initial text changes 
you suggest,
but not the API change moving UnixDomainSocketAddress to java.net.

We can resolve that question before finalizing the CSR.

Thanks again,

Michael

On 29/06/2020 10:46, Alan Bateman wrote:
> 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/20200703/581f19f6/attachment.htm>