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.
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.
-Alan