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
