2

Whenever I run this program it gives me different result. Can someone explain to me, or give me some topics where I could find answer in order to understand what happens in the code?

class IntCell {
    private int n = 0;
    public int getN() {return n;}
    public void setN(int n) {this.n = n;}
}

public class Count extends Thread {
    static IntCell n = new IntCell();

    public void run() {
      int temp;
      for (int i = 0; i < 200000; i++) {
        temp = n.getN(); 
        n.setN(temp + 1);
      }
    }

    public static void main(String[] args) {
      Count p = new Count();
      Count q = new Count();
      p.start();
      q.start();
      try { p.join(); q.join(); }
      catch (InterruptedException e) { }
      System.out.println("The value of n is " + n.getN());
    }
}
Derik Daro
  • 97
  • 2
  • 9
  • 1
    I think you need to start from the [_very beginning_](https://docs.oracle.com/cd/E19455-01/806-5257/6je9h032e/index.html), learning about concurrency in Java. This code looks like it's designed to produce random results; it's a fundamental point that without memory barriers concurrency is unpredictable - that's what makes it so hard. In short, explaining why this code produces random answers is unlikely to help you - you need to do **much** more reading before even attempting to write concurrent code. – Boris the Spider Jan 14 '17 at 15:46

4 Answers4

4

The reason is simple: you don't get and modify your counter atomically such that your code is prone to race condition issues.

Here is an example that illustrates the problem:

  1. Thread #1 calls n.getN() gets 0
  2. Thread #2 calls n.getN() gets 0
  3. Thread #1 calls n.setN(1) to set n to 1
  4. Thread #2 is not aware that thread #1 has already set n to 1 so still calls n.setN(1) to set n to 1 instead of 2 as you would expect, this is called a race condition issue.

Your final result would then depend on the total amount of race condition issues met while executing your code which is unpredictable so it changes from one test to another.

One way to fix it, is to get and set your counter in a synchronized block in order to do it atomically as next, indeed it will enforce the threads to acquire an exclusive lock on the instance of IntCell assigned to n before being able to execute this section of code.

synchronized (n) {
    temp = n.getN();
    n.setN(temp + 1);
}

Output:

The value of n is 400000

You could also consider using AtomicInteger instead of int for your counter in order to rely on methods of type addAndGet(int delta) or incrementAndGet() to increment your counter atomically.

Nicolas Filotto
  • 43,537
  • 11
  • 94
  • 122
1

The access to the IntCell n static variable is concurrent between your two threads :

static IntCell n = new IntCell();

public void run() {
  int temp;
  for (int i = 0; i < 200000; i++) {
    temp = n.getN(); 
    n.setN(temp + 1);
  }
}

Race conditions make that you cannot have a predictable behavior when n.setN(temp + 1); is performed as it depends on which thread has previously called :temp = n.getN();.
If it the current thread, you have the value put by the thread otherwise you have the last value put by the other thread.

You could add synchronization mechanism to avoid the problem of unexpected behavior.

davidxxx
  • 125,838
  • 23
  • 214
  • 215
0

You are running 2 threads in parallel and updating a shared variable by these 2 threads, that is why your answer is always different. It is not a good practice to update shared variable like this.

To understand, you should first understand Multithreading and then notify and wait, simple cases

Community
  • 1
  • 1
Vikas Sachdeva
  • 5,633
  • 2
  • 17
  • 26
0

You modify the same number n with two concurrent Threads. If Thread1 reads n = 2, then Thread2 reads n = 2 before Thread2 has written the increment, Thread1 will increment n to 3, but Thread2 will no more increment, but write another "3" to n. If Thread1 finishes its incrementation before Thread2 reads, both will increment.

Now both Threads are concurrent and you can never tell which one will get what CPU cycle. This depends on what else runs on your machine. So You will always lose a different number of incrementations by the above mentioned overwriting situation.

To solve it, run real incrementations on n via n++. They go in a single CPU cycle.

Martin G
  • 229
  • 1
  • 6