I know that concurrently accessing the same object from different threads, without synchronisation, is in general a bad thing. But what about this case:
I have multiple threads running (consider two, ThreadA & ThreadB). I also have this static class to keep count of the number of times a Thread does something.
public class Counter {
static private int counter=0;
static public void incCounter() {
counter++;
}
}
What happens if ThreadA and ThreadB both call Counter.incCounter()?
It's not safe.
Each thread will attempt to read counter, add one to it, and write back the result. You're not guaranteed what order these reads and writes happen in, or even if the results are visible to each thread.
In particular, one failure case would be that each thread reads the value 0, increments it to 1, and writes back the value 1. This would give the counter the value 1 even after two threads attempted to increment it.
Consider using AtomicInteger.incrementAndGet() instead.
Its value will either be 1 or 2. There's no difference between static and non static variables in this context.
It doesn't matter whether it's a static object or an instance: if you change it from multiple threads, you're going to have a problem.
to avoid conflict use the keyword synchronized.
public class Counter {
static private int counter=0;
public static synchronized void incCounter() {
counter++;
}
}
this keywords allows only one thread for time to call incCounter().
AtomicInteger.Dave is correct, but a quick fix is just to add the "synchronized" keyword to that method description; if multiple threads call that method, they will block at the method boundary until the one inside (that won the race) increments and exists, then the 2nd caller will enter.
This is a lot like designing a good "getInstance()" method on a Singleton class; you typically want it to be synchronized so you don't have the case where 2+ threads enter the method, ALL see that the "instance" is null, and then ALL create a new instance, assign it to the local member and return it.
Your threads can end up with different references to the "same" instance in that case. So you synchronize the code block, only let the first thread create the instance if it's null, and otherwise ALWAYS return the same one to all callers.
The if(instance == null) check plus the return are cheap; on the order of microseconds I believe for the future calls to getInstance (or in your example incCounter) so no need to shy away from the synchronized keyword if you need it; that's what it is for.
That being said, if you can't spare microseconds... well then you might be using the wrong language :)
