miechooy
miechooy

Reputation: 3422

Java threading issue?

I am wondering why the result is not 400 000. There are two threads why does it gets blocked?

class IntCell {
    private int n = 0;
    public int getN() {return n;}
    public void setN(int n) {this.n = n;}
}
class Count extends Thread {
    private static IntCell n = new IntCell();
    @Override 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());
    }
}

Why there is so problem with that?

Upvotes: 1

Views: 90

Answers (3)

Nicolas Filotto
Nicolas Filotto

Reputation: 45005

Because the way you increment your variable is not an atomic operation indeed to increment it you:

  1. Get the previous value
  2. Add one to this value
  3. Set a new value

They are 3 operations not done atomically you should either us a synchronized block or use an AtomicInteger instead.

With a synchronized block it would be something like:

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

With an AtomicInteger you will need to rewrite your code as next:

class IntCell {
    private final AtomicInteger n = new AtomicInteger();
    public int getN() {return n.get();}
    public void incrementN(int n) {this.n.addAndGet(n);}
}

for (int i = 0; i < 200000; i++) {
    n.incrementN(1);
}

The approach with an AtomicInteger is non blocking so it will be faster

Upvotes: 6

rtoip
rtoip

Reputation: 172

When two threads access one object at the same time, they interfere with each other, and the result is not deterministic. For example, imagine that p reads the value of n and gets, say, 0, then q reads the same value and gets 0 too, then p sets value to 1 and q also sets it to 1 (because it still thinks that it has value 0). Now the value of n is increased by 1, even though both counters "incremented" it once. You need to use synchronized block to make sure the counters won't interfere with each other. See https://docs.oracle.com/javase/tutorial/essential/concurrency/locksync.html for more.

Upvotes: 2

Mureinik
Mureinik

Reputation: 312249

The problem here is that you allow for race conditions. Consider the block inside the loop:

temp = n.getN();
n.setN(temp + 1);

The code context switch between the time you get the current N and by the time you increment it, making you set an "old" value. One way around this is to ensure the inner part of the loop runs in a synchronized block:

for (int i = 0; i < 200000; i++) {
    synchronized (n) { / Here!
        temp = n.getN();
        n.setN(temp + 1);
    }
}

Upvotes: 2

Related Questions