Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-21 Thread Xuelei Fan
> On Sep 22, 2017, at 6:06 AM, Rob McKenna wrote: > > Thanks Xuelei, webrev below: > > http://cr.openjdk.java.net/~robm/8184328/webrev.02/ > Looks fine to me. > Presumably this won't require a CSR? > Agreed. Xuelei >-Rob > >> On 15/09/17 04:38, Xuelei Fan

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-21 Thread Rob McKenna
Thanks Xuelei, webrev below: http://cr.openjdk.java.net/~robm/8184328/webrev.02/ Presumably this won't require a CSR? -Rob On 15/09/17 04:38, Xuelei Fan wrote: > I still prefer to not-depends on socket receiving timeout. But I'm fine if > you want to move on with it. > > As we can close

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-15 Thread Xuelei Fan
I still prefer to not-depends on socket receiving timeout. But I'm fine if you want to move on with it. As we can close the super socket in the current implementation, it implies that application can handle it already. So you may not need the system property and 5 times retries. I think

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-15 Thread Xuelei Fan
On 9/15/2017 8:22 AM, Rob McKenna wrote: This test calls close directly. (3rd last line in the stack) I believe this is the only possible stack (with the new parameter) once autoclose is set to false. If autoclose is true we'd skip the call to waitForClose and just go directly to Socket.close()

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-15 Thread Rob McKenna
This test calls close directly. (3rd last line in the stack) I believe this is the only possible stack (with the new parameter) once autoclose is set to false. If autoclose is true we'd skip the call to waitForClose and just go directly to Socket.close() unless I'm mistaken. -Rob On

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-15 Thread Rob McKenna
Ah, right! This is the part I was missing. So my fix is intended to address this specific circumstance only (where we get caught in the while loop in waitForClose() indefinitely despite having set a read timeout). In this situation it would be reasonable for somebody to set a read timeout in the

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-15 Thread Xuelei Fan
On 9/15/2017 7:44 AM, Rob McKenna wrote: Perhaps I'm misunderstanding you here. Can you illustrate this a bit further? The basic point is simple: removing the closing blocking even receiving timeout is not set. Applications already have to set a read timeout I did not get the point.

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-15 Thread Xuelei Fan
On 9/15/2017 7:41 AM, Rob McKenna wrote: On 15/09/17 07:07, Xuelei Fan wrote: On 9/15/2017 7:00 AM, Rob McKenna wrote: When we call close() on the SSLSocket that calls close() on the underlying java Socket which closes the native socket. Sorry, I did not get the point. Please see the

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-15 Thread Rob McKenna
On 15/09/17 07:32, Xuelei Fan wrote: > On 9/15/2017 7:16 AM, Rob McKenna wrote: > >On 13/09/17 03:52, Xuelei Fan wrote: > >> > >> > >>On 9/13/2017 8:52 AM, Rob McKenna wrote: > >>>Hi Xuelei, > >>> > >>>This behaviour is already exposed via the autoclose boolean in: > >>> >

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-15 Thread Rob McKenna
Perhaps I'm misunderstanding you here. Can you illustrate this a bit further? Applications already have to set a read timeout, my proposal doesn't alter this fact. (i.e. if the read timeout isn't set applications which call close could potentially get stuck in readReply indefinitely) -Rob

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-15 Thread Rob McKenna
On 15/09/17 07:07, Xuelei Fan wrote: > On 9/15/2017 7:00 AM, Rob McKenna wrote: > >When we call close() on the SSLSocket that calls close() on the > >underlying java Socket which closes the native socket. > > > Sorry, I did not get the point. Please see the close() implementation of > SSLSocket

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-15 Thread Xuelei Fan
On 9/15/2017 7:16 AM, Rob McKenna wrote: On 13/09/17 03:52, Xuelei Fan wrote: On 9/13/2017 8:52 AM, Rob McKenna wrote: Hi Xuelei, This behaviour is already exposed via the autoclose boolean in:

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-15 Thread Xuelei Fan
On 9/15/2017 7:07 AM, Rob McKenna wrote: But they are inextricably linked regardless. When we close an SSLSocket it performs a readReply which is subject to the read timeout. So if no read timeout is specified, the call to readReply will hang indefinitely. That's one of what I worried about.

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-15 Thread Rob McKenna
On 13/09/17 03:52, Xuelei Fan wrote: > > > On 9/13/2017 8:52 AM, Rob McKenna wrote: > >Hi Xuelei, > > > >This behaviour is already exposed via the autoclose boolean in: > > >

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-15 Thread Rob McKenna
But they are inextricably linked regardless. When we close an SSLSocket it performs a readReply which is subject to the read timeout. So if no read timeout is specified, the call to readReply will hang indefinitely. If a read timeout is specified we would need to maintain two separate timeouts

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-15 Thread Xuelei Fan
On 9/15/2017 7:00 AM, Rob McKenna wrote: When we call close() on the SSLSocket that calls close() on the underlying java Socket which closes the native socket. Sorry, I did not get the point. Please see the close() implementation of SSLSocket (sun.security.ssl.SSLSocketImpl.close()) about the

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-15 Thread Rob McKenna
When we call close() on the SSLSocket that calls close() on the underlying java Socket which closes the native socket. -Rob On 13/09/17 04:09, Xuelei Fan wrote: > It's a little bit complicated for layered SSL connections. Application can > build a SSL connection on existing socket (we call

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-13 Thread Xuelei Fan
It's a little bit complicated for layered SSL connections. Application can build a SSL connection on existing socket (we call it layered SSL connections). The problem scenarios make look like: 1. open a socket for applications. 2. established a SSL connection on the existing socket. 3. close

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-13 Thread Xuelei Fan
On 9/13/2017 8:52 AM, Rob McKenna wrote: W.r.t. a separate timeout, the underlying mechanics of a close already depend on the readTimeout in this situation. That's a concerns of mine. In order to work for your countermeasure, applications have to set receiving timeout, and take care of the

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-13 Thread Xuelei Fan
On 9/13/2017 8:52 AM, Rob McKenna wrote: Hi Xuelei, This behaviour is already exposed via the autoclose boolean in: https://docs.oracle.com/javase/8/docs/api/javax/net/ssl/SSLSocketFactory.html#createSocket-java.net.Socket-java.io.InputStream-boolean- I did not get the point. What do you

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-13 Thread Chris Hegarty
Xuelei, Without diving deeper into this issue, Rob’s suggested approach seems reasonable to me, and better than existing out-of-the-box behaviour. I’m not sure what issues you are thinking of, with using the read timeout in combination with a retry mechanism, in this manner? If the network is

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-13 Thread Rob McKenna
Hi Xuelei, This behaviour is already exposed via the autoclose boolean in: https://docs.oracle.com/javase/8/docs/api/javax/net/ssl/SSLSocketFactory.html#createSocket-java.net.Socket-java.io.InputStream-boolean- My position would be that allowing 5 retries allows us to say with some confidence

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-13 Thread Xuelei Fan
On 9/13/2017 5:52 AM, Xuelei Fan wrote: In theory, there are intermittent compatibility problems as this update may not close the SSL connection over the existing socket layer gracefully, even for high speed networking environments, while the underlying socket is alive.  The impact could be

Re: [RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

2017-09-13 Thread Xuelei Fan
In theory, there are intermittent compatibility problems as this update may not close the SSL connection over the existing socket layer gracefully, even for high speed networking environments, while the underlying socket is alive. The impact could be serious in some environment. For safe, I