Conditional checking on Atomic Integer thread safe?

Question

Sample code

@ApplicationScoped
public class AtomicIntegerSample {

private final AtomicInteger successFullSyncCount = new AtomicInteger(0);
private final AtomicLong overallSyncTimeTaken  = new AtomicLong(0);

public void incrementSuccessFullSyncCount() {
    this.successFullSyncCount.incrementAndGet();
}


public void addOverallSyncTimeTaken(final Long syncTimeTaken) {
    overallSyncTimeTaken.addAndGet(syncTimeTaken);
}
/**
 * Can be called by multiple threads
 * @return
 */
public long getAverageSuccessfulSyncTimeTaken() {
    if (successFullSyncCount.get() == 0) {
        return 0;
    } else {
        return overallSyncTimeTaken.get() / successFullSyncCount.get();
    }
}

In method getAverageSuccessfulSyncTimeTaken() there is conditional check to avoid arithmetic exception.

The class ApplicationScoped, and this method can be called by multiple threads concurrently.

Is the method thread safe?. If I replace the code as follows, is it thread safe?

 /**
 * Can be called by multiple threads
 * @return
 */
public long getAverageSuccessfulSyncTimeTaken() {
       return overallSyncTimeTaken.get() / successFullSyncCount.get();

}

Then arithmetic exception will be thrown. How to optimally synchronize this code?. I mean only if successFullSyncCount.get() == 0

Answer 1

It won't throw any exception, but it could return incorrect results:

• the time and the count are incremented separately. They're not incremented both at once, in a single atomic operation
• the time and the count are read separately, instead of being read both at once in a single atomic operation.

So, you could very well read a time of 100 in a thread, then have another thread increment the count a few times, then read the count and perform the division. That can thus lead to an average time per sync that is lower than it is in reality. It could be a good enough approximation, or not.