Locking/Singleton in JCAUtil

Bernd Eckenfels ecki at zusammenkunft.net
Fri May 23 00:34:13 UTC 2014


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


More information about the security-dev mailing list