I got this code from a student, and it does not work properly because of a race condition involving x++ and x--. He added synchronized to the run() method trying to get rid of this bug, but obviously this only excludes threads from entering run() on the same object (which was never a problem in the first place) but doesn't prevent independent objects from updating the same static variable x at the same time.
public class DataRace implements Runnable {
static volatile int x;
public synchronized void run() {
for (int i = 0; i < 10000; i++) {
x++;
x--;
}
}
public static void main(String[] args) throws Exception {
Thread [] threads = new Thread[100];
for (int i = 0; i < threads.length; i++)
threads[i] = new Thread(new DataRace());
for (int i = 0; i < threads.length; i++)
threads[i].start();
for (int i = 0; i < threads.length; i++)
threads[i].join();
System.out.println(x); // x not always 0!
}
}
Since we cannot synchronize on x (because it is primitive), the best solution I can think of is to create a new static object like static String lock = ""; and enclose the x++ and x-- within a synchronized block, locking on lock. But this seems really awkward. Is there a better way?