Kdtyjb

Let's say I have a runnable with a simple integer count variable which is incremented every time runnable runs. One instance of this object is submitted to run periodically in a scheduled executor service.

class Counter implements Runnable {

    private int count = 0;



    @Override

    public void run() {

      count++;

    }

}



Counter counter = new Counter();

ScheduledExecutorService executorService = Executors.newScheduledThreadPool(5);

executorService.scheduleWithFixedDelay(counter, 1, 1, TimeUnit.SECONDS);

Here, the object is accessing its own internal state inside of different threads (reading and incrementing). Is this code thread-safe or is it possible that we lose updates to the count variable when it's scheduled in a different thread?

Does an object always see its latest internal state irrespective of thread?

Just to be clear for the purposes of this question and its answers, an object doesn't do anything; it's just memory. Threads are the executing entity. It's misleading to say does an object see whatever. It's the thread that's doing the seeing/reading of object state.

This isn't specified in the javadoc, but

Executors.newScheduledThreadPool(5);

returns a ScheduledThreadPoolExecutor.

Your code is using

executorService.scheduleWithFixedDelay(counter, 1, 1, TimeUnit.SECONDS);

The javadoc for ScheduledThreadPoolExecutor#scheduledWithFixedDelay states

Submits a periodic action that becomes enabled first after the given
initial delay, and subsequently with the given delay between the
termination of one execution and the commencement of the next.

The class javadoc further clarifies

Successive executions of a periodic task scheduled via
scheduleAtFixedRate or scheduleWithFixedDelay do not overlap.
While different executions may be performed by different threads, the
effects of prior executions happen-before those of subsequent ones.

As such, each execution of Counter#run is guaranteed to see the value of count after it's been incremented by the previous execution. For example, the third execution will read a count value of 2 before it performs its increment.

You don't need volatile or any other additional synchronization mechanism for this specific use case.

No, this code is not thread-safe because there isn't any happens-before relation between increments made in different threads started with ScheduledExecutorService.

To fix it, you need to either mark the variable as volatile or switch to AtomicInteger or AtomicLong.

UPDATE:

As @BoristheSpider mentioned, in general in case of increment/decrement making a variable volatile is not enough since increment/decrement is not atomic itself and calling it from several threads concurrently will cause race conditions and missed updates. However, in this particular case scheduleWithFixedDelay() guarantees (according to Javadoc) that there will be overlapping executions of scheduled task, so volatile will also work in this particular case even with increment.

No, this code is not thread-safe since there isn't any happens before relation between different threads accessing count.