RFR [13] JDK-8221882: Use fiber-friendly java.util.concurrent.locks in JSSE
Xuelei Fan
xuelei.fan at oracle.com
Wed Apr 3 18:40:27 UTC 2019
Updated webrev (updated SSLContext.java, HttpsClient,
HttpsURLConnectionImpl):
http://cr.openjdk.java.net/~xuelei/8221882/webrev.01/
On 4/3/2019 1:03 AM, Alan Bateman wrote:
> On 03/04/2019 04:12, Xuelei Fan wrote:
>> Hi,
>>
>> Could I get the following update reviewed?
>> http://cr.openjdk.java.net/~xuelei/8221882/webrev.00/
>>
>> To benefits from with Fibers [1], there is a need to use explicit
>> locks, java.util.concurrent.locks, for synchronization in JSSE and the
>> SunJSSE provider.
>>
>> Most of the update is replacing synchronized blocks with
>> java.util.concurrent.locks.ReentrantLock locking/unlocking.
> I looked through the changes and don't see anything obviously wrong.
> When you say "Most of the updates ..." then does it mean they are some
> changes that aren't equivalent? It would be useful to call those out.
Other than the straightforward replacing 'synchronized' with explicit
lock, there are some code clean-up:
1. Replacing 'synchronized' blocks with lazy initialization holder for
static fields, for SSLServerSocketFactory.getDefault() and
SSLSocketFactory.getDefault().
2. Removing the 'synchronized' modifier that is not really necessary,
for BaseSSLSocketImpl.setSoTimeout(), DTLSInputRecord.close(),
HttpsURLConnectionImpl.getIn/OutputStream().
3. Removing the 'synchronized' modifier by re-org to use final filed,
for SunX509KeyManagerImpl#X509Credentials.getIssuerX500Principals().
4. Removing the the 'synchronized' modifier by re-org to use volatile
field, for SSLContext.s/getDefault().
> One other about switching from monitors to explicit locks is calling out
> any classes that extend classes that use synchronization and assume that
> sub-classes do the same.
>
> In passed I see a few places where the locking might be excessive, e.g.
> I assume SSLContext.defaultContext could be a volatile so that
> getDefault just needs to return it when it's not null. I'm not
> suggesting you do those changes now but looking at the locking suggests
> there may be a few places that could be improved.
>
As we are already here, I would like to make the improvement for
SSLContext. I believe there are also some other improvement we could do.
I make more evaluation later.
Thanks,
Xuelei
More information about the security-dev
mailing list