Reputation: 3422
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
Reputation: 45005
Because the way you increment your variable is not an atomic operation indeed to increment it you:
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
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
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