Patch for adding SO_REUSEPORT socket option

[Seán Coffey]

One comment  on the supportability side from me :

java/net/AbstractPlainDatagramSocketImpl.java

 > +         case SO_REUSEPORT:
 > +             if (o == null || !(o instanceof Boolean)) {
 > +                 throw new SocketException("bad argument for 
SO_REUSEPORT");
 > +             }
 > +             if 
(!supportedOptions().contains(StandardSocketOptions.SO_REUSEPORT)) {
 > +                 throw new 
UnsupportedOperationException("unsupported option");

Can we improve the message ? e.g "SO_REUSEPORT is an unsupported 
option"  - line occurs twice.

Ditto for java/net/AbstractPlainSocketImpl.java and 
java/net/PlainDatagramSocketImpl.java and java/net/PlainSocketImpl.java  
(windows & unix variants)


java/net/DualStackPlainDatagramSocketImpl.java, 
java/net/DualStackPlainSocketImpl.java, java/net/PlainSocketImpl.java, 
java/net/TwoStacksPlainDatagramSocketImpl.java and 
java/net/TwoStacksPlainSocketImpl.java also need tweaking on that message.

Thanks,
Sean.

On 17/02/16 11:43, Chris Hegarty wrote:
>
>> From: net-dev [mailto:net-dev-bounces at openjdk.java.net] On Behalf Of 
>> Lu, Yingqi
>> Sent: Thursday, February 11, 2016 9:47 AM
>> To: Alan Bateman <Alan.Bateman at oracle.com>; Volker Simonis 
>> <volker.simonis at gmail.com>; Michael McMahon 
>> <michael.x.mcmahon at oracle.com>
>> Cc: Kaczmarek, Eric <eric.kaczmarek at intel.com>; Viswanathan, Sandhya 
>> <sandhya.viswanathan at intel.com>; Kharbas, Kishor 
>> <kishor.kharbas at intel.com>; Aundhe, Shirish 
>> <shirish.aundhe at intel.com>; net-dev at openjdk.java.net
>> Subject: RE: Patch for adding SO_REUSEPORT socket option
>>
>> Hi Alan,
>>
>> Here is the most recent modification of the patch. The link is 
>> available here:
>> http://cr.openjdk.java.net/~mcberg/jdk/6432031/webrev.11/
>
> I know this has gone through several rounds of review already.
> I do not want to stall progress, I am reasonably happy with the
> changes, but I have a few comments ( that can be addressed later,
> if needed ).
>
>  - MulticastSocket.java: I would add a javadoc link to
>    {@linkplain StandardSocketOptions.SO_REUSEPORT SO_REUSEPORT}
>
>  - Maybe the socket impl set/getOption(..) methods should check
>    supportedOptions().contains(name) first.  There seems to be
>    many places where SO_REUSEPORT is special-cased, that could
>    be replaced with a general supported options check, no?
>
>  - How useful is it to add SO_REUSEPORT to non-listening stream
>    orientated sockets ?
>
> -Chris.
>
>
>
>> Please let us know if everything is all right.
>>
>> Again, thank you very much for your help!
>>
>> Thanks,
>> Lucy
>>
>>
>> -----Original Message-----
>> From: Alan Bateman [mailto:Alan.Bateman at oracle.com]
>> Sent: Wednesday, February 10, 2016 10:38 AM
>> To: Lu, Yingqi <yingqi.lu at intel.com>; Volker Simonis 
>> <volker.simonis at gmail.com>; Michael McMahon 
>> <michael.x.mcmahon at oracle.com>
>> Cc: net-dev at openjdk.java.net; Viswanathan, Sandhya 
>> <sandhya.viswanathan at intel.com>; Kharbas, Kishor 
>> <kishor.kharbas at intel.com>; Aundhe, Shirish 
>> <shirish.aundhe at intel.com>; Kaczmarek, Eric <eric.kaczmarek at intel.com>
>> Subject: Re: Patch for adding SO_REUSEPORT socket option
>>
>> On 05/02/2016 17:27, Lu, Yingqi wrote:
>>> Hi Alan,
>>>
>>> Here is the new modification of the patch. The link is available here:
>>> http://cr.openjdk.java.net/~mcberg/jdk/6432031/webrev.10/
>>>
>>> In this version, we make supportedOptions() in 
>>> AbstractPlainSocketImpl and AbstractPlainDatagramSocketImpl return 
>>> an immutable set of the socket options. In addition, we corrected 
>>> couple formatting issues.
>>>
>>> Please let us know your feedback and comments.
>>>
>> I've looked through the latest revision. Just a couple of small things:
>>
>> In MulticastSocket then a small typo (in two places) where you have 
>> "SO_REUSPORT" instead of "SO_REUSEPORT". Also it links to
>> setOption(int,Object) then I assume it should be 
>> setOption(SocketOption,Object).
>>
>> In AbstractPlainSocketImpl and AbstractPlainDatagramSocketImpl then 
>> supportedOptions looks technically correct but there is no need to 
>> create an unmodifiableSet on each call to supportedOptions. Instead 
>> you can simply do:
>>
>> Set<SocketOption<?>> options = socketOptions; if (options == null) {
>>       if (isReusePortAvailable()) {
>>           options = new HashSet<>();
>>           options.addAll(super.supportedOptions());
>>           options.add(StandardSocketOptions.SO_REUSEPORT);
>>           options = Collections.unmodifiableSet(options);
>>       } else {
>>           options = super.supportedOptions();
>>       }
>>       socketOptions = options;
>> }
>> return options;
>>
>> I don't see any other issues at this time and I'm happy to sponsor 
>> and help you get this in.
>>
>> -Alan.
>>
>>
>>