Bug ID: JDK-6362070 Random is not thread-safe

Type: Bug
Component: hotspot
Sub-Component: compiler
Affected Version: 1.4.2

Priority: P4
Status: Resolved
Resolution: Fixed
OS: linux
CPU: x86

Submitted: 2005-12-12
Updated: 2011-02-16
Resolved: 2006-06-26

Other
1.4.2_13 b01Fixed

FULL PRODUCT VERSION :
Both oss:
java version "1.4.2_08"
Java(TM) 2 Runtime Environment, Standard Edition (build 1.4.2_08-b03)
Java HotSpot(TM) Client VM (build 1.4.2_08-b03, mixed mode)


ADDITIONAL OS VERSION INFORMATION :
Linux mole 2.6.11.4-21.9-smp #1 SMP Fri Aug 19 11:58:59 UTC 2005 x86_64 x86_64 x86_64 GNU/Linux

SunOS sol10sqa 5.10 Generic i86pc i386 i86pc

A DESCRIPTION OF THE PROBLEM :
The method random is not thread safe as assured in the API doc.

I have just tested this bug with JDK 1.4.2_10 and discovered that the
bug is also present in the update 10. 

In Java 5 the bug doesn't exists anymore.

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
The class attached in the source code section of this bug clarifies this. Two threads are generating random numbers with Math.random(). When proving the set of randoms generated by each thread it occurs that they  have occasionally the same number in the set.


EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
No output by this class.
ACTUAL -
Double (459.9604747381139) already exists in the set!!!
Double (486.519625428948) already exists in the set!!!
Double (339.88893672717944) already exists in the set!!!
Double (409.237483031485) already exists in the set!!!
Double (693.3775776858432) already exists in the set!!!
Double (490.4033037331255) already exists in the set!!!
Double (129.6123926350977) already exists in the set!!!
Double (161.41201486200575) already exists in the set!!!
Double (268.77128077945014) already exists in the set!!!
Double (311.83956630726016) already exists in the set!!!
Double (301.8506694025317) already exists in the set!!!
Double (182.41583026971554) already exists in the set!!!
Double (463.02838547913615) already exists in the set!!!
[...]


REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------

import java.util.HashSet;
import java.util.Set;

/**
 * @author Jan Tietjens (jtietjen)
 */
public class MathRandomTest extends Thread {
  double [] randomNumbers;

  public MathRandomTest(int numberOfRandomNumbers) {
    randomNumbers = new double[numberOfRandomNumbers];
  }

  public void run() {
    synchronized(this){

      for (int i = 0; i < randomNumbers.length; i++) {
        //System.out.println("getName() = " + getName() + " generating RandomNumber");
        randomNumbers[i] = (1000 * Math.random());
      }
    }
  }

  private double[] getRandomNumbers() {
    return randomNumbers;
  }

  public static void main(String[] args) throws InterruptedException {
    while(true){

      int numberOfRandomNumbers = 1000;
      if (args.length > 0) {
        numberOfRandomNumbers = Integer.parseInt(args[0]);
      }

      MathRandomTest firstMathRandomTest = new MathRandomTest(numberOfRandomNumbers);
      firstMathRandomTest.setName("FirstMathRandomTest");
      firstMathRandomTest.start();

      MathRandomTest secondMathRandomTest = new MathRandomTest(numberOfRandomNumbers);
      secondMathRandomTest.setName("SecondMathRandomTest");
      secondMathRandomTest.start();

      firstMathRandomTest.join();
      secondMathRandomTest.join();

      Set set= new HashSet();
      addResultArrayToSet(firstMathRandomTest, set);
      addResultArrayToSet(secondMathRandomTest, set);
    }

  }

  private static void addResultArrayToSet(MathRandomTest firstMathRandomTest, Set set) {
    double[] randomNumbers = firstMathRandomTest.getRandomNumbers();
    for (int i = 0; i < randomNumbers.length; i++) {
      double randomNumber = randomNumbers[i];
      Double aDouble = new Double(randomNumber);
        if(!set.add(aDouble)){
          System.out.println("Double (" + aDouble+ ") already exists in the set!!!");
        };
    }
  }
}

---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
Build a synchronized statement around Math.random().

EVALUATION That's great: have built the above fix for Solaris x86 and Windows, both are running the testcase on multi-CPU AMD machines with no problems.
28-03-2006
EVALUATION The problem is a missing call to lock before the cmpxchg8. * /tmp/geta17255 Mon Mar 27 10:51:40 2006 --- c1_LIRAssembler_i486.cpp Mon Mar 27 10:51:39 2006 *********** * 1836,1841 **** --- 1836,1844 ---- assert(op->new_value()->as_register_lo() == ebx, "wrong register"); assert(op->new_value()->as_register_hi() == ecx, "wrong register"); Register addr = op->addr()->as_register(); + if (os::is_MP()) { + __ lock(); + } __ cmpxchg8(addr); } else { Apparently Intel chips preservse the atomicity without the lock but amd requires it. It seems silly to have an atomic compare exchange which isn't atomic in an mp environment but it's certainly speced as requiring the lock prefix. The code was changed in 1.5 as part of 4904703.
27-03-2006
EVALUATION If there is indeed a problem with sun.misc.AtomicLong, perhaps the underlying problem should be fixed instead and this bug redispatched, perhaps to hotspot/runtime?
15-12-2005
EVALUATION I could reproduce this on my dual cpu Win2K system. The problem occurs only with the client compiler. If the sun/misc/AtomicLongCSImpl.attemptUpdate method is excluded, the system works properly. Fix for CR 4476333 (1.4.0_01 and above) altered the way of synchornization by having sun.misc.AtomicLong. When the above method is optimized or inlined on a multi processor system, the problem occurs.
15-12-2005
WORK AROUND add the following line into .hotspot_compiler file exclude sun/misc/AtomicLongCSImpl attemptUpdate
15-12-2005
EVALUATION I believe the current (mustang) implementation of Random is thread-safe (it is certainly designed to be) and the problem only exists in legacy releases. (It would be good if someone could confirm this) If so, it is not worth fixing, in my opinion. I recommend closing as Will Not Fix, but the decision is up to the maintainers of 1.4.2.
12-12-2005