Patch for adding SO_REUSEPORT socket option

Alan Bateman Alan.Bateman at oracle.com
Fri Jan 29 14:37:37 UTC 2016


On 28/01/2016 06:03, Lu, Yingqi wrote:
> Hi Alan,
>
> Here is a new version of the patch: http://cr.openjdk.java.net/~mcberg/jdk/6432031/webrev.08/
>
> In this version, based on your comment, we have changed following items:
>
> 1. Remove the dependency of DatagramSocketImpl and MulticastSocket with AbstractPlainSocketImpl.
>
> 2. Made addReusePortOption() to be both thread safe and package private.
>
> Please let us know your feedback and comments.
>
>
I looking the latest patch and it's looking quite good but I think there 
is still an issue with the supportedOptions() methods in the java.net 
classes. You changes also highlight an existing bug in 
SocketImpl::supportedOptions whereby it returns a mutable rather than 
immutable set - I'll create a bug for this.

The issue is that the patch changes SocketImpl.socketOptions and 
SocketImpl.serverSocketOptions and so these sets of options will be 
incorrect if an alternative SocketImpl is created.

For now then I think the simplest is to change 
AbstractPlainSocketImpl.supportedOptions() to:

         @Override
         protected Set<SocketOption<?>> supportedOptions() {
             Set<SocketOption<?>> options = super.supportedOptions();
             if (isReusePortAvailable()) {
                 options = new HashSet<>(options);
                 options.add(StandardSocketOptions.SO_REUSEPORT);
                 return Collections.unmodifiableSet(options);
             } else {
                 return options;
             }
         }

That will ensure that the caller get an immutable set of options that 
include SO_REUSEPORT when it supported. Clearly it would be better to 
cache this but we can sort that later once you are okay with not 
changing the set at the level of SocketImpl.

Otherwise just a few minor comments:

- in src/java.base/windows/native/libnet/net_util_md.h then it might be 
better to put the #define SO_REUSEPORT near the top or at least before 
the function prototypes.

- test/java/nio/channels/DatagramChannel/MulticastSendReceiveTests.java 
- I assume this change is not needed.

- test/java/nio/channels/DatagramChannel/SocketOptionTests.java - the 
Arrays.asList could be formatted better to keep the line length consistent.

That's all I have, I think we are close to the final line on this one.

-Alan.


More information about the net-dev mailing list