There's a significant difference here: Random reads the field into a local
and then operates only on the local. Looking at the code, I only see one
possible (bizarre) circumstance where you can hit NPE. If code was
transformed to:
static double random() {
Random rnd = randomNumberGenerator; // assume null is read
if (randomNumberGenerator == null) ... // note the field is re-read
instead of local, and assume it's not null now
return rnd.nextDouble(); // NPE
}
I don't see a case where any sane compiler would do this but I think it's
legal transformation given that the field is non-volatile and single
threaded execution would not detect difference. Unless phantom reads
cannot be introduced even for plain fields, in which case I don't see how
NPE can occur.
Having said that, David's suggestion is cleaner anyway.
Sent from my phone
On Aug 21, 2013 8:36 PM, "Steven Schlansker" <stevenschlans...@gmail.com>
wrote:
>
> On Aug 21, 2013, at 4:37 PM, Brian Burkhalter <brian.burkhal...@oracle.com>
> wrote:
>
> > With respect to this issue
> >
> > http://bugs.sun.com/view_bug.do?bug_id=6470700
> >
> > the code of concern from java.lang.Math is
> >
> > 701 private static Random randomNumberGenerator;
> > 702
> > 703 private static synchronized Random initRNG() {
> > 704 Random rnd = randomNumberGenerator;
> > 705 return (rnd == null) ? (randomNumberGenerator = new Random())
> : rnd;
> > 706 }
> >
> > 731 public static double random() {
> > 732 Random rnd = randomNumberGenerator;
> > 733 if (rnd == null) rnd = initRNG();
> > 734 return rnd.nextDouble();
> > 735 }
> >
> > where the class variable "randomNumberGenerator" is not used anywhere
> else in the class. The complaint is that this is an instance of the broken
> double-checked locking pattern. While at first glance this might appear to
> be the case, it does not seem so to me. It looks more like an attempt to
> avoid calling a synchronized method if "randomNumberGenerator" has already
> been initialized.
>
>
> This looks very much like the "Broken multithreaded version" from
> http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html
>
> // Broken multithreaded version
> // "Double-Checked Locking" idiom
> class Foo {
> private Helper helper = null;
> public Helper getHelper() {
> if (helper == null)
> synchronized(this) {
> if (helper == null)
> helper = new Helper();
> }
> return helper;
> }
> // other functions and members...
> }
>
>
> "Unfortunately, that code just does not work in the presence of either
> optimizing compilers or shared memory multiprocessors."
>
>
