Is volatile enough in this case?

Viewed 197

Below is my app's start/stop procedure, start/stop code is handled by a single-threaded ThreadPoolExecutor, so I'm guaranteed there's only one thread can be active at the same time.

I'm asking about isRunning variable. Is making the variable volatile enough? The variable will be accessed (read/modify) from different threads (but only one at the same time!)

EDIT: Added variable reading (beginning of startProcedure() and stopProcedure()). I forgot about that part, my apologies.

EDIT2: I think it may be hard to notice, but startProcedure() and stopProcedure() are functions used to create startQuery and stopQuery - Runnables used by threads.

public final class Work {

    private static final ThreadPoolExecutor processor = new ThreadPoolExecutor(1, 1,
            0L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>(1),
            new ThreadPoolExecutor.DiscardPolicy());
    private static final Runnable startQuery = Work::startProcedure,
            stopQuery = Work::stopProcedure;

    private static boolean isRunning = false;

    private Work() {}

    private static void startProcedure() {
        if (isRunning) return;
        isRunning = true;
        //<some code>
    }

    private static void stopProcedure() {
        if (!isRunning) return;
        //<some code>
        isRunning = false;
    }

    //------Public API

    public static void start() {
        processor.execute(startQuery);
    }

    public static void stop() {
        processor.execute(stopQuery);
    }

}
2 Answers

First of all, volatile has very little to do with "thread-safety". volatile is about visibility and the guarantees it offers around that; specifically it works around "happens-before" principle (I will not dive into this, though).

Your case is a little bit interesting: you have a single thread. So the real question is, are actions that are done as part of the start method visible to the next stop method? In other words does ThreadPoolExecutor::execute offer any visibility guarantees?

It seems to me the answer is: yes and you do not need volatile at all. The ExecutorService, that ThreadPoolExecutor implements says:

Memory consistency effects: Actions in a thread prior to the submission of a Runnable or Callable task to an ExecutorService happen-before any actions taken by that task...

The way I interpret this is: actions that are done in the start thread will happen-before actions that are done in the stop thread. But this guarantee can only be made if start and stop are called (and wait) one after another - like in your example. As soon as you change the inner implementation, this will not work.

Yes volatile keyword is good enough for your use case.

The Java volatile keyword is used to mark a Java variable as being stored in main memory. More precisely that means, that every read of a volatile variable will be read from the computer's main memory, and not from the CPU cache, and that every write to a volatile variable will be written to main memory, and not just to the CPU cache.

Related