Hello,
by browsing the source code I run across the JCAUtil class. It is
(among other stuff) responsible for providing a SecureRandom singleton.
The code looks a bit strange.
First of all, it defines a LOCK object, but instead of using an
unreachable instancde (which is a common pattern for those kind of LOCK
objects) it uses the public class itself:
private static final Object LOCK = JCAUtil.class;
Typical this would be a problem as I can lock up the class. In this
specific case the LOCK is only used in one place, and there it is used
for a double checked locking, which is I guess good as it only checks
the monitor before any user code can lock it. Nevertheless, I would
recommend to change this to a more common pattern (or remove the field
and us synchronized(JCAUtil.class) to make it more explicite.
Another option would be to get rid of all the volatile/lock/DCL by
having a static initialisation. If this is not possible for dependency
reasons, I would have expected a comment like "this needs to be lazy
because..."
With final and without volatile it also looks more predictable:
private static final SecureRandom secureRandom = new SecureRandom();
public static SecureRandom getSecureRandom() { return secureRandom; }
WDYT?
Gruss
Bernd
