3

I get the same output with and without the volatile keyword.

Have I made a mistake in the code?

public class MutliThreadExample extends Thread{

    volatile int value;
    
    public static void main(String[] args) throws InterruptedException {
        MutliThreadExample thread1 = new MutliThreadExample();
        MutliThreadExample thread2 = new MutliThreadExample();
        thread1.start();
        thread1.join();
        thread2.start();
    }

    @Override
    public void run() {
        for (int i = 1; i <= 5; i++) {
            value = value + 1;
            System.out.println(value + "-" + Thread.currentThread());
            try {
                Thread.sleep(1000);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }

}

I am not able to use volatile keyword properly in Java.

Bohemian
  • 412,405
  • 93
  • 575
  • 722
shravan
  • 31
  • 1
  • 3
    `value = value + 1` requires an `AtomicInteger`, not just a volatile. – Louis Wasserman Apr 22 '23 at 18:31
  • 2
    Hi & welcome! :) ..maybe...but: `thread1.start();thread1.join();thread2.start();` *is sequential* (i.e. *not parallel*) – xerx593 Apr 22 '23 at 18:32
  • 1
    `volatile` makes reads and writes atomic, but not their combination. See, for example, https://www.scaler.com/topics/volatile-keyword-in-java/. Incrementing a volatile variable is _not_ safe and needs a full `AtomicInteger`. – Louis Wasserman Apr 22 '23 at 18:45
  • @Louis Wasserman `volatile` is not about atomicity, do not please confuse others: https://stackoverflow.com/questions/19744508/volatile-vs-atomic#:~:text=Volatile%20and%20Atomic%20are%20two,on%20variables%20are%20performed%20atomically. – Mikhail2048 Apr 22 '23 at 19:39
  • 1
    Each instance of `MutliThreadExample` has its own `value`, so there is only one thread accessing it anyway; volatile isn't really necessary for your code. – Mark Rotteveel Apr 23 '23 at 11:46

1 Answers1

6

Am I doing anything wrong in this ?

Yes. Multiple things.

Firstly, your application is effectively single-threaded (NOT multi-threaded) because of this:

    thread1.start();
    thread1.join();
    thread2.start();

What that says is

  1. Start thread1
  2. Wait for thread1 to finish
  3. Start thread2.

So thread1 and thread2 will not be running at the same time.

Secondly, the threads do this:

    Thread.sleep(1000);

When you call sleep, variable values that are cached in registers, etcetera are liable to be written to main memory. That is liable to make this code behave differently, and interfere with any observations you are attempting to make.

Thirdly, they do this:

    System.out.println(value + "-" + Thread.currentThread());

In the println call, the I/O stack typically does some synchronization under the hood, and that will also interfere with the observation of value. Furthermore value may have changed since you incremented it.

Fourthly, you increment the variable like this:

    value = value + 1;

That is not an atomic operation, even when value is declared as volatile. It is in fact a read operation followed by an addition followed by a write operation. The same would be true if you wrote

    value++;  // or ++value;

If two threads actually execute these statement simultaneously, they are liable to lose an increment due to the interleaving of the read, add, write operations.

(Declaring a variable as volatile gives a happens before relationship between a write of the variable by one thread a subsequent reads of the same variable by other threads. But that is not sufficient to implement a counter that can be updated by two or more threads in a thread-safe fashion. To do that you need to use synchronized, an AtomicInteger or equivalent.)

Finally, you have declared value as an instance variable of MutliThreadExample, and thread1 and thread2 are different instances of that class. That means that thread1 and thread2 are updating different value variables.


In summary your code produces the same output in both versions because it is not multi-threaded. The sleep and println calls are also liable to interfere with the "observations". And the threads are updating different value variables anyway.

If you fixed all of those problems, your code is not thread-safe, either with the volatile or without it. Neither version would be guaranteed produce either correct or incorrect answers. They would both have non-deterministic behavior.

Reliably demonstrating the effects of volatile is difficult. And making basic programming mistakes doesn't help.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216