RFR 8153674: Expected SocketException not thrown when calling bind() with setReuseAddress(false)
Vyom Tewari
vyom.tewari at oracle.com
Thu Sep 29 08:16:22 UTC 2016
Hi All,
Please find the latest
webrev(http://cr.openjdk.java.net/~vtewari/8153674/webrev0.1/index.html
<http://cr.openjdk.java.net/%7Evtewari/8153674/webrev0.1/index.html>). I
had removed the SO_REUSEPORT & spec from constructor.
Thanks,
Vyom
On Thursday 29 September 2016 01:38 AM, Chris Hegarty wrote:
> Thank you Lucy,
>
> We’ll proceed with removing the setting of SO_REUSEPORT from the
> MulticastSocket constructor and spec.
>
> -Chris.
>
>> On 28 Sep 2016, at 20:02, Lu, Yingqi <yingqi.lu at intel.com
>> <mailto:yingqi.lu at 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.tewari at oracle.com]
>> *Sent:*Wednesday, September 28, 2016 1:26 AM
>> *To:*Chris Hegarty <chris.hegarty at oracle.com
>> <mailto:chris.hegarty at oracle.com>>; Mark Sheppard
>> <mark.sheppard at oracle.com <mailto:mark.sheppard at oracle.com>>; net-dev
>> <net-dev at openjdk.java.net <mailto:net-dev at openjdk.java.net>>;
>> Kaczmarek, Eric <eric.kaczmarek at intel.com
>> <mailto:eric.kaczmarek at intel.com>>; Viswanathan, Sandhya
>> <sandhya.viswanathan at intel.com
>> <mailto:sandhya.viswanathan at intel.com>>; Kharbas, Kishor
>> <kishor.kharbas at intel.com <mailto:kishor.kharbas at intel.com>>; Aundhe,
>> Shirish <shirish.aundhe at intel.com <mailto:shirish.aundhe at intel.com>>;
>> Lu, Yingqi <yingqi.lu at intel.com <mailto:yingqi.lu at 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
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/net-dev/attachments/20160929/335d1656/attachment-0001.html>
More information about the net-dev
mailing list