RFR 8153674: Expected SocketException not thrown when calling bind() with setReuseAddress(false)

[Chris Hegarty]

Mark,

On 14/09/16 13:23, Mark Sheppard wrote:
>
> Hi Chris,
>     so are you accepting that it is correct to add the overridden
> methods in MulticastSocket and that these need
> appropriate javadoc ?

I think we need these, but they should just call their super
equivalents, i.e. no implicit setting of SO_REUSEPORT. They would
exist solely as a place to locate guidance, or a note, that one
will likely want to set SO_REUSEPORT too.

> or are you advocating pushing the handing of the SO_REUSEPORT into the
> base DatagramSocket class ?

It is already there. I am not proposing to change this.

> It is not clear how your code changes fit in the proposed fix i.e. the
> explicit setting of the option to false?

My proposal is an alternative. It is not related to the current
webrev.

> With the current proposed changes then I think it would be sufficient to
> invoke setReuseAddress(true) in MulticastSocket constructors
> rather than
>
>         // Enable SO_REUSEADDR before binding
>         setReuseAddress
> <https://java.se.oracle.com/source/s?defs=setReuseAddress&project=jdk9-dev>(*true*);
>
>         // Enable SO_REUSEPORT if supported before binding
>         *if* (supportedOptions
> <https://java.se.oracle.com/source/xref/jdk9-dev/jdk/src/java.base/share/classes/java/net/MulticastSocket.java#supportedOptions>().contains
> <https://java.se.oracle.com/source/s?defs=contains&project=jdk9-dev>(StandardSocketOptions
> <https://java.se.oracle.com/source/s?defs=StandardSocketOptions&project=jdk9-dev>.SO_REUSEPORT
> <https://java.se.oracle.com/source/s?defs=SO_REUSEPORT&project=jdk9-dev>)) {
>             *this*.setOption
> <https://java.se.oracle.com/source/s?defs=setOption&project=jdk9-dev>(StandardSocketOptions
> <https://java.se.oracle.com/source/s?defs=StandardSocketOptions&project=jdk9-dev>.SO_REUSEPORT
> <https://java.se.oracle.com/source/s?defs=SO_REUSEPORT&project=jdk9-dev>, *true*);
>         }
>
>
> as the overridden setReuseAddress takes care of SO_REUSEPORT

Yes, this is what Vyom has proposed, in the webrev.

I would like to explore an alternative, so see what it would look like.

-Chris.


> regards
> Mark
>
> On 14/09/2016 11:43, Chris Hegarty wrote:
>> Vyom,
>>
>> On 11/09/16 08:01, Vyom Tewari wrote:
>>> Hi All,
>>>
>>> Please review the below code change.
>>>
>>> Bug        : https://bugs.openjdk.java.net/browse/JDK-8153674
>>>
>>> Webrev  :
>>> http://cr.openjdk.java.net/~vtewari/8153674/webrev0.0/index.html
>>> <http://cr.openjdk.java.net/%7Evtewari/8153674/webrev0.0/index.html>
>>>
>>> This change override the "get/setReuseAddress"  for MulticaseSocket and
>>> will abstract with both reuse attributes(SO_REUSEADDR & SO_REUSEPORT).
>>
>> This issue arises since the changes for 6432031 "Add support for
>> SO_REUSEPORT" [1], which sets SO_REUSEPORT on MulticastSocket, if
>> the available. So setting setReuseAddress(false) alone is no longer
>> sufficient to disable address/port reuse, one must also set
>> SO_REUSEPORT to false.
>>
>> I would be really nervous about changing set/getReuseAddress, without
>> at least updating the javadoc to indicate that it is now, for MS,
>> operating on two socket options.  Although, I do have sympathy
>> here since SO_REUSEPORT and SO_REUSEADDR are almost identical when
>> dealing with multicasting.
>>
>> An alternative, maybe; Since the MS constructors document that
>> SO_REUSEPORT will be set, where available, maybe a javadoc note
>> on the set/getReuseAddress methods would be sufficient, that
>> indicates that StandardSocketOptions#SO_REUSEPORT may also need
>> to be set to have the desired effect?
>>
>> If so, then code would have to:
>>
>>     setReuseAddress(false);
>>
>>     if (supportedOptions().contains(StandardSocketOptions.SO_REUSEPORT))
>>         this.setOption(StandardSocketOptions.SO_REUSEPORT, false);
>>
>>   , but at least it is explicit.
>>
>> Q: not all MS constructors document that SO_REUSEPORT is set, but
>> they should, right? This seems to have slipped past during 6432031 [1].
>>
>> -Chris.
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-6432031
>