'Is Java volatile keyword necessary if variable gets accessed by only one Thread, but passed through lambda of other Thread?
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();
}
}
Solution 1:[1]
(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. In other words, we want mutableVariableWrapper itself to not be mutable.
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. mutableVariableWrapper.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. To emphasize this fact of being one extra step away from the payload, I changed your variable name from mutableVariable to mutableVariableWrapper in this example code.
If you merely want to see the current value, call AtomicInteger#get.
Solution 2:[2]
The volatile isn't needed in your example.
Lets expand the code a bit:
[email protected] = 2;
[email protected]();
// next line we call only the constructor; not the run method.
[email protected](new SomeRunnable())
mainThread@final Runnable r = this.blockingQueue.take();
[email protected]().this.mutableVariable++;
[email protected]().System.out.println(this.mutableVariable);
There is only 1 thread accessing the mutableVariable and that is the main thread.
And that means you don't need volatile since a single thread needs to behave as if the instructions are executed sequentially.
[edit] Solomon Slow already gave this answer in the comments.
Solution 3:[3]
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:
this.mutableVariable++will set mutableVariable to 3System.out.println(this.mutableVariable);will output 3 as required.
Sources
This article follows the attribution requirements of Stack Overflow and is licensed under CC BY-SA 3.0.
Source: Stack Overflow
| Solution | Source |
|---|---|
| Solution 1 | |
| Solution 2 | |
| Solution 3 | Orr Benyamini |
