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 (sun.security.ssl.SSLSocketImpl.close()) about the details.
Running my original test against an instrumented 8u-dev produces the following: java.lang.Exception: Stack trace at java.lang.Thread.dumpStack(Thread.java:1336) at java.net.Socket.close(Socket.java:1491) at sun.security.ssl.BaseSSLSocketImpl.close(BaseSSLSocketImpl.java:624) at sun.security.ssl.SSLSocketImpl.closeSocket(SSLSocketImpl.java:1579) at sun.security.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1980) at sun.security.ssl.SSLSocketImpl.waitForClose(SSLSocketImpl.java:1793) at sun.security.ssl.SSLSocketImpl.closeSocket(SSLSocketImpl.java:1592) at sun.security.ssl.SSLSocketImpl.closeInternal(SSLSocketImpl.java:1726) at sun.security.ssl.SSLSocketImpl.close(SSLSocketImpl.java:1615) at ssl.SSLClient.close(SSLClient.java:143) at ssl.SocketTimeoutCloseHang.ReadHang.testSSLServer(ReadHang.java:77) -Rob > > Xuelei > > > -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 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 the SSL connection, but leaving data in the socket. > >>4. establish another SSL connection on the socket, as the existing data in > >>the socket, the connection cannot be established. > >>5. establish another app connection on the socket, as the existing data in > >>the socket, the connection cannot be established. > >>.... > >> > >>Timeout happens even on very high speed network. If a timeout happens and > >>the SSL connection is not closed gracefully, and then the following > >>applications breaks. IMHO, we need to take care of the case. > >> > >>Xuelei > >> > >>On 9/13/2017 1:06 PM, Chris Hegarty wrote: > >>>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 so > >>>slow, surely there will be other issues with connecting and reading, why > >>>is closing any different. > >>> > >>>-Chris. > >>> > >>>>On 13 Sep 2017, at 16:52, Rob McKenna <rob.mcke...@oracle.com> 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- > >>>> > >>>>My position would be that allowing 5 retries allows us to say with some > >>>>confidence that we're not going to get a close_notify from the server. > >>>>If this is the case I think its reasonable to close the connection. > >>>> > >>>>W.r.t. a separate timeout, the underlying mechanics of a close already > >>>>depend on the readTimeout in this situation. (waiting on a close_notify > >>>>requires performing a read so the read timeout makes sense in this > >>>>context) I'm happy to alter that but I think that the combination of > >>>>a timeout and a retry count is straightforward and lower impact. > >>>> > >>>>In my opinion the default behaviour of potentially hanging indefinitely > >>>>is worse than the alternative here. (bearing in mind that we are closing > >>>>the underlying socket) > >>>> > >>>>I'll file a CSR as soon as we settle on the direction this fix will > >>>>take. > >>>> > >>>> -Rob > >>>> > >>>>On 13/09/17 05:52, 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 serious in some environment. > >>>>> > >>>>>For safe, I may suggest turn this countermeasure off by default. And > >>>>>providing options to turn on this countermeasure: > >>>>>1. Close the SSL connection gracefully by default; or > >>>>>2. Close the SSL connection after a timeout. > >>>>> > >>>>>It's hardly to say 5 times receiving timeout is better/safer than timeout > >>>>>once in this context. As you have already had a system property to > >>>>>control, > >>>>>you may be able to use options other than the customized socket receiving > >>>>>timeout, so that the closing timeout is not mixed/confused/dependent > >>>>>on/with > >>>>>the receiving timeout. > >>>>> > >>>>>Put all together: > >>>>>1. define a closing timeout, for example "jdk.tls.waitForClose". > >>>>>2. the property default value is zero, no behavior changes. > >>>>>3. applications can set positive milliseconds value for the property. The > >>>>>SSL connection will be closed in the set milliseconds (or about the > >>>>>maximum > >>>>>value between SO_TIMEOUT and closing timeout), the connection is not > >>>>>grant > >>>>>to be gracefully. > >>>>> > >>>>>What do you think? > >>>>> > >>>>>BTW, please file a CSR as this update is introducing an external system > >>>>>property. > >>>>> > >>>>>Thanks, > >>>>>Xuelei > >>>>> > >>>>>On 9/11/2017 3:29 PM, Rob McKenna wrote: > >>>>>>Hi folks, > >>>>>> > >>>>>>In high latency environments a client SSLSocket with autoClose set to > >>>>>>false > >>>>>>can hang indefinitely if it does not correctly recieve a close_notify > >>>>>>from the server. > >>>>>> > >>>>>>In order to rectify this situation I would like to suggest that we > >>>>>>implement an integer JDK property (jdk.tls.closeRetries) which instructs > >>>>>>waitForClose to attempt the close no more times than the value of the > >>>>>>property. I would also suggest that 5 is a reasonable default. > >>>>>> > >>>>>>Note: each attempt times out based on the value of > >>>>>>Socket.setSoTimeout(int timeout). > >>>>>> > >>>>>>Also, the behaviour here is similar to that of waitForClose() when > >>>>>>autoClose is set to true, less the retries. > >>>>>> > >>>>>>http://cr.openjdk.java.net/~robm/8184328/webrev.01/ > >>>>>> > >>>>>> -Rob > >>>>>> > >>>