RFR: 8263743: redundant lock in SSLSocketImpl

[Xue-Lei Andrew Fan]

Remove redundant lock in SSLSocketImpl.

In the SSLSocketImpl, there is a socket level lock while reading application data (see readApplicationRecord).

                socketLock.lock();
                try {
                    plainText = decode(buffer);
                } finally {
                    socketLock.unlock();
                }
If an application data read is in progress, other calling to SSLSocket APIs (for example getUseClientMode()) could be blocked if socket level locks are used.

No new regression test.  Simple fix, hard to trigger the deadlock.

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

Commit messages:
 - 8263743: redundant lock in SSLSocketImpl

Changes: https://git.openjdk.java.net/jdk/pull/3053/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3053&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8263743
  Stats: 7 lines in 1 file changed: 0 ins; 6 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3053.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3053/head:pull/3053

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

[Xue-Lei Andrew Fan]

On Wed, 17 Mar 2021 17:23:03 GMT, Xue-Lei Andrew Fan <[hidden email]> wrote:

> Remove redundant lock in SSLSocketImpl.
>
> In the SSLSocketImpl, there is a socket level lock while reading application data (see readApplicationRecord).
>
>                 socketLock.lock();
>                 try {
>                     plainText = decode(buffer);
>                 } finally {
>                     socketLock.unlock();
>                 }
> If an application data read is in progress, other calling to SSLSocket APIs (for example getUseClientMode() in a handshake complete listener) could be blocked if socket level locks are used.
>
> No new regression test.  Simple fix, hard to trigger the deadlock.

ping ...

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

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

[Jamil Nimeh-2]

On Wed, 17 Mar 2021 17:23:03 GMT, Xue-Lei Andrew Fan <[hidden email]> wrote:

> Remove redundant lock in SSLSocketImpl.
>
> In the SSLSocketImpl, there is a socket level lock while reading application data (see readApplicationRecord).
>
>                 socketLock.lock();
>                 try {
>                     plainText = decode(buffer);
>                 } finally {
>                     socketLock.unlock();
>                 }
> If an application data read is in progress, other calling to SSLSocket APIs (for example getUseClientMode() in a handshake complete listener) could be blocked if socket level locks are used.
>
> No new regression test.  Simple fix, hard to trigger the deadlock.

After reading over the description in the bug I think this looks fine.

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

Marked as reviewed by jnimeh (Reviewer).

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

[Xue-Lei Andrew Fan]

On Wed, 17 Mar 2021 17:23:03 GMT, Xue-Lei Andrew Fan <[hidden email]> wrote:

> Remove redundant lock in SSLSocketImpl.
>
> In the SSLSocketImpl, there is a socket level lock while reading application data (see readApplicationRecord).
>
>                 socketLock.lock();
>                 try {
>                     plainText = decode(buffer);
>                 } finally {
>                     socketLock.unlock();
>                 }
> If an application data read is in progress, other calling to SSLSocket APIs (for example getUseClientMode() in a handshake complete listener) could be blocked if socket level locks are used.
>
> No new regression test.  Simple fix, hard to trigger the deadlock.

This pull request has now been integrated.

Changeset: a678a38d
Author:    Xue-Lei Andrew Fan <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/a678a38d
Stats:     7 lines in 1 file changed: 0 ins; 6 del; 1 mod

8263743: redundant lock in SSLSocketImpl

Reviewed-by: jnimeh

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

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