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

Alan Bateman Alan.Bateman at oracle.com
Sun Oct 11 15:48:37 UTC 2020


I went through the latest patch, v18.

I'm still a bit uncomfortable with splitting up SocketChannelImpl and 
ServerSocketChannelImpl but okay with go along with this for now if we 
can continue explore alternatives. Given that the InetXXX and UnixXXX 
sub-classes aren't too big then one thing that we can try is just 
special casing the protocol family in the 4-5 methods that are 
different. In the mean-time, I think the abstract methods in 
SocketChannelImpl and ServerSocketChannelImpl should have a clear method 
description on what the implXXX method should. I would prefer the other 
methods be changed to final so that it's clear that they cannot be 
overridden. implSocket is new in v17 or v18 and I'd prefer if that were 
abstract so it can be implemented in UnixXXX to throw UOE.

I think the JFR events need another look. Are UnixSocketReadEvent and 
UnixSocketWriteEvent really needed? SocketXXXEvent "address" is just a 
String so it can be used for any address type, the port can be left as 
0. Part of this comment is wondering if a parallel set of events are 
really needed. Part of it that I would prefer if the read/write methods 
were final in SocketChannelImpl.

You might want to take a pass over the files in the patch as several of 
the new files have copyright dates starting 2012, 2019 and other years, 
probably initially copied from other files.

A few specific comments on v18:

src/java.base/share/classes/sun/nio/ch/UnixDomainSocketChannelImpl.java
- supportedOptions doesn't need to wrap the unmodifable collection with 
Collections.unmodifiableSet
- I think implBind should be re-worked so the local != null case is not 
handled in the loop body. That will help future maintainers.

src/java.base/windows/classes/sun/nio/ch/UnixDomainSocketsUtil.java
- I think we should look at adding JAVA_IO_TMPDIR to StaticProperty.

src/java.base/unix/native/libnio/ch/UnixDomainSockets.c
- are the AIX specific includes really needed?
- can unixSocketAddressToSockaddr use if-then-else to avoid the goto.
- bind0 checks if the path is NULL, is this a left over?
- the declaration of accept0 is mis-aligned, might be left over from 
refactoring

src/java.base/unix/native/libnio/ch/InheritedChannel.c
- might be clearer if matchFamily is renamed to toInetFamily

src/java.base/unix/classes/sun/nio/ch/InheritedChannel.java
- peerAddress0 is replaced by peerAddressInet and peerAddressUnix0 which 
looks a bit consistent, maybe rename those to inetPeerAddress0 and 
unixPeerAddress0 ?

src/java.base/windows/classes/sun/nio/ch/PipeImpl.java
- we've gone through a few iterations on this and only a few minor comments
- source/sink can be final and I assume no need to change their type 
from SourceChannel/SinkChannel
- no need to initialize noUnixDomainSockets to false
- can the comments at L178-179 be turned into a method description.

Can the tests for java.net.UnixDomainSocketAddress be moved to 
jdk/test/java/net/UnixSocketAddress?

Could test/jdk/java/nio/file/spi/TestProvider.java be used as 
alternative provider in the test to avoid DummyPath?

test/jdk/java/nio/channels/etc/ProtocolFamilies.java has been updated to 
add "expectedFamily", is that used?

test/jdk/java/nio/channels/unixdomain/WindowsDriver.java and 
NonWindowsDriver.java
- are these needed now? Both of them run the same 5 tests and not clear 
why these tests don't have their own @test tags.

test/jdk/java/nio/channels/unixdomain/Security.java
- is there any reason why this test can't run with run 
main/othervm/java.security.policy=java.policy1 -Djava.security.manager ?

That's all I have from this pass over the changes.

-Alan


More information about the nio-dev mailing list