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