Run runnable by thread pool, should I use volatile?

Question

Like the example code like follows.

I'm going to using a thread pool to run the object of TestRunnable in period.

Should I declare the variable total as volatile?

public class TestRunnable implements Runnable {
    private int total;
    @Override
    public void run() {
        if (total > 10) {
            return;
        } else {
            total += 1;
            System.out.print("Run in times: " + total);
        }
    }
}

Answer 1

Assuming the fact, that your variable is declared as

 private int total;

(Not static)

It will not be shared between multiple threads (As long as you keep creating new instance for each submitted thread), so there is no need to declare it as volatile.

If you do use the same instance multiple times - then you should consider using AtomicInteger instead of regular int, since operation

total+= 1;

or

total++;

Is not an atomic operation, which might lead to some unexpected results in multithreaded env.

Answer 2

Even if you would mark the field volatile it would be worth nothing since the if statement isn't atomic. Consider the following situation:

1. A thread executes TestRunnable's run() method.
2. The expression total > 10 evaluates to false because total is 10.
3. As a result, the thread enters the else branch and trys to increment the variable.
4. Another thread does the same, but total hasn't been incremented yet.
5. Now, both threads incremented total, which could leave it in an inconsistent state.

Note that this can only happen if you submit the same instance multiple times to your pool (at least that's the only situation I can think of). However, since you're managing state within that Runnable—which seems wrong to me—that's probably what you're doing.

As pointed out by @DmitriyKotov, you should switch to an AtomicInteger. Since Java 8 you could easily do:

total.getAndUpdate(currentVal -> currentVal > 10 ? currentVal : currentVal + 1);