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

[Vyom Tewari]

Hi All,

Please find the latest 
webrev(http://cr.openjdk.java.net/~vtewari/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 > 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 >; Mark Sheppard 
mailto:mark.shepp...@oracle.com>>; net-dev 
mailto:net-dev@openjdk.java.net>>; 
Kaczmarek, Eric >; Viswanathan, Sandhya 
>; Kharbas, Kishor 
mailto:kishor.khar...@intel.com>>; Aundhe, 
Shirish mailto:shirish.aun...@intel.com>>; 
Lu, Yingqi 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(...)
   

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

[Chris Hegarty]

> On 29 Sep 2016, at 09:16, Vyom Tewari  wrote:
> 
> Hi All,
> 
> Please find the latest 
> webrev(http://cr.openjdk.java.net/~vtewari/8153674/webrev0.1/index.html). I 
> had removed the SO_REUSEPORT & spec from constructor.

Thank you Vyom, this looks good to me.

-Chris.

> 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  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 ; Mark Sheppard 
>>> ; net-dev ; Kaczmarek, 
>>> Eric ; Viswanathan, Sandhya 
>>> ; Kharbas, Kishor 
>>> ; Aundhe, Shirish ; Lu, 
>>> Yingqi 
>>> 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 MulticastSocket SO_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 optio

Re: RFR(S): 8166850: No runtime error expected after calling NET_MapSocketOption

[Chris Hegarty]

On 28 Sep 2016, at 16:39, Langer, Christoph  wrote:
> 
> Hi,
>  
> during my work on exception sites in the JDK, I spotted another minor place 
> that should be fixed. In the Windows native implementations of 
> DualStackPlainDatagramSocketImpl and DualStackPlainSocketImpl, there are 
> calls to evaluate OS API errors after unsuccessful return of 
> NET_MapSocketOption where this should not be done. Please review my fix. I 
> also took the opportunity to clean up some indentations.
>  
> Bug: https://bugs.openjdk.java.net/browse/JDK-8166850
> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8166850.0/

The changes, and cleanup, look fine.

-Chris.

RE: RFR(S): 8166850: No runtime error expected after calling NET_MapSocketOption

[Langer, Christoph]

Pushed: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/153b4781adcc

Thanks, Chris, for reviewing.

> -Original Message-
> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> Sent: Donnerstag, 29. September 2016 13:40
> To: Langer, Christoph 
> Cc: net-dev@openjdk.java.net
> Subject: Re: RFR(S): 8166850: No runtime error expected after calling
> NET_MapSocketOption
> 
> On 28 Sep 2016, at 16:39, Langer, Christoph 
> wrote:
> >
> > Hi,
> >
> > during my work on exception sites in the JDK, I spotted another minor place
> that should be fixed. In the Windows native implementations of
> DualStackPlainDatagramSocketImpl and DualStackPlainSocketImpl, there are
> calls to evaluate OS API errors after unsuccessful return of
> NET_MapSocketOption where this should not be done. Please review my fix. I
> also took the opportunity to clean up some indentations.
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8166850
> > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8166850.0/
> 
> The changes, and cleanup, look fine.
> 
> -Chris.