RFR: 8245194: Unix domain socket channel implementation [v34]

[Michael McMahon]

On Wed, 21 Oct 2020 16:38:16 GMT, Chris Hegarty <chegar at openjdk.org> wrote:

>> Michael McMahon has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 36 additional commits since the last revision:
>> 
>>  - Merge branch 'master' into unixdomainchannels
>>  - updated one error in review
>>  - further test update from Daniel
>>  - forgot to stage updated test files in last commit
>>  - update from Daniel's test comments
>>  - Merge branch 'master' into unixdomainchannels
>>  - final feedback from Alan
>>  - Merge branch 'master' into unixdomainchannels
>>  - - Alan's review patch from Oct 18
>>    - Reverted changes to JFR unit tests
>>  - - added NullTest and fix in SocketChannel.open
>>    - updated test bug ids
>>  - ... and 26 more: https://git.openjdk.java.net/jdk/compare/d3e47796...01d7cf53
>
> test/jdk/java/nio/channels/unixdomain/Bind.java line 51:
> 
>> 49:         if (!supported()) {
>> 50:             System.out.println("Unix domain channels not supported");
>> 51:             return;
> 
> Here it would be better to throw the `jtreg.SkippedException`, rather than silently returning.  Same comment for any other tests that check supported-ness before continuing to execute.

Ok

> test/jdk/java/nio/channels/unixdomain/Bind.java line 66:
> 
>> 64:             SocketChannel.open(StandardProtocolFamily.UNIX).close();
>> 65:         } catch (UnsupportedOperationException e) {
>> 66:             return false;
> 
> In this case `ServerSocketChannel.open(UNIX)` should also throw UOE.

I'll add a specific test that checks if `SocketChannel.open(UNIX)` throws UOE than `ServerSocketChannel.open(UNIX)` should also.

> test/jdk/java/nio/channels/unixdomain/policy1 line 1:
> 
>> 1: grant {
> 
> It is often the case that a copyright and license header is added to such policy files, policy1, policy2, policy3

OK

> test/jdk/java/nio/channels/unixdomain/NonBlockingAccept.java line 59:
> 
>> 57:             serverSocketChannel.bind(null);
>> 58:             SocketChannel socketChannel = serverSocketChannel.accept();
>> 59:             System.out.println("The socketChannel is : expected Null " + socketChannel);
> 
> The null-ness of the returned socket channel be asserted, regardless of whether or not a prior version of the code threw an exception.

OK

> test/jdk/java/nio/channels/unixdomain/IOExchanges.java line 49:
> 
>> 47: 
>> 48: public class IOExchanges {
>> 49:     static boolean supported = true;
> 
> This should really be `unixDomainSupported` or some such, to indication that it us used to seed the set of protocol families to test with; IP-only, or IP+Unix.  When I wrote this test originally, the intent was that it could operate with channels of different protocol types, IP, rsocket, and now Unix. Happy to see that it is finally being integrated. Maybe update the copyright year range to start at 2018, see https://cr.openjdk.java.net/~chegar/rsocket/IOExchanges/test/jdk/jdk/net/RdmaSockets/rsocket/SocketChannel/IOExchanges.java.html

Ok, thanks for review!

-------------

PR: https://git.openjdk.java.net/jdk/pull/52