RFR 8245194: Unix domain socket channel implementation

[Alan Bateman]

On 17/07/2020 17:02, Alan Bateman wrote:
> On 17/07/2020 15:57, Michael McMahon wrote:
>> :
>>
>> I have added a new revision of the webrevs/specdiffs including these 
>> and Daniel's comments at:
>>
>> [1] http://cr.openjdk.java.net/~michaelm/8245194/api.webrev/webrev.5/
> How tied are you to adding getProtocolFamily to these APIs? It's 
> technically an incompatible change to add abstract methods to these 
> classes although the number of implementations outside of the JDK is 
> probably tiny. I guess I'm mostly wondering if the method is actually 
> useful. I could imagine a testing checking to see if a channel is 
> unbound and then binding it to a socket address that is appropriate 
> for the channel's socket but I'm less sure about other uses. Are you 
> mostly thinking diagnostics, in which case it can be in the string 
> representation.
One other comment on getProtocolFamily() is that if it is added then we 
should look at NetworkChannel as that's the interface that all channels 
to a socket implement.

A few other comments on the API webrev (v5):

NetPermission("allowUnixDomainChannels"). A coarse grain permission is 
fine but I'm not sure about the target name. It's not strictly tied to 
channels and the "allow" prefix is superfluous. You might want to 
consider something like "unixDomainSocket" and just specify that it is 
required in order to accept, bind, or connect a Unix domain socket.

UnixDomainSocketAddress. The javadoc for the factory methods should 
probably make it clear that the path can be empty. of(Path) might also 
need something to cover the corner case that the default file system is 
a custom file system (it will throw IAE for this case). The hashCode 
method specifies that the hash code is based on the path string where 
it's based on the Path object, might be simpler to just specify that it 
returns the hash code of the UnixDomainSocketAddress.

The javadoc updates to SocketChannel and ServerSocketChannel look okay 
to me. A few long length inconsistencies/formatting but these are just 
nits.

SelectorProvider. I don't think the list items need to repeat "Internet 
protocol" because it's already in the paragraph for the lists. Also just 
a minor point that there is no need for all the javadoc links to use 
qualified class names as SelectorProvider imports all these types.

The API additions to the jdk.net package look good too.

The first 3 paragraphs added to the java.nio.channels package 
description are okay.

I have a lot of comments on the additions to the java.nio.channel 
package description. Some of these paragraphs don't make sense here, 
e.g. paragraphs 4 and 5 candropped and replaced with an apiNote in 
DatagramChannel to say that channels to datagram-oriented sockets do not 
support Unix domain sockets. There is also a lot of text that is too 
specific to SocketChannel and ServerSocketChannel so I think we'll need 
to find a better place for this. I don't think there is any need to 
reference to the security manager as it is already covered by the API docs.

Are you proposing that the java.nio.channels.tmpdir property be standard 
or JDK specific? I think we need to know this before suggesting a name 
for this property.

Overall I think the API/docs are looking quite good, just not sure about 
getProtocolFamily and the additions to the package description will 
probably go through a few iterations.

-Alan.