RFR 8245194: Unix domain socket channel implementation

[Michael McMahon]

Hi Daniel,

Thanks for the review.

On 16/07/2020 16:28, Daniel Fuchs wrote:
> Hi Michael,
>
> The package-info for java.net has a section about Addresses.
> Given that UnixDomainSocketAddress is now in class in the
> java.net package, I wonder if, for clarity, that section
> (or the Socket section that comes after) should be updated
> to say that java.net sockets only support the
> Internet Protocol, and point to java.nio.channels for
> channels supporting the UNIX domain protocol?
>
> Maybe a sub-section "About UNIX domain" - similar to "About IPv6",
> in either one of these two sections?
>

I'm not sure. The package level docs for java.net don't mention
SocketAddress or InetSocketAddress either. Maybe it should have a paragraph
like that but I feel it's out of scope for this work.

I notice that there is no equivalent explanation relating to 
StandardProtocolFamily
which is also only used in NIO. Maybe both of these could be addressed 
in another
doc update?

> http://cr.openjdk.java.net/~michaelm/8245194/specdiff/specout.4/java.base/java/net/package-summary.html 
>
>
> Similarly - maybe UnixDomainSocketAddress could have an @see
> pointing to the SocketChannel and ServerSocketChannel?
>

I agree that is a simple change, which would be useful.

>
> In 
> http://cr.openjdk.java.net/~michaelm/8245194/specdiff/specout.4/java.base/java/nio/channels/ServerSocketChannel.html
> two args bind method:
>
> "SecurityException: If a security manager has been installed and its 
> checkListen method denies the operation for an Internet protocol 
> socket address or NetPermission("allowUnixDomainChannels") when the 
> socket address is a Unix domain socket address"
>
> It sounds like some words (verb?) are missing just before NetPermission?
> Possibly a comma or semicolon after "for an Internet protocol socket 
> address" could make the parsing easier too.
>
How about rewording as:

      * @throws  SecurityException
      *          If a security manager has been installed and its {@link
      *          SecurityManager#checkListen checkListen} method denies
      *          the operation for an <i>Internet protocol</i> socket 
address,
      *          or for a <i>Unix domain</i> socket address if it denies
      *          {@link NetPermission}{@code("allowUnixDomainChannels")}.

>
> In 
> http://cr.openjdk.java.net/~michaelm/8245194/specdiff/specout.4/java.base/java/nio/channels/SocketChannel.html
> open(SocketAddress) method:
>
> "UnresolvedAddressException - If the given remote -a-d-d-r-e-s-s- is 
> an InetSocketAddress that is not fully resolved"
>
> For consistency with the description of the method where "remote 
> address" is used, I would not remove the "address" word here.
>

Right. That should not have been removed.

> For the bind(SocketAddress), the @throws SecurityException clause seems
> to have the same issue than outlined above for ServerSocketChannel.
>

Yes, I suggest the same wording as above

> Otherwise the spec look good to me. Nice job of implementing the
> feature with such a small API surface!
>

Excellent. Thanks for the review

Michael

> best regards
>
> -- daniel
>
>