Reputation: 10531
Because it always prints out '3'. No synchronization needed? I am testing this simple thing because I am having a trouble in a real multiple thread problem, which isn't good to illustrate the problem, because it's large. This is a simplified version to showcase the situation.
class Test {
public static int count = 0;
class CountThread extends Thread {
public void run()
{
count++;
}
}
public void add(){
CountThread a = new CountThread();
CountThread b = new CountThread();
CountThread c = new CountThread();
a.start();
b.start();
c.start();
try {
a.join();
b.join();
c.join();
} catch (InterruptedException ex) {
ex.printStackTrace();
}
}
public static void main(String[] args) {
Test test = new Test();
System.out.println("START = " + Test.count);
test.add();
System.out.println("END: Account balance = " + Test.count);
}
Upvotes: 0
Views: 120
Reputation: 9711
To make the class threadsafe either make count volatile
to force memory fences between threads, or use AtomicInteger, or rewrite like this (my preference):
class CountThread extends Thread {
private static final Object lock = new Object();
public void run()
{
synchronized(lock) {
count++;
}
}
}
Upvotes: 0
Reputation: 4973
Seems to me like count++
is fast enough to finish until you invoke 'run' for the other class. So basically it runs sequential.
But, if this was a real life example, and two different threads were usingCountThread
parallelly, then yes, you would have synchronization problem.
To verify that, you can try to print some test output before count++ and after, then you'll see if b.start()
is invoking count++
before a.start()
finished. Same for c.start()
.
Consider using AtomicInteger instead, which is way better than synchronizing when possible -
incrementAndGet
public final int incrementAndGet()
Atomically increments by one the current value.
Upvotes: 2
Reputation: 108859
This code is not thread-safe:
public static int count = 0;
class CountThread extends Thread {
public void run()
{
count++;
}
}
You can run this code a million times on one system and it might pass every time. This does not mean is it is thread-safe.
Consider a system where the value in count
is copied to multiple processor caches. They all might be updated independently before something forces one of the caches to be copied back to main RAM. Consider that ++
is not an atomic operation. The order of reading and writing of count
may cause data to be lost.
The correct way to implement this code (using Java 5 and above):
public static java.util.concurrent.atomic.AtomicInteger count =
new java.util.concurrent.atomic.AtomicInteger();
class CountThread extends Thread {
public void run()
{
count.incrementAndGet();
}
}
Upvotes: 1
Reputation: 116828
Because it always prints out '3'. No synchronization needed?
It is not thread safe and you are just getting lucky. If you run this 1000 times, or on different architectures, you will see different output -- i.e. not 3.
I would suggest using AtomicInteger
instead of a static field ++ which is not synchronized
.
public static AtomicInteger count = new AtomicInteger();
...
public void run() {
count.incrementAndGet();
}
...
Upvotes: 4
Reputation: 100527
It is not thread safe.
It just happened to be way to short to have measurable chance to show the issue. Consider counting to much higher number (1000000?) in run
to increase chance of 2 operations on multiple threads to overlap.
Also make sure your machine is not single core CPU...
Upvotes: 0
Reputation: 41958
It's not thread safe just because the output is right. Creating a thread causes a lot of overhead on the OS side of things, and after that it's just to be expected that that single line of code will be done within a single timeslice. It's not thread safe by any means, just not enough potential conflicts to actually trigger one.
Upvotes: 0