Is volatile enough in this case?

Question

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);
    }

}

You example only shows assignments to the isRunning variable. If there isn't any code that _examines_ the variable, then it doesn't matter how you declare it. If there is code that you haven't _shown_ in your example, then it's going to be pretty hard for anybody to comment on it in any meaningful way. — Solomon Slow, Sep 12 '20 at 13:49
@SolomonSlow You're right, I forgot to add the variable reading part. Just added it at the beginning of `startProcedure()` and `stopProcedure()` — WLTY, Sep 12 '20 at 13:51
@WLTY He is asking about how threads access this variable, not some trivial reads inside the setters. How is `isRunning` used in the tasks? — akuzminykh, Sep 12 '20 at 13:54
@akuzminykh read the code once again, `startProcedure()` and `stopProcedure()` are being used by the threads, there's no other code the threads perform — WLTY, Sep 12 '20 at 13:56
@WLTY As you don't show any functionality and this is all we have, then I say yes: `volatile` is enough. I'm assuming that you are only asking about visibility and nothing else. — akuzminykh, Sep 12 '20 at 14:08
@akuzminykh I think you're starting to understand me :) My question is: Is volatile enough to make isRunning visible to any next performing thread? — WLTY, Sep 12 '20 at 14:21
@WLTY However, just to point it out, `volatile` is enough in *any* case when it's about visibility *only*. If your code breaks because of wrong synchronization, that's a different story. — akuzminykh, Sep 12 '20 at 14:28
If only one thread runs at a time, there's no point of talking about thread safety. After all it's a sequential execution. — Ravindra Ranwala, Sep 12 '20 at 15:08

score 2 · Answer 1 · answered Sep 12 '20 at 18:12

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.

You may be right, but on the other hand the quoted part may mean that **submission** of the Runnable will be visible, so `ExecutorService.submit()` can be used safely across multiple threads. Here's more: https://stackoverflow.com/questions/16058419/java-executorservice-should-i-put-a-lock-before-to-use-execute — WLTY, Sep 12 '20 at 19:05

score 0 · Answer 2 · answered Sep 12 '20 at 15:15

0

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.

answered Sep 12 '20 at 15:15

Suman

818
6
17

1

there is no such thing as "main memory". and if you quote something - might include where this quote comes from. and a simple "yes" is no answer either – Eugene Sep 12 '20 at 17:23
Do correct and enlighten me with your understanding and share how I am wrong. – Suman Sep 12 '20 at 18:31
Volatile doesn't mean every read will happen from the computer's main memory. This is a common misconception. If volatiles were read/written from main memory every time, the performance impact would be very underwhelming. The actual cost depends on the CPU architecture which, in a broader context, using protocols such as the MESI, shares a **coherent cache line** ("spying other caches") enforcing that when two threads read from the same memory address, they should never simultaneously read different values. https://software.rajivprab.com/2018/04/29/myths-programmers-believe-about-cpu-caches/ – Pedro Lourenço Oct 05 '20 at 09:10

Is volatile enough in this case?

2 Answers2