The question refers only to the current provided example (not in general):

Is it safe to omit the the "volatile"-keyword for mutableVariable here, or is it absolutely necessary to add it in terms of thread-safety?

The mutableVariable gets only accessed in the Main-Thread, but because of the lambda-expression, I dont know if it gets "passed" through Thread-2 and therefore could be cached there, or Thread-2 sees only a cached value?

Thanks for answers and best reagards.

package test.java;

import java.util.concurrent.BlockingQueue;
import java.util.concurrent.LinkedBlockingQueue;

import org.junit.Test;

public class A {

    private final BlockingQueue<Runnable> blockingQueue = new LinkedBlockingQueue<Runnable>();
    private int mutableVariable = 1; // is "volatile"-keyword absolutely necessary here, or can it be omitted?!


    @Test
    public void test() throws InterruptedException {

        // executed in Thread-Main
        final Thread thread = new Thread(() -> {
            // executed in Thread-2
            this.blockingQueue.add(() -> {
                // executed in Thread-Main
                this.mutableVariable  ;
                System.out.println(this.mutableVariable); // should always be 3
            });
        });

        // executed in Thread-Main
        this.mutableVariable = 2;
        thread.start();
        final Runnable r = this.blockingQueue.take();
        r.run();
    }

}

CodePudding user response：

There is no need for volatile keyword in this specific case.

Lets call the main thread T1.

When you call thread.start() from T1 the value of mutableVariable is 2.

thread.start() will execute the this.blockingQueue.add(...) in a new thread T2.

Runnable r will block until the T2 thread call to this.blockingQueue.add(...) is done, and when it is done running your Runnable is in the queue.

r.run() won't be executed before T2 has finished running it's entire logic which is adding Runnable to the queue.

So when r.run() is executed:

1. this.mutableVariable will set mutableVariable to 3
2. System.out.println(this.mutableVariable); will output 3 as required.

CodePudding user response：

(Caveat: I am not a concurrency expert.)

Given your very specific situation, yes this looks to me to be thread-safe without needing volatile. But not recommended.

Yes, thread-safe

Objects established before cross-threaded use

I say thread-safe because the thread is in an instance method, so we know the A object would be instantiated before that method could be called. Since the mutableVariable is initialized in its declaration, we know the object’s existence would have begun before the thread-related method can be called. As the variable is clearly established before the thread, and you change its value only within the thread method, then there is no need for volatile.

Only one background thread

And I say thread-safe because you are starting exactly one thread.

If multiple threads were involved, this code:

this.mutableVariable  ;
// Something other thread might be incrementing this variable at this point, between these statements. 
System.out.println(this.mutableVariable); 

… would intermittently fail, with a value printed that was set by some other thread.

No, not recommended

I say not recommended because there are so many required conditions here, all implicit, that this code in practice would be quite brittle. A programmer later could easily break one of those conditions. For example, being a plain member field on A, anyone writing another method would assume they have the right to modify that value. Such a modification would not be safe.

AtomicInteger

Instead, I recommend writing self-documenting code whenever practical. In particular, using the Atomic… classes makes thread-safety concerns quite obvious.

In your case, I would use AtomicInteger. So this:

private int mutableVariable = 1 ; 

… becomes this:

private final AtomicInteger mutableVariableWrapper = new AtomicInteger( 1 ) ;

Notice the addition of final. This prevents any code from replacing our AtomicInteger object with another. Such replacement without a volatile would not be thread-safe.

The AtomicInteger has thread-safe methods for incrementing such as incrementAndGet. So this:

this.mutableVariable  ;
System.out.println(this.mutableVariable); 

… becomes this:

int newValue = this.mutableVariable.incrementAndGet() ;
System.out.println( newValue ); 

Notice in that line above how, with an AtomicInteger, we are one step away from our desired value. The AtomicInteger is a container, with its int payload nested inside. That line above does not directly address that payload value. That line asks the AtomicInteger to locate the payload, the int, and increment it on our behalf in a thread-safe manner.

If you simply want to see the current value, call AtomicInteger#get.