[RFR] 8184328: JDK8u131 socketRead0 hang at SSL read

[Xuelei Fan]

> On Sep 22, 2017, at 6:06 AM, Rob McKenna <rob.mckenna at oracle.com> 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 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 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 it's fine just call fatal() for the
>> first timeout.
>> 
>> Xuelei
>> 
>>> On 9/15/2017 4:32 PM, Xuelei Fan wrote:
>>>> 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() unless I'm mistaken.
>>>> 
>>> I did not find the call to fatal() in the current implementation.  I think
>>> you mean you added the call to fatal() in your update so that when
>>> timeout, a fatal() will always get called?
>>> 
>>> Thinking about two things:
>>> 1. application have to set receiving timeout in order to  get receiving
>>> timeout.
>>> I have a concern about it, as described in other comments.
>>> 
>>> 2. can we close the super socket?
>>> It is a surprise to me to close super socket even we don't allocate it. It
>>> does not feel right to me, but this is the current behavior.  All right, I
>>> get your point.
>>> 
>>> Xuelei
>>> 
>>>>      -Rob
>>>> 
>>>>> On 15/09/17 07:55, Xuelei Fan wrote:
>>>>>> 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 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)
>>>>>> 
>>>>> It is just one possible stacks of many.  There are cases where no
>>>>> fatal()
>>>>> get called.  For example, application call close() method directly.
>>>>> 
>>>>> Xuelei