Hi Chris,
so are you accepting that it is correct to add the overridden
methods in MulticastSocket and that these need
appropriate javadoc ?
or are you advocating pushing the handing of the SO_REUSEPORT into the
base DatagramSocket class ?
It is not clear how your code changes fit in the proposed fix i.e. the
explicit setting of the option to false?
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
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
