RFR: 8274524: SSLSocket.close() hangs if it is called during the ssl handshake

Alexey Bakhtin abakhtin at openjdk.java.net
Fri Feb 11 09:30:08 UTC 2022


On Thu, 10 Feb 2022 23:59:40 GMT, Xue-Lei Andrew Fan <xuelei at openjdk.org> wrote:

>> Please review the patch for the JDK-8274524
>> 
>> SSLSocket.close() could cause an intermittent hang of the socket read operation. It happens in case of SO_TIMEOUT is set to 0 (infinite timeout).
>> SSLSocket.close() reads from the socket as part of the skip() operation to prevent TCP Connection reset (see JDK-8268965). Socket reads are performed in a loop for small chunks. These read operations could cause a deadlock, in case of SO_TIMEOUT = 0
>> I suggest to force non-zero SO_TIMEOUT during the skip() operation to prevent such deadlock
>> 
>> This is a second iteration of review. Previous PR was closed without integration: https://github.com/openjdk/jdk/pull/5760
>
> src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java line 1792:
> 
>> 1790:                     try {
>> 1791:                         if (soTimeout == 0)
>> 1792:                             setSoTimeout(DEFAULT_SKIP_TIMEOUT);
> 
> This set will impact the socket overall behavior unexpectedly, not just the close() method.
> 
> Maybe, an input stream level synchronization is missed in the SSLSocketInputRecord method, so that logic of deplete could be interrupted.

Hi @XueleiFan,
Not quite sure What could be wrong with setting SO_TIMEOUT during the socketClose() operation. We still close the socket and SO_TIMEOUT will be restored just after skip() operation is completed. These changes could affect the parallel read of the handshake records (we hold appInput.readLock during the skip) but we still close the socket and interrupt connection in this case.
If you still think these changes are incorrect, What do you think about the most first version of the patch: https://github.com/openjdk/jdk/pull/7432/commits/d1c2a4fe23916ff0f1c38150bc0ea1c9c38fe39b
This version adds a new lock in the SSLSocketInputRecord and protects the parallel execution of the read and skip operations.

-------------

PR: https://git.openjdk.java.net/jdk/pull/7432



More information about the security-dev mailing list