On 28 Sep 2016, at 20:02, Lu, Yingqi <yingqi...@intel.com
<mailto:yingqi...@intel.com>> wrote:
Hi Vyom,
Thank you very much checking with us.
We agree that SO_RESUEADDR and SO_REUSEPORT behave the same way for
MulticastSocket. There is no need to check and enable SO_REUSEPORT
for this particular type of socket. SO_REUSEADDR is sufficient.
Thanks,
Lucy
*From:*Vyom Tewari [mailto:vyom.tew...@oracle.com]
*Sent:*Wednesday, September 28, 2016 1:26 AM
*To:*Chris Hegarty <chris.hega...@oracle.com
<mailto:chris.hega...@oracle.com>>; Mark Sheppard
<mark.shepp...@oracle.com <mailto:mark.shepp...@oracle.com>>; net-dev
<net-dev@openjdk.java.net <mailto:net-dev@openjdk.java.net>>;
Kaczmarek, Eric <eric.kaczma...@intel.com
<mailto:eric.kaczma...@intel.com>>; Viswanathan, Sandhya
<sandhya.viswanat...@intel.com
<mailto:sandhya.viswanat...@intel.com>>; Kharbas, Kishor
<kishor.khar...@intel.com <mailto:kishor.khar...@intel.com>>; Aundhe,
Shirish <shirish.aun...@intel.com <mailto:shirish.aun...@intel.com>>;
Lu, Yingqi <yingqi...@intel.com <mailto:yingqi...@intel.com>>
*Subject:*Re: RFR 8153674: Expected SocketException not thrown when
calling bind() with setReuseAddress(false)
Hi All,
I had off line discussion here at Oracle and we decided not to
override getReuseAddr/setReuseAddr for MulticastSocket. If user
wants, he can set the SO_REUSEPORT with "setOption"before bind.
For MulticastSocketSO_REUSEADDR&SO_REUSEPORT are semantically
equivalent which means either option will be sufficient to indicate
that the address&port is reusable. So setting SO_REUSEPORT in
constructor is really necessary/required ?
I am looking some comments on this from Intel people(they are in mail
chain) who did this original change, before we decide how we wants to
proceed on this issue.
Thanks,
Vyom
On Wednesday 14 September 2016 08:47 PM, Chris Hegarty wrote:
On 14/09/16 15:53, Mark Sheppard wrote:
that's true wrt SO_REUSEPORT in MulticastSocket constructor.
But the
same could have been argued for the original
invocation of setReuseAddress, by default , in the
constructors, which
is encapsulating, what pereceived as, common or defacto
practice wrt applying SO_REUSEADDR on mcast sockets at the
system level.
As I understand it, it is generally perceived that
SO_REUSEADDR and
SO_REUSEPORT are semantically equivalent for multicast sockets.
As such, I think in the case of MulticastSocket, the fact
that the
setRuseAddress() is called in the constructor, it is appropriate
to set SO_REUSEPORT also when it exists in the OS.
I take your point on the semantics of setReuseAddress in
DatagramSocket
as per its spec. The spec does allude to MulticastSocket.
As such, the current proposal's changes just lack the appropriate
javadoc to describe its behavior, and its additional
functionality of
setting SO_REUSEPORT.
It is not necessary that overridden method should mirror the
semantics
of the base class method.
If it is accepted that it is generally perceived that
SO_REUSEADDR and
SO_REUSEPORT are semantically equivalent for multicast sockets,
then it seems appropriate that an overriding
setReuseAddress(..) method
in MulticastSocket can reflect this.
That sounds reasonable.
-Chris.
regards
Mark
On 14/09/2016 14:58, Chris Hegarty wrote:
One additional remark.
Was it appropriate to update the legacy MC constructors
to set the new JDK 9 SO_REUSEPORT in the first place?
This can be achievable, opt-in from new code, by creating
an unbound MS, setting the option, then binding.
-Chris.
On 14/09/16 14:47, Chris Hegarty wrote:
Mark,
On 14/09/16 14:22, Mark Sheppard wrote:
Hi Chris,
I don't fully understand your objections to
the approach taken.
Is there a compatibility issue with the addition
of the additional
methods to MulticastSocket?
The concern is with setReuseAddress performing an
operation that
is not clear from the method specification, e.g. from
setReuseAddress()
* Enable/disable the SO_REUSEADDR socket option.
This is no longer accurate. The proposed changes
would affect
SO_REUSEPORT too.
I don't see Datagram.setReuseAddress(...)
handling the SO_REUSEPORT
option, this has to be done explicitly via
setOption at this level of
abstraction.
Yes, it is a bit odd, but these are legacy classes. I
am not opposed
to adding a new method for this, or something else. I
just want to
avoid future issues and confusion when
setReuseAddress is called and
then it is noticed that, the somewhat orthogonal
option, SO_REUSEPORT's
value has changed. setReuseAddress's spec is very
clear about what it
does.
MulticastSocket is a subclass of DatagramSocket
(that in itself is a
questionable structuring), and as such
has specialized behaviour, and part of that
specialization is the
setting of the setting SO_REUSEADDR and SO_REUSEPORT
in its constructors, to enable address reuse
semantics, prior to
binding
an address.
Understood. Of course, the setting of SO_REUSEPORT is
new in 9,
hence the problem.
As part of that specialization, it would seem
appropriate that
MulticastSocket manipulate the SO_REUSEPORT
option in a consistent way. Adding an overridden
setReuseAddress(...)
method provides that consistency and
encapsulates the specialized behaviour.
I agree with the abstraction, just not that
setReuseAddress should
be used to achieve it. The name and spec of this
method is so
tied to SO_REUSEADDR.
Is alternatively proposal to NOT do anything to
MulticastSocket, BUT
document clearly how to handle the failing
scenario, that an
MulticastSocket
requires both setReuseAddress() and a setOption
call to disable
reuseaddress semantics on an unbound
MulticastSocket ?
That is one option, and the option that I was
suggesting as a possible
alternative.
This then raises the question of why have a
convenience method, such as
setReuseAddress() in the first place, when it can
be handled
adequately via the setOption
We are moving away from these option specific getter
and setter
methods, in favor of the more general get/setOption
methods, as
the latter are more adaptable.
If setReuseAddress is to operate on more than
SO_REUSEADDR, then
its spec should be very clear about this.
-Chris.
regards
Mark
On 14/09/2016 13:34, Chris Hegarty wrote:
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>
<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>
<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>
<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>
<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>
<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>
<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>
<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>
<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>
<http://cr.openjdk.java.net/%7Evtewari/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
