CODEWITHSUNDEEP
javamultithreadingrace-condition
Fixee
Fixee

Reputation: 1639

How do I make this Java code operate properly? [Multi-threaded, race condition]

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?

Upvotes: 3

Views: 230

Answers (3)

Alexei Kaigorodov
Alexei Kaigorodov

Reputation: 13525

The variable x is static and reside in a class, so access to it should be synchronized on that class: either make a static method, or use synchronized block on DataRace.class.

Upvotes: 1

Daniel Miladinov
Daniel Miladinov

Reputation: 1592

Using AtomicInteger does what you want, and it makes explicit the intent to have operations on x be atomic. After quite a few runs of the following, I got 0's every time:

import java.util.concurrent.atomic.AtomicInteger;

public class DataRace implements Runnable {
    static volatile AtomicInteger x = new AtomicInteger(0);

    public void run() {
        for (int i = 0; i < 10000; i++) {
            x.incrementAndGet();
            x.decrementAndGet();
        }
    }

    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 **is now** always 0!
    }
}

Upvotes: 6

MK.
MK.

Reputation: 34527

AtomicInteger is what you are looking for.

Upvotes: 1

Related Questions