11u: RFR[M]: 8240827: Downport SSLSocketImpl.java from "8221882: Use fiber-friendly java.util.concurrent.locks in JSSE"

[Doerr, Martin]

Hi Götz,

I guess you don't want to backport the whole [2] because there are too many conflicts?

Technically, your partial backport of [2] looks good.

Are you planning to merge it with [3] before pushing?
I think [2] should not be flagged as backported when you only pick a subset of it.

Best regards,
Martin


> -----Original Message-----
> From: jdk-updates-dev <jdk-updates-dev-bounces at openjdk.java.net> On
> Behalf Of Lindenmaier, Goetz
> Sent: Dienstag, 10. März 2020 17:02
> To: jdk-updates-dev <jdk-updates-dev at openjdk.java.net>
> Subject: 11u: RFR[M]: 8240827: Downport SSLSocketImpl.java
> from "8221882: Use fiber-friendly java.util.concurrent.locks in JSSE"
> 
> Hi,
> 
> I started downporting [1], "8209333: Socket reset issue for TLS 1.3 socket
> close"
> for parity with 11.0.8-oracle.
> 
> Severin pointed me to [3], "8219991: New fix of the deadlock in
> sun.security.ssl.SSLSocketImpl", a follow-up that fixes an issue
> introduced by [1]. I agree that it would make sense
> to bring this fix to jdk11u-dev along with [1].
> 
> [1] applies clean to jdk11u-dev.
> Unfortunately, [3] does not apply at all.
> 
> In jdk13 a major rework of JSSE was done:
> [2] "8221882: Use fiber-friendly java.util.concurrent.locks in JSSE"
> 
> [2] removes the synchronized keywords from a lot of functions
> in JSSE and replaces them by manual locking. [3] exploits
> the new control flow within these reworked functions in
> SSLSocketImpl.java. It needs to be designed differently
> to apply directly on top of [1].
> 
> Instead, I propose to downport the changes of [2] to the only
> file touched by [3], SSLSocketImpl.java.
> 
> I took the patch of [2] for SSLSocketImpl.java and patched
> it on top of [1] to jdk11u-dev. It applies clean. On top
> of this, [3] applies clean, too.
> 
> The SSLSocketImpl.java part of [2] uses a public lock
> introduced in OutputRecord. I revoked this part of the change
> as the changes to OutputRecord are not important here, so
> it'd like to skip them. Here the partial webrev of the
> revoked code:
> http://cr.openjdk.java.net/~goetz/wr20/8240827-
> Downport_SSLSocketImpl_from_8221882-jdk11/01-
> revoked/src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java.u
> diff.html
> 
> The locks introduced by [2] in SSLSocketImpl.java are private,
> so there is no direct dependency to them in other code.
> 
> I ran the three changes through our nightly testing
> and saw no issues.
> 
> Please review:
> http://cr.openjdk.java.net/~goetz/wr20/8240827-
> Downport_SSLSocketImpl_from_8221882-jdk11/01/
> 
> Best regards,
>   Goetz.
> 
> [1] "8209333: Socket reset issue for TLS 1.3 socket close"
> https://bugs.openjdk.java.net/browse/JDK-8209333
> http://hg.openjdk.java.net/jdk/jdk12/rev/8a61a04c456c
> 
> [2] "8221882: Use fiber-friendly java.util.concurrent.locks in JSSE"
> https://bugs.openjdk.java.net/browse/JDK-8221882
> http://hg.openjdk.java.net/jdk/jdk/rev/dfba4e321ab3
> 
> [3] "8219991: New fix of the deadlock in sun.security.ssl.SSLSocketImpl"
> https://bugs.openjdk.java.net/browse/JDK-8219991
> http://hg.openjdk.java.net/jdk/jdk/rev/ca251ef47e0b