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

[Lindenmaier, Goetz]

... sorry meant 'reviews' not 'downports' ...

> -----Original Message-----
> From: jdk-updates-dev <jdk-updates-dev-bounces at openjdk.java.net> On
> Behalf Of Lindenmaier, Goetz
> Sent: Sunday, March 15, 2020 10:16 PM
> To: Doerr, Martin <martin.doerr at sap.com>; jdk-updates-dev <jdk-updates-
> dev at openjdk.java.net>
> Subject: [CAUTION] RE: 11u: RFR[M]: 8240827: Downport SSLSocketImpl.java
> from "8221882: Use fiber-friendly java.util.concurrent.locks in JSSE"
> 
> Thanks!
> 
> Having two downports I now push this.
> Also, our testing is fine. The changes participated in several
> nightly build and test cycles.
> 
> Beset regards,
>   Goetz.
> 
> > -----Original Message-----
> > From: Doerr, Martin <martin.doerr at sap.com>
> > Sent: Friday, March 13, 2020 4:44 PM
> > To: Doerr, Martin <martin.doerr at sap.com>; Lindenmaier, Goetz
> > <goetz.lindenmaier at sap.com>; jdk-updates-dev <jdk-updates-
> > dev at openjdk.java.net>
> > Subject: RE: 11u: RFR[M]: 8240827: Downport SSLSocketImpl.java from
> > "8221882: Use fiber-friendly java.util.concurrent.locks in JSSE"
> >
> > Ah, you backport the part of [2] as a separate issue which is not flagged as
> > backport. This is fine with me.
> >
> > Best regards,
> > Martin
> >
> >
> > > -----Original Message-----
> > > From: jdk-updates-dev <jdk-updates-dev-bounces at openjdk.java.net>
> On
> > > Behalf Of Doerr, Martin
> > > Sent: Freitag, 13. März 2020 16:39
> > > To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; jdk-updates-dev
> > > <jdk-updates-dev at openjdk.java.net>
> > > Subject: RE: 11u: RFR[M]: 8240827: Downport SSLSocketImpl.java
> > > from "8221882: Use fiber-friendly java.util.concurrent.locks in JSSE"
> > >
> > > 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