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.

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


Reply via email to