21

I'm making a program in java that races a few cars against each other. Each car is a separate thread.

When cars complete the race, each one all calls this method. I've tested the method at varying timer speeds, and it seems to work fine. But I do realize that each thread is accessing the variable carsComplete, sometimes at the exact same time (at least at the scope the date command is giving me).

So my question is: is this method thread-safe?

 public static String completeRace() { Date accessDate = new Date(); System.out.println("Cars Complete: " + carsComplete + " Accessed at " + accessDate.toString()); switch(++carsComplete) { case 1: return "1st"; case 2: return "2nd"; case 3: return "3rd"; default: return carsComplete + "th"; } }
Improve this question

5 Answers 5

Reset to default
29

No, you should be using something like java.util.concurrent.atomic.AtomicInteger. Look at its getAndIncrement() method.

Improve this answer
Sign up to request clarification or add additional context in comments.

6 Comments

Or synchronize on something before incrementing.
@Ted: I'd bet that syncing on something is more expensive.
@Eric - I'd be surprised if AtomicInteger didn't do some syncing at some point (perhaps in native code). It would be interesting to benchmark. From the source for AtomicInteger, it seems to depend on an atomic compare-and-set algorithm that retries until it detects no problems. There's no worst-case bound for this; in highly threaded scenarios, I think it could be very slow at random times.
@Ted Hopp The CAS loop should be faster than any real lock though (how would one even get cheaper than a simple CAS?). And if you need a fair lock, that will cost much more performance for a rather unlikely scenario.
@Voo - on the average, I agree: CAS loops tend to be fast. My point, though, was that in a highly contentious setting, any particular thread can starve indefinitely. (Then there's the issue that not every hardware architecture supports atomic CAS. However, I'll concede that just about any machine running Java does.)
|
9

Pre-increment on int is not thread safe, use AtomicInteger which is lock-free:

AtomicInteger carsComplete = new AtomicInteger(); //... switch(carsComplete.incrementAndGet())

BTW the code below is not thread safe as well. Can you tell why?

carsComplete.incrementAndGet(); switch(carsComplete.get())
Improve this answer

2 Comments

Because thread 1 can reach the switch statement, yield to thread 2, which runs incrementAndGet thus incrementing the backing store for carsComplete, at which point thread 1 executes again with the backing store having been incremented twice, once by each thread.
Exactly, however no prize this time ;-).
9

++ operator is not atomic. Look at here http://madbean.com/2003/mb2003-44/. For atomic operations you can use AtomicInteger

AtomicInteger atomicInteger = new java.util.concurrent.atomic.AtomicInteger(0)

and every time you want to increment you can call atomicInteger.incrementAndGet() method which returns a primitive int. 0 is the default initial value for the atomic integer.

Improve this answer

2 Comments

i am still not getting why atomic operations are thread safe ? what happens when two threads executing CAS at the same time ?? isn't it a race condition ??
@Hardik there is no race condition because happens before(en.wikipedia.org/wiki/Happened-before) relation is enforced by atomic operations and hence the resulting thread safety.
6

Same as in C++ the operator ++ is not atomic.

It is actually more than 1 instruction being executed under the hood (don't be fooled by seeing just a simple ++i; it is load/add/store) and since there is more than 1 instruction involved without synchronization you may have various interleavings with wrong results.

If you need to incrent the carsComplete in a thread-safe manner you can use java's construct AtomicInteger or you could synchronize the whole method

Improve this answer

Comments

3

Question is “is pre increment operator thread safe ?”

Ans : No , Why ? because of number of instructions involved. Atomic means single operation , here load/add/store operations need to be performed. So not an atomic operation.

 Same for post increment.
Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.