Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

2016-09-14 Thread Xuelei Fan
On 9/15/2016 9:45 AM, Artem Smotrakov wrote: Well, in this particular case it's not clear that it has the same issue with free port (at least to me). The exception occurred on client side, so it's not the case where we don't know where the handshake came from. ;-) Yeh, you catch the point. But

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

2016-09-14 Thread Artem Smotrakov
Well, in this particular case it's not clear that it has the same issue with free port (at least to me). The exception occurred on client side, so it's not the case where we don't know where the handshake came from. I can make this enhancement, but like I said I don't think it's going to

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

2016-09-14 Thread Xuelei Fan
On 9/15/2016 9:23 AM, Artem Smotrakov wrote: Hi Xuelei, For this one, I am not sure that it would help here since the test failed after it already had started handshaking. It has the same issue as a free-port is used. We don't actually know the handshake is coming from the right client.

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

2016-09-14 Thread Artem Smotrakov
Hi Xuelei, For this one, I am not sure that it would help here since the test failed after it already had started handshaking. I would prefer to have it as a separate enhancement. Artem On 09/14/2016 06:19 PM, Xuelei Fan wrote: As you were already there, I would suggest to consider the

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

2016-09-14 Thread Xuelei Fan
As you were already there, I would suggest to consider the SSLSocketSample.java template as the comment in JDK-8163924 review thread. Thanks, Xuelei On 9/15/2016 9:13 AM, Artem Smotrakov wrote: Not urgent, but I would appreciate if someone can get a chance to look at this. Artem On

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

2016-09-14 Thread Artem Smotrakov
Not urgent, but I would appreciate if someone can get a chance to look at this. Artem On 09/07/2016 03:17 PM, Artem Smotrakov wrote: Sending to net-dev@openjdk.java.net as well. Artem On 09/07/2016 12:28 PM, Artem Smotrakov wrote: Hello, Please review the following patch for

RFR - 8165988: Test JarURLConnectionUseCaches.java fails at windows: failed to clean up files after test

2016-09-14 Thread Rob McKenna
Hi folks, A resource cleanup issue can cause this test to fail on windows, fix is to run in othervm: http://cr.openjdk.java.net/~robm/8165988/webrev.01/ -Rob

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

2016-09-14 Thread Chris Hegarty
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

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

2016-09-14 Thread Mark Sheppard
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

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

2016-09-14 Thread Chris Hegarty
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,

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

2016-09-14 Thread Chris Hegarty
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

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

2016-09-14 Thread Mark Sheppard
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? I don't see Datagram.setReuseAddress(...) handling the SO_REUSEPORT option, this has to be done explicitly via

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

2016-09-14 Thread 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

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

2016-09-14 Thread Mark Sheppard
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

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

2016-09-14 Thread Chris Hegarty
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